Merge ~cjwatson/launchpad:tracer-action-leak into launchpad:master

[Colin Watson]

Clear connection._lp_statement_action after tracing it

Since database connections are typically cached, leaving TimedAction references lying around on them makes life more difficult for the cyclic garbage collector and makes it harder to track down memory leaks.

[Cristian Gonzalez (cristiangsp)]

Looks good!

review: Approve

diff --git a/lib/lp/services/webapp/adapter.py b/lib/lp/services/webapp/adapter.py
index 637ad17..a707e11 100644
--- a/lib/lp/services/webapp/adapter.py
+++ b/lib/lp/services/webapp/adapter.py
@@ -637,6 +637,7 @@ class LaunchpadTimeoutTracer(PostgresTimeoutTracer):
                 # action may be None if the tracer was installed after
                 # the statement was submitted.
                 action.finish()
+                connection._lp_statement_action = None
             info = sys.exc_info()
             transaction.doom()
             try:
@@ -732,6 +733,7 @@ class LaunchpadStatementTracer:
             # statement was submitted or if the timeline tracer is not
             # installed.
             action.finish()
+            connection._lp_statement_action = None
         log_sql = getattr(_local, 'sql_logging', None)
         if log_sql is not None or self._debug_sql or self._debug_sql_extra:
             data = None
diff --git a/lib/lp/services/webapp/tests/test_statementtracer.py b/lib/lp/services/webapp/tests/test_statementtracer.py
index bc5ddef..4ba6b53 100644
--- a/lib/lp/services/webapp/tests/test_statementtracer.py
+++ b/lib/lp/services/webapp/tests/test_statementtracer.py
@@ -368,3 +368,23 @@ class TestLoggingWithinRequest(TestCaseWithFactory):
                 tracer.connection_raw_execute_success(
                     self.connection, None, 'SELECT * FROM three', ())
             self.assertEqual(['SELECT * FROM three'], logger.statements)
+
+    def test_clears_timeline_action_reference(self):
+        # The tracer doesn't leave TimedAction references lying around in
+        # the connection.
+        tracer = da.LaunchpadStatementTracer()
+        with person_logged_in(self.person):
+            with StormStatementRecorder():
+                tracer.connection_raw_execute(
+                    self.connection, None, 'SELECT * FROM one', ())
+                self.assertIsNotNone(self.connection._lp_statement_action)
+                tracer.connection_raw_execute_success(
+                    self.connection, None, 'SELECT * FROM one', ())
+                self.assertIsNone(self.connection._lp_statement_action)
+                tracer.connection_raw_execute(
+                    self.connection, None, 'SELECT * FROM one', ())
+                self.assertIsNotNone(self.connection._lp_statement_action)
+                tracer.connection_raw_execute_error(
+                    self.connection, None, 'SELECT * FROM one', (),
+                    Exception())
+                self.assertIsNone(self.connection._lp_statement_action)