From 8482ce6b8ca9f1110b86bff4c3f998655fdecf8a Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 May 2016 21:46:56 +0200 Subject: [PATCH] Improve application context popping Exceptions during teardown handling will no longer leave application contexts lingering around. This fixes #1767 --- CHANGES | 2 ++ flask/ctx.py | 80 +++++++++++++++++++++++--------------------- tests/test_appctx.py | 22 ++++++++++++ 3 files changed, 66 insertions(+), 38 deletions(-) diff --git a/CHANGES b/CHANGES index 9e3b805f..0fa71e08 100644 --- a/CHANGES +++ b/CHANGES @@ -77,6 +77,8 @@ Version 0.11 - ``send_from_directory`` now raises BadRequest if the filename is invalid on the server OS (pull request ``#1763``). - Added the ``JSONIFY_MIMETYPE`` configuration variable (pull request ``#1728``). +- Exceptions during teardown handling will no longer leave bad application + contexts lingering around. Version 0.10.2 -------------- diff --git a/flask/ctx.py b/flask/ctx.py index 3401bd79..480d9c5c 100644 --- a/flask/ctx.py +++ b/flask/ctx.py @@ -181,12 +181,14 @@ class AppContext(object): def pop(self, exc=_sentinel): """Pops the app context.""" - self._refcnt -= 1 - if self._refcnt <= 0: - if exc is _sentinel: - exc = sys.exc_info()[1] - self.app.do_teardown_appcontext(exc) - rv = _app_ctx_stack.pop() + try: + self._refcnt -= 1 + if self._refcnt <= 0: + if exc is _sentinel: + exc = sys.exc_info()[1] + self.app.do_teardown_appcontext(exc) + finally: + rv = _app_ctx_stack.pop() assert rv is self, 'Popped wrong app context. (%r instead of %r)' \ % (rv, self) appcontext_popped.send(self.app) @@ -341,38 +343,40 @@ class RequestContext(object): """ app_ctx = self._implicit_app_ctx_stack.pop() - clear_request = False - if not self._implicit_app_ctx_stack: - self.preserved = False - self._preserved_exc = None - if exc is _sentinel: - exc = sys.exc_info()[1] - self.app.do_teardown_request(exc) - - # If this interpreter supports clearing the exception information - # we do that now. This will only go into effect on Python 2.x, - # on 3.x it disappears automatically at the end of the exception - # stack. - if hasattr(sys, 'exc_clear'): - sys.exc_clear() - - request_close = getattr(self.request, 'close', None) - if request_close is not None: - request_close() - clear_request = True - - rv = _request_ctx_stack.pop() - assert rv is self, 'Popped wrong request context. (%r instead of %r)' \ - % (rv, self) - - # get rid of circular dependencies at the end of the request - # so that we don't require the GC to be active. - if clear_request: - rv.request.environ['werkzeug.request'] = None - - # Get rid of the app as well if necessary. - if app_ctx is not None: - app_ctx.pop(exc) + try: + clear_request = False + if not self._implicit_app_ctx_stack: + self.preserved = False + self._preserved_exc = None + if exc is _sentinel: + exc = sys.exc_info()[1] + self.app.do_teardown_request(exc) + + # If this interpreter supports clearing the exception information + # we do that now. This will only go into effect on Python 2.x, + # on 3.x it disappears automatically at the end of the exception + # stack. + if hasattr(sys, 'exc_clear'): + sys.exc_clear() + + request_close = getattr(self.request, 'close', None) + if request_close is not None: + request_close() + clear_request = True + finally: + rv = _request_ctx_stack.pop() + + # get rid of circular dependencies at the end of the request + # so that we don't require the GC to be active. + if clear_request: + rv.request.environ['werkzeug.request'] = None + + # Get rid of the app as well if necessary. + if app_ctx is not None: + app_ctx.pop(exc) + + assert rv is self, 'Popped wrong request context. ' \ + '(%r instead of %r)' % (rv, self) def auto_pop(self, exc): if self.request.environ.get('flask._preserve_context') or \ diff --git a/tests/test_appctx.py b/tests/test_appctx.py index f24704ef..13b61eee 100644 --- a/tests/test_appctx.py +++ b/tests/test_appctx.py @@ -146,3 +146,25 @@ def test_context_refcounts(): assert res.status_code == 200 assert res.data == b'' assert called == ['request', 'app'] + + +def test_clean_pop(): + called = [] + app = flask.Flask(__name__) + + @app.teardown_request + def teardown_req(error=None): + 1 / 0 + + @app.teardown_appcontext + def teardown_app(error=None): + called.append('TEARDOWN') + + try: + with app.test_request_context(): + called.append(flask.current_app.name) + except ZeroDivisionError: + pass + + assert called == ['test_appctx', 'TEARDOWN'] + assert not flask.current_app