From ce6feb72790c15977e40fb9e92bbc7237c8ee485 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Tue, 15 Jul 2025 10:41:35 -0700 Subject: [PATCH] event: ensure acquire release semantics for muxnote disposal While it is sufficient to use the relaxed ordering for the acquire semantics in the `_dispatch_muxnote_retain`, we need to use the acquire release semantics on the release in `_dispatch_muxnote_release` to ensure that any pending retains are not interrupted. This should hopefully alleviate the occasional crashes that have been observed with the muxnote reference counting. Fixes: #887, #844 --- src/event/event_windows.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/event/event_windows.c b/src/event/event_windows.c index 94674a3bf..af67dfd4c 100644 --- a/src/event/event_windows.c +++ b/src/event/event_windows.c @@ -260,8 +260,16 @@ _dispatch_muxnote_retain(dispatch_muxnote_t dmn) static void _dispatch_muxnote_release(dispatch_muxnote_t dmn) { - uintptr_t refcount = os_atomic_dec(&dmn->dmn_refcount, relaxed); + // We perform a minor optimization here - perform the decrement with + // release semantics. In the case that we are going to dispose of the + // value, we perform the acquire fence. This reduces the cost on the + // normal path by avoiding the acquire fence. This should be more + // beneficial on ARM64, as X64 being TSO'ed doesn't gain much. However, + // `mfence` being isolated should hopefully be a bit more efficient than + // the repeated `lock` if there is contention. + uintptr_t refcount = os_atomic_dec(&dmn->dmn_refcount, release); if (refcount == 0) { + os_atomic_thread_fence(acquire); _dispatch_muxnote_dispose(dmn); } else if (refcount == UINTPTR_MAX) { DISPATCH_INTERNAL_CRASH(0, "muxnote refcount underflow");