launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/python-oops-wsgi/traceback-reference-cycle into lp:python-oops-wsgi with lp:~cjwatson/python-oops-wsgi/tox as a prerequisite.

Commit message:
Avoid traceback reference cycles.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/python-oops-wsgi/traceback-reference-cycle/+merge/402761

`sys.exc_info()` returns a tuple containing the exception's traceback object, which has references to the frames in that traceback.  Storing that in a local variable in one of those frames creates a reference cycle.  Clear those variables in `finally` blocks to avoid this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/python-oops-wsgi/traceback-reference-cycle into lp:python-oops-wsgi.

=== modified file 'NEWS'
--- NEWS	2021-05-14 11:57:27 +0000
+++ NEWS	2021-05-14 11:57:27 +0000
@@ -7,6 +7,7 @@
 ----
 
 * Add tox testing support. (Colin Watson)
+* Avoid traceback reference cycles. (Colin Watson)
 
 0.0.14
 ------

=== modified file 'oops_wsgi/middleware.py'
--- oops_wsgi/middleware.py	2018-05-08 10:31:49 +0000
+++ oops_wsgi/middleware.py	2021-05-14 11:57:27 +0000
@@ -171,10 +171,13 @@
                     except SoftRequestTimeout:
                         exc_info = sys.exc_info()
                     do_oops = True
-                if do_oops:
-                    report = config.create(make_context(exc_info=exc_info))
-                    report['HTTP_STATUS'] = status.split(' ')[0]
-                    config.publish(report)
+                try:
+                    if do_oops:
+                        report = config.create(make_context(exc_info=exc_info))
+                        report['HTTP_STATUS'] = status.split(' ')[0]
+                        config.publish(report)
+                finally:
+                    del exc_info
                 state['response'] = (status, headers)
             return oops_write
         try:
@@ -212,7 +215,10 @@
             raise
         except Exception:
             exc_info = sys.exc_info()
-            return [on_exception(exc_info)]
+            try:
+                return [on_exception(exc_info)]
+            finally:
+                del exc_info
 
     return oops_middleware
 
@@ -251,7 +257,10 @@
         raise
     except Exception:
         exc_info = sys.exc_info()
-        yield on_error(exc_info)
+        try:
+            yield on_error(exc_info)
+        finally:
+            del exc_info
     finally:
         if hasattr(app_body, 'close'):
             app_body.close()

=== modified file 'oops_wsgi/tests/test_middleware.py'
--- oops_wsgi/tests/test_middleware.py	2018-05-08 10:31:49 +0000
+++ oops_wsgi/tests/test_middleware.py	2021-05-14 11:57:27 +0000
@@ -424,8 +424,11 @@
                 raise ValueError('boom, yo')
             except ValueError:
                 exc_info = sys.exc_info()
-            start_response(
-                '500 FAIL', [('content-type', 'text/plain')], exc_info)
+            try:
+                start_response(
+                    '500 FAIL', [('content-type', 'text/plain')], exc_info)
+            finally:
+                del exc_info
             yield 'body'
         config = self.config_for_oopsing()
         contents = self.wrap_and_run(inner, config)
@@ -455,8 +458,11 @@
                 raise ValueError('boom, yo')
             except ValueError:
                 exc_info = sys.exc_info()
-            start_response(
-                '500 FAIL', [('content-type', 'text/plain')], exc_info)
+            try:
+                start_response(
+                    '500 FAIL', [('content-type', 'text/plain')], exc_info)
+            finally:
+                del exc_info
             yield 'body'
         config = self.config_for_oopsing(capture_create=True)
         self.wrap_and_run(inner, config)