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

ThreadPoolExecutorTest.test_no_stale_references may hang with GIL disabled #117688

Closed
colesbury opened this issue Apr 9, 2024 · 0 comments
Closed
Labels
tests Tests in the Lib/test dir topic-free-threading

Comments

@colesbury
Copy link
Contributor

colesbury commented Apr 9, 2024

In #114824, I modified test_no_stale_references so that it passes in the --disable-gil build. Unfortunately, that change was not sufficient and the test may still hang once the GIL is actually disabled.

Relevant code:

my_object = MyObject()
my_object_collected = threading.Event()
my_object_callback = weakref.ref(
my_object, lambda obj: my_object_collected.set())

collected = my_object_collected.wait(timeout=support.SHORT_TIMEOUT)

The problem is due to the combination of two issues:

  • Due to biased reference counting, the destructor for my_object is usually called on the main thread asynchronously (by the eval breaker logic)
  • The destructor may be called somewhere in the implementation of my_object_collected.wait(). The my_object_collected.wait() implementation holds some of the same locks that my_object_collected.set() also needs. This can lead to deadlock if the timing is unlucky: the my_object_collected.set() call from the weakref callback tries to acquire locks already held by the current thread and deadlocks.

Linked PRs

@colesbury colesbury added tests Tests in the Lib/test dir topic-free-threading labels Apr 9, 2024
colesbury added a commit to colesbury/cpython that referenced this issue Apr 9, 2024
colesbury added a commit to colesbury/cpython that referenced this issue Apr 9, 2024
colesbury added a commit to colesbury/cpython that referenced this issue Apr 12, 2024
colesbury added a commit that referenced this issue Apr 15, 2024
…#117720)

Check `my_object_collected.wait()` in a loop to give the main thread a
chance to merge the reference count fields. Additionally, call
`my_object_collected.set()` in a background thread to avoid deadlocking
when the destructor is called asynchronously via the eval breaker
within the body of of `my_object_collected.wait()`.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…sabled (python#117720)

Check `my_object_collected.wait()` in a loop to give the main thread a
chance to merge the reference count fields. Additionally, call
`my_object_collected.set()` in a background thread to avoid deadlocking
when the destructor is called asynchronously via the eval breaker
within the body of of `my_object_collected.wait()`.
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 topic-free-threading
Projects
None yet
Development

No branches or pull requests

1 participant