Skip to content

test_code.test_free_different_thread is flaky when the GIL is disabled #117683

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

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

This test case checks that freeing a code object on a different thread then where the co_extra was set is safe. However, the test make some assumptions about when destructors are called that aren't always true in the free-threaded build:

@threading_helper.requires_working_threading()
def test_free_different_thread(self):
# Freeing a code object on a different thread then
# where the co_extra was set should be safe.
f = self.get_func()
class ThreadTest(threading.Thread):
def __init__(self, f, test):
super().__init__()
self.f = f
self.test = test
def run(self):
del self.f
gc_collect()
self.test.assertEqual(LAST_FREED, 500)
SetExtra(f.__code__, FREE_INDEX, ctypes.c_voidp(500))
tt = ThreadTest(f, self)
del f
tt.start()
tt.join()
self.assertEqual(LAST_FREED, 500)

In particular, the assertion self.test.assertEqual(LAST_FREED, 500) in the ThreadTest occasionally fails because LAST_FREED is still None. The underlying issue is that biased reference counting can delay the calling of the code object's destructor.

Normally, the gc_collect() calls are sufficient to ensure that the code object is collected. They sort of are -- the code object is being freed -- but it happens concurrently in the main thread and may not be finished by the time ThreadTest calls the self.test.assertEqual(LAST_FREED, 500).

The timeline I've seen when debugging this is:

  1. The main thread starts ThreadTest
  2. ThreadTest deletes the final reference to f. The total reference count is now zero, but it's represented as ob_ref_local=1, ob_ref_shared=-1, so TestThread enqueues it to be merged by the main thread.
  3. The main thread merges the reference count fields and starts to call the code object's destructor
  4. ThreadTest calls gc_collect() and then self.test.assertEqual(LAST_FREED, 500), which fails
    ...
  5. The main thread finishes calling the code object's destructor, which sets LAST_FREED to 500.

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 9, 2024
colesbury added a commit to colesbury/cpython that referenced this issue Apr 12, 2024
colesbury added a commit to colesbury/cpython that referenced this issue Apr 15, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
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