]> granicus.if.org Git - python/commitdiff
Fix a c.f.as_completed() refleak previously introduced in bpo-27144 (#3270)
authorAntoine Pitrou <pitrou@free.fr>
Sun, 3 Sep 2017 13:09:23 +0000 (15:09 +0200)
committerGitHub <noreply@github.com>
Sun, 3 Sep 2017 13:09:23 +0000 (15:09 +0200)
Lib/concurrent/futures/_base.py
Lib/test/test_concurrent_futures.py

index 88521ae317e53df1cfc2d997941f016d30ef2e5e..70c7b6195931408e73aad3beae0c83f18f5dbf0a 100644 (file)
@@ -171,15 +171,24 @@ def _create_and_install_waiters(fs, return_when):
     return waiter
 
 
-def _yield_and_decref(fs, ref_collect):
+def _yield_finished_futures(fs, waiter, ref_collect):
     """
-    Iterate on the list *fs*, yielding objects one by one in reverse order.
-    Before yielding an object, it is removed from each set in
-    the collection of sets *ref_collect*.
+    Iterate on the list *fs*, yielding finished futures one by one in
+    reverse order.
+    Before yielding a future, *waiter* is removed from its waiters
+    and the future is removed from each set in the collection of sets
+    *ref_collect*.
+
+    The aim of this function is to avoid keeping stale references after
+    the future is yielded and before the iterator resumes.
     """
     while fs:
+        f = fs[-1]
         for futures_set in ref_collect:
-            futures_set.remove(fs[-1])
+            futures_set.remove(f)
+        with f._condition:
+            f._waiters.remove(waiter)
+        del f
         # Careful not to keep a reference to the popped value
         yield fs.pop()
 
@@ -216,7 +225,8 @@ def as_completed(fs, timeout=None):
         waiter = _create_and_install_waiters(fs, _AS_COMPLETED)
     finished = list(finished)
     try:
-        yield from _yield_and_decref(finished, ref_collect=(fs,))
+        yield from _yield_finished_futures(finished, waiter,
+                                           ref_collect=(fs,))
 
         while pending:
             if timeout is None:
@@ -237,9 +247,11 @@ def as_completed(fs, timeout=None):
 
             # reverse to keep finishing order
             finished.reverse()
-            yield from _yield_and_decref(finished, ref_collect=(fs, pending))
+            yield from _yield_finished_futures(finished, waiter,
+                                               ref_collect=(fs, pending))
 
     finally:
+        # Remove waiter from unfinished futures
         for f in fs:
             with f._condition:
                 f._waiters.remove(waiter)
index f1226fe7090959ab6db57155c9b71e9e901a1fe7..03f8d1d711227df2cc1dc5f0b722fd9997a38f7a 100644 (file)
@@ -405,7 +405,7 @@ class AsCompletedTests:
         # to finished futures.
         futures_list = [Future() for _ in range(8)]
         futures_list.append(create_future(state=CANCELLED_AND_NOTIFIED))
-        futures_list.append(create_future(state=SUCCESSFUL_FUTURE))
+        futures_list.append(create_future(state=FINISHED, result=42))
 
         with self.assertRaises(futures.TimeoutError):
             for future in futures.as_completed(futures_list, timeout=0):