gh-144446: Fix thread safety of gi_frame, cr_frame and ag_frame in free-threading - #156037
Conversation
… in free-threading build
…e-144446.k3QzXa.rst
|
ping @mpage for review |
|
This looks correct to me. I'm a little worried about the performance impact of adding a |
|
Benchmark results look like a small regression (~1%). I don't think we need to use an exchange in PyFrameObject *f = FT_ATOMIC_LOAD_PTR_RELAXED(frame->frame_obj);
if (f != NULL) {
FT_ATOMIC_STORE_PTR_RELAXED(frame->frame_obj, NULL);
if (!_PyObject_IsUniquelyReferenced((PyObject *)f)) {
take_ownership(f, frame);
Py_DECREF(f);
return;
}
Py_DECREF(f);
} |
|
|
|
|
This fails on some buildbots with: |
Fixes thread safety of reading
gi_frame,cr_frameandag_framewhile the generator is running or finishing in another thread. Previously the reader could create a frame object for an interpreter frame that was concurrently being cleared, or two threads could each create a frame object for the same frame.The frame object is now created within the generator's critical section, re-checking the frame state after acquiring it so a finished generator returns
Noneinstead of a frame for a cleared iframe. Clearing the generator's frame ingen_clear_frame()andclear_gen_frame()now also holds the critical section so it cannot race with the getter.frame->frame_objis now set with a compare exchange as the running thread can create it without holding the generator's critical section (e.g. throughsys._getframe()) — the losing thread discards its frame object and uses the existing one — and it is cleared with an atomic exchange and read with an acquire load.