Skip to content

Commit cad8786

Browse files
committed
Fix memory ordering for the event reset.
1 parent 1623556 commit cad8786

3 files changed

Lines changed: 24 additions & 5 deletions

File tree

Include/internal/pycore_lock.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ PyAPI_FUNC(int) _PyEvent_IsSet(PyEvent *evt);
8787
// Export for '_testinternalcapi' shared extension
8888
PyAPI_FUNC(void) _PyEvent_Notify(PyEvent *evt);
8989

90+
91+
// Reset an event. There must be no active waiters.
92+
void _PyEvent_Reset(PyEvent *evt);
93+
9094
// Wait for the event to be set. If the event is already set, then this returns
9195
// immediately.
9296
PyAPI_FUNC(void) PyEvent_Wait(PyEvent *evt);

Python/lock.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,15 @@ _PyEvent_Notify(PyEvent *evt)
313313
}
314314
}
315315

316+
void
317+
_PyEvent_Reset(PyEvent *evt)
318+
{
319+
assert(evt != NULL);
320+
// It's not safe to reset an event with active waiters.
321+
assert(_Py_atomic_load_uint8(&evt->v) != _Py_HAS_PARKED);
322+
_Py_atomic_store_uint8(&evt->v, _Py_UNLOCKED);
323+
}
324+
316325
void
317326
PyEvent_Wait(PyEvent *evt)
318327
{

Python/pystate.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3347,14 +3347,20 @@ try_acquire_interp_guard(PyInterpreterState *interp, PyInterpreterGuard *guard)
33473347

33483348
Py_ssize_t old_value = _Py_atomic_add_ssize(&interp->finalization_guards.countdown, 1);
33493349
if (old_value == 0) {
3350-
// Reset the event.
33513350
// We first have to notify the finalization thread if it's waiting on us, but
3352-
// it will get trapped waiting on the RW lock. When it goes to check
3353-
// again after we release the lock, it will see that the countdown is
3351+
// it will get trapped waiting on the RW lock, so we don't have to worry about
3352+
// another active waiter while we reset the event.
3353+
_PyEvent_Notify(&interp->finalization_guards.done);
3354+
// Now, when the finalization thread goes to check the countdown
3355+
// after we release the lock, it will see that the countdown is
33543356
// non-zero and begin waiting again (hence why we need to reset the
33553357
// event).
3356-
_PyEvent_Notify(&interp->finalization_guards.done);
3357-
memset(&interp->finalization_guards.done, 0, sizeof(PyEvent));
3358+
// It is worth noting that this event is intentionally a non-atomic check
3359+
// for remaining finalization guards (that is, even if the event is set,
3360+
// it is not necessarily guaranteed that countdown is zero). We use it
3361+
// to simply prevent the finalization from constantly spinning and
3362+
// atomically reading the countdown.
3363+
_PyEvent_Reset(&interp->finalization_guards.done);
33583364
}
33593365
_PyRWMutex_RUnlock(&interp->finalization_guards.lock);
33603366

0 commit comments

Comments
 (0)