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

Race between _PyParkingLot_Park and _PyParkingLot_UnparkAll when handling interrupts #114944

Closed
mpage opened this issue Feb 3, 2024 · 2 comments
Labels
topic-free-threading type-bug An unexpected behavior, bug, or error

Comments

@mpage
Copy link
Contributor

mpage commented Feb 3, 2024

Bug report

Bug description:

There is a potential race when _PyParkingLot_UnparkAll is executing in one thread and another thread is unblocked because of an interrupt in _PyParkingLot_Park. Consider the following scenario:

  1. Thread T0 is blocked in _PyParkingLot_Park on address A.
  2. Thread T1 executes _PyParkingLot_UnparkAll on address A. It finds the wait_entry for T0 and unlinks its list node.
  3. Immediately after (2), T0 is woken up due to an interrupt. It then segfaults trying to unlink the node that was previously unlinked in (2).

I haven't attempted to write a minimal repro for this. It occurs reliably on MacOS on this PR when running ./python.exe -m test test_asyncio.test_events --match test_get_event_loop_new_process.

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

@mpage
Copy link
Contributor Author

mpage commented Feb 3, 2024

Paging Dr. @colesbury

@colesbury
Copy link
Contributor

Ahh, good catch. We handle this case for the single unpark, but not unparkAll.

DinoV pushed a commit that referenced this issue Feb 5, 2024
…nparkAll` when handling interrupts (#114945)

Fix race between `_PyParkingLot_Park` and `_PyParkingLot_UnparkAll` when handling interrupts

There is a potential race when `_PyParkingLot_UnparkAll` is executing in
one thread and another thread is unblocked because of an interrupt in
`_PyParkingLot_Park`. Consider the following scenario:

1. Thread T0 is blocked[^1] in `_PyParkingLot_Park` on address `A`.
2. Thread T1 executes `_PyParkingLot_UnparkAll` on address `A`. It
   finds the `wait_entry` for `T0` and unlinks[^2] its list node.
3. Immediately after (2), T0 is woken up due to an interrupt. It
   then segfaults trying to unlink[^3] the node that was previously
   unlinked in (2).

To fix this we mark each waiter as unparking before releasing the bucket
lock. `_PyParkingLot_Park` will wait to handle the coming wakeup, and not
attempt to unlink the node, when this field is set. `_PyParkingLot_Unpark`
does this already, presumably to handle this case.
@mpage mpage closed this as completed Feb 5, 2024
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this issue Feb 14, 2024
…gLot_UnparkAll` when handling interrupts (python#114945)

Fix race between `_PyParkingLot_Park` and `_PyParkingLot_UnparkAll` when handling interrupts

There is a potential race when `_PyParkingLot_UnparkAll` is executing in
one thread and another thread is unblocked because of an interrupt in
`_PyParkingLot_Park`. Consider the following scenario:

1. Thread T0 is blocked[^1] in `_PyParkingLot_Park` on address `A`.
2. Thread T1 executes `_PyParkingLot_UnparkAll` on address `A`. It
   finds the `wait_entry` for `T0` and unlinks[^2] its list node.
3. Immediately after (2), T0 is woken up due to an interrupt. It
   then segfaults trying to unlink[^3] the node that was previously
   unlinked in (2).

To fix this we mark each waiter as unparking before releasing the bucket
lock. `_PyParkingLot_Park` will wait to handle the coming wakeup, and not
attempt to unlink the node, when this field is set. `_PyParkingLot_Unpark`
does this already, presumably to handle this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-free-threading type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants