GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||
GeezerVison is showing github.com — gh-116738: Add locks for FT build in signalmodule by johng · Pull Request #161221 · python/cpython Conversation
|
|
CC @ngoldbaum if you have bandwdith, thanks! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I think this needs a different approach. First, this doesn’t protect all accesses to the handler pointers, so it’s incomplete. It also doesn’t protect all the state the GIL previously serialized access to, so it’s incomplete from that perspective. Finally, there are no tests and fixes really should come with tests that run quickly and are deterministically broken before the fixes are applied. Second, a mutex cannot protect every read of signal state. PyErr_SetInterruptEx() may run inside a C signal handler, where acquiring a mutex is unsafe. Correctly implementing signal handlers and handling signal safety is incredibly difficult, see https://iafisher.com/2026/08/sigprint and other posts in that series for examples. That means we’d need to come up with some other signal-safe approach anyway. A lock-free design could cover the path that must be signal-safe as well as reads that need the handler object. Here is an outline of a design: signal.signal() could update an atomic flag indicating whether a Python handler is installed. PyErr_SetInterruptEx() could read that flag without touching a Python object. For signal.getsignal() and Python signal dispatch, each reader could mark a read in progress before loading an atomic handler pointer and taking a strong reference. After replacing the pointer, signal.signal() could keep the old handler alive until earlier reads finish. You’d need to add bookkeeping to track live reads and add or expand tests for the interactions between threads and signals to stress-test the bookkeeping. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.