gh-149816: #96 Fix a race condition in invoke_gc_callback with free threading

[dzaramelcone]

The optimization that avoided allocating an info dict when no gc callbacks were registered introduces a time of check to time of use race in free threaded mode (lineno 1943-1955). I just pulled out the check. Since info is always non-null, I also replaced XDECREF use with DECREF. (lineno 1959, lineno 1982).

I implemented a stress test for this and uncovered what I believe to be a bug with _PyList_GetItemRef. I am following suit with python/cpython#149909 and not including the test with my PR for two reasons:

1. My implementation was resource intensive and not deterministic.
2. I didn't want to scope creep fixing the bug with _PyList_GetItemRef (but I'm happy to open an issue for it!)

also, this is my first PR against CPython, I am doing it from PyCon US as part of the sprint! :) Hello world!

• Issue: 22 free-threading race conditions #149816

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[encukou]

Could you add the test, since you already wrote it? It should be deterministic if it the bug if fixed, ritght?

If it's resource-intensive, you can mark is with @support.requires_resource('cpu').

[dzaramelcone]

Could you add the test, since you already wrote it? It should be deterministic if it the bug if fixed, ritght?

If it's resource-intensive, you can mark is with @support.requires_resource('cpu').

Thank you for the review Petr!

That test is flaky and relies on basically random permutations of thread ordering to reproduce it.

however, now that I think of it, the original issue included a test that reliably reproduced the issue such that TSAN would flag it, so I will just include that one.

[dzaramelcone]

Could you add the test, since you already wrote it? It should be deterministic if it the bug if fixed, ritght?

If it's resource-intensive, you can mark is with @support.requires_resource('cpu').

@encukou added!

[encukou]

Thank you!

Did you consider if (PyList_GET_SIZE(gcstate->callbacks) == 0) { return; } just after the PyList_CheckExact?
The optimization seems worth keeping: the callbacks list will commonly be empty. Tuple & string creation are fast, but still much heavier than checking the size.

[dzaramelcone]

Thank you!

Did you consider if (PyList_GET_SIZE(gcstate->callbacks) == 0) { return; } just after the PyList_CheckExact? The optimization seems worth keeping: the callbacks list will commonly be empty. Tuple & string creation are fast, but still much heavier than checking the size.

Hey Petr! Sorry for the delay.

That looks like it should be fine to me, so I added the early return.

Thank you for the review! Hope you are well!