From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sat, 3 Aug 2019 03:20:06 +0000 (-0400)
Subject: Strong reference listen function wrapped by "once"
X-Git-Tag: rel_1_3_7~12
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=63ac8595fd0c93edbb323ec42c9ceafa5db4b4a0;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

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
index 0000000000..7ba9e929ee
--- /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 e7c30901ff..260d6e86f1 100644
--- 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 41a20c41d1..a5c1f48034 100644
--- 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 3103b5f066..c12b3414c7 100644
--- 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()