gh-151364: Add critical sections to kqueue#151376
Conversation
|
The bug report was only on |
| static int | ||
| kqueue_event_init(PyObject *op, PyObject *args, PyObject *kwds) | ||
| { | ||
| int res; | ||
| Py_BEGIN_CRITICAL_SECTION(self); | ||
| res = kqueue_event_init_lock_held(op, args, kwds); | ||
| Py_END_CRITICAL_SECTION(); | ||
| return res; | ||
| } |
There was a problem hiding this comment.
We very rarely add locking to __init__ functions because there are few reasons to support concurrent reinitialization on a live object, and it slows the single-threaded case.
There was a problem hiding this comment.
_io.TextIOWrapper.__init__ was recently modified to add @critical_section (commit db4b194). Was it a bad idea? Or it's just that it should be an exception, and not the general rule?
There was a problem hiding this comment.
I updated the PR to not use a critical section on kqueue_event_init().
| @@ -0,0 +1,2 @@ | |||
| Add critical sections to :func:`select.kqueue` objects. Patch by Victor | |||
There was a problem hiding this comment.
Do users know what "critical sections" mean in this case? It's probably better to say something like "Fix thread-safety issues in select.kqueue objects," because that's what users care more about.
There was a problem hiding this comment.
Good idea, I updated the Changelog entry.
Don't use a critical section on kqueue_event_init().
kqueue.kqfdbetweenclose()and theclosedgetter with free-threading build #151364