Improve application context popping

Exceptions during teardown handling will no longer leave application
contexts lingering around.  This fixes pallets#1767

Author: mitsuhiko

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
 --------------

flask/ctx.py

@@ -181,12 +181,14 @@ def push(self):
 
     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 @@ def pop(self, exc=_sentinel):
         """
         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 \

tests/test_appctx.py

@@ -146,3 +146,25 @@ def index():
     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