gh-135641: Fix flaky test_capi.test_lock_two_threads test case
#135642
Conversation
The mutex may have the _Py_HAS_PARKED bit set in rare cases.
colesbury
added
skip news
needs backport to 3.13
| // gh-135641: in rare cases the lock may still have `_Py_HAS_PARKED` set | ||
| // (m->_bits == 3) due to bucket collisions in the parking lot hash table | ||
| // between this mutex and the `test_data.done` event. |
There was a problem hiding this comment.
Are there any consequences of this being set incorrectly? I think another solution, though much more invasive, might be to move num_waiters from Bucket onto wait_entry.
There was a problem hiding this comment.
Are there any consequences of this being set incorrectly?
The unlock will just take a slower path. The approximate (conservative) setting of _Py_HAS_PARKED is taken from WebKit, so I'm not too worried about it. It's just easy to forget about when writing these sorts of tests.
I think another solution, though much more invasive, might be to move num_waiters from Bucket onto wait_entry.
You can do this nicely and efficiently if you restructure the buckets into a tree of linked lists, which is something we talked about when merging the original parking lot PR, but hasn't been a priority.
There's currently a potential O(N^2) behavior in parking lot due to a linear scan (where N=threads waiting on locks) that using a balanced tree would fix. That's not something I'd want to backport though.
Here's the equivalent fix in Go for their mutex implementation: golang/go@990124d
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
pythongh-135642) The mutex may have the `_Py_HAS_PARKED` bit set. (cherry picked from commit 17ac393) Co-authored-by: Sam Gross <[email protected]>
pythongh-135642) The mutex may have the `_Py_HAS_PARKED` bit set. (cherry picked from commit 17ac393) Co-authored-by: Sam Gross <[email protected]>
|
GH-135687 is a backport of this pull request to the 3.14 branch. |
|
GH-135688 is a backport of this pull request to the 3.13 branch. |
…se (gh-135642) (gh-135688) The mutex may have the `_Py_HAS_PARKED` bit set. (cherry picked from commit 17ac393) Co-authored-by: Sam Gross <[email protected]>
…se (gh-135642) (gh-135687) The mutex may have the `_Py_HAS_PARKED` bit set. (cherry picked from commit 17ac393) Co-authored-by: Sam Gross <[email protected]>
pythongh-135642) The mutex may have the `_Py_HAS_PARKED` bit set.
The mutex may have the
_Py_HAS_PARKEDbit set in rare cases.test_lock_two_threadsflaky intest_capi#135641