Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_concurrent_futures: test_free_reference is flaky #130957

Open
colesbury opened this issue Mar 7, 2025 · 1 comment
Open

test_concurrent_futures: test_free_reference is flaky #130957

colesbury opened this issue Mar 7, 2025 · 1 comment
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@colesbury
Copy link
Contributor

colesbury commented Mar 7, 2025

Bug report

Seen in https://buildbot.python.org/#/builders/1368/builds/2775/steps/6/logs/stdio

The test attempts to ensure that the executor doesn't hold on to the result object, but the executor necessarily holds onto it for a brief period from when it sets the result in in the future until the variable goes out of scope.

We should use support.sleeping_retry to check if the weakref eventually (ideally quickly) becomes dead.

======================================================================
FAIL: test_free_reference (test.test_concurrent_futures.test_thread_pool.ThreadPoolExecutorTest.test_free_reference)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ec2-user/buildbot/buildarea/3.x.itamaro-macos-arm64-aws.macos-with-brew.refleak.nogil/build/Lib/test/test_concurrent_futures/executor.py", line 132, in test_free_reference
    self.assertIsNone(wr())
    ~~~~~~~~~~~~~~~~~^^^^^^
AssertionError: <test.test_concurrent_futures.executor.MyObject object at 0x200041b0100> is not None

----------------------------------------------------------------------

def test_free_reference(self):
# Issue #14406: Result iterator should not keep an internal
# reference to result objects.
for obj in self.executor.map(make_dummy_object, range(10)):
wr = weakref.ref(obj)
del obj
support.gc_collect() # For PyPy or other GCs.
self.assertIsNone(wr())

try:
result = ctx.run(self.task)
except BaseException as exc:
self.future.set_exception(exc)
# Break a reference cycle with the exception 'exc'
self = None
else:
self.future.set_result(result)

Linked PRs

@colesbury colesbury added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Mar 7, 2025
@colesbury
Copy link
Contributor Author

colesbury commented Mar 7, 2025

You can more easily reproduce this failure by adding a time.sleep():

diff --git a/Lib/concurrent/futures/thread.py b/Lib/concurrent/futures/thread.py
index 909359b6487..73cce87b5dd 100644
--- a/Lib/concurrent/futures/thread.py
+++ b/Lib/concurrent/futures/thread.py
@@ -90,6 +90,7 @@ def run(self, ctx):
             self = None
         else:
             self.future.set_result(result)
+            time.sleep(1)

     __class_getitem__ = classmethod(types.GenericAlias)

colesbury added a commit to colesbury/cpython that referenced this issue Mar 7, 2025
The weak reference may not be immediately dead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant