Strong reference listen function wrapped by "once"

Fixed issue in event system where using the ``once=True`` flag with
dynamically generated listener functions would cause event registration of
future events to fail if those listener functions were garbage collected
after they were used, due to an assumption that a listened function is
strongly referenced.  The "once" wrapped is now modified to strongly
reference the inner function persistently, and documentation is updated
that using "once" does not imply automatic de-registration of listener
functions.

Fixes: #4794
Change-Id: I06388083d1633dcc89e8919eb1e51270f966df38
(cherry picked from commit e22d2b089fad1f79d53bc6238626f32f0e796ad0)

diff --git a/doc/build/changelog/unreleased_13/4794.rst b/doc/build/changelog/unreleased_13/4794.rst
new file mode 100644 (file)
index 0000000..7ba9e92
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4794.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, events
+    :tickets: 4794
+
+    Fixed issue in event system where using the ``once=True`` flag with
+    dynamically generated listener functions would cause event registration of
+    future events to fail if those listener functions were garbage collected
+    after they were used, due to an assumption that a listened function is
+    strongly referenced.  The "once" wrapped is now modified to strongly
+    reference the inner function persistently, and documentation is updated
+    that using "once" does not imply automatic de-registration of listener
+    functions.

diff --git a/lib/sqlalchemy/event/api.py b/lib/sqlalchemy/event/api.py
index e7c30901ff7fe59ac9d24f87e0a8dfcf79dd6c89..260d6e86f1d60d22f763013bf71d3794d5ede0bd 100644 (file)
--- a/lib/sqlalchemy/event/api.py
+++ b/lib/sqlalchemy/event/api.py
@@ -64,6 +64,13 @@ def listen(target, identifier, fn, *args, **kw):
     .. versionadded:: 0.9.4 Added ``once=True`` to :func:`.event.listen`
        and :func:`.event.listens_for`.
 
+    .. warning:: The ``once`` argument does not imply automatic de-registration
+       of the listener function after it has been invoked a first time; a
+       listener entry will remain associated with the target object.
+       Associating an arbitrarily high number of listeners without explictitly
+       removing them will cause memory to grow unbounded even if ``once=True``
+       is specified.
+
     .. note::
 
         The :func:`.listen` function cannot be called at the same time
@@ -124,6 +131,13 @@ def listens_for(target, identifier, *args, **kw):
     .. versionadded:: 0.9.4 Added ``once=True`` to :func:`.event.listen`
        and :func:`.event.listens_for`.
 
+    .. warning:: The ``once`` argument does not imply automatic de-registration
+       of the listener function after it has been invoked a first time; a
+       listener entry will remain associated with the target object.
+       Associating an arbitrarily high number of listeners without explictitly
+       removing them will cause memory to grow unbounded even if ``once=True``
+       is specified.
+
     .. seealso::
 
         :func:`.listen` - general description of event listening

diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py
index 41a20c41d159f26b8d3129eb8510acfbef25552d..a5c1f4803465efeedd2697998bab047bee706aa8 100644 (file)
--- a/lib/sqlalchemy/util/langhelpers.py
+++ b/lib/sqlalchemy/util/langhelpers.py
@@ -1438,6 +1438,9 @@ def only_once(fn):
     once = [fn]
 
     def go(*arg, **kw):
+        # strong reference fn so that it isn't garbage collected,
+        # which interferes with the event system's expectations
+        strong_fn = fn  # noqa
         if once:
             once_fn = once.pop()
             return once_fn(*arg, **kw)

diff --git a/test/base/test_events.py b/test/base/test_events.py
index 3103b5f066b6cdc44b49ae489dfba7527d70812e..c12b3414c761a4d193ab0f4b47c904b74fbf6a73 100644 (file)
--- a/test/base/test_events.py
+++ b/test/base/test_events.py
@@ -1146,6 +1146,33 @@ class RemovalTest(fixtures.TestBase):
         eq_(m3.mock_calls, [call("x")])
         eq_(m4.mock_calls, [call("z")])
 
+    def test_once_doesnt_dereference_listener(self):
+        # test for [ticket:4794]
+
+        Target = self._fixture()
+
+        canary = Mock()
+
+        def go(target, given_id):
+            def anonymous(run_id):
+                canary(run_id, given_id)
+
+            event.listen(target, "event_one", anonymous, once=True)
+
+        t1 = Target()
+
+        assert_calls = []
+        given_ids = []
+        for given_id in range(100):
+            given_ids.append(given_id)
+            go(t1, given_id)
+            if given_id % 10 == 0:
+                t1.dispatch.event_one(given_id)
+                assert_calls.extend(call(given_id, i) for i in given_ids)
+                given_ids[:] = []
+
+        eq_(canary.mock_calls, assert_calls)
+
     def test_propagate(self):
         Target = self._fixture()