Rodrigo Mesquita pushed to branch wip/romes/27729 at Glasgow Haskell Compiler / GHC Commits: 16112a3c by Rodrigo Mesquita at 2026-08-24T16:02:49+01:00 fixup! rts: Fix race condition in MSG_UPD_TSO_FLAGS execution - - - - - 3 changed files: - rts/Messages.c - rts/Threads.c - rts/Threads.h Changes: ===================================== rts/Messages.c ===================================== @@ -84,27 +84,41 @@ owns, racing with its actual owner mutating it, since it is no longer the owner. The message meant for a TSO should only be executed when the receiving Capability is still the owner of that TSO. Otherwise, it must be forwarded to -the new owner. The pseudo code for handling a message that targets a particular -TSO will look something like: +the new owner. + +The general pattern is one where there's a top-level function which assumes can +be called by capabilities other than the TSO's owner. The function checks +whether the current capability is the TSO owner. If yes, execute the action. If +not, then it sends a message to the current TSO's owner. On receiving the +message, the new capability will just call that top-level function, which will +ensure the message is forwarded again if the TSO owner changed. +It will look something like: + + runMyMsg(Capability *from, StgTSO *target, ...) { + +#if defined(THREADED_RTS) + Capability *owner = RELAXED_LOAD(&target->cap) + if (owner != from) { + MessageMyMsg* msg = ... + sendMessage(cap, owner, msg) + return + } +#endif + + actuallyDoTheWork(...) + } executeMessage(...) { if (i == &stg_MY_MSG_info) { MessageMyMsg* msg = (MessageMyMsg*) m - - Capability *owner = RELAXED_LOAD(&msg->tso->cap); - if (owner != cap) { - sendMessage(cap, owner, (Message *)msg); - return; - } - - actuallyExecute(...) + runMyMsg(cap, m->tso, ...) } } -See `executeMessage`'s `stg_MSG_UPD_TSO_FLAG_info` and +See example `updThreadFlag` and `executeMessage`'s `stg_MSG_UPD_TSO_FLAG_info`, or `stg_MSG_CLONE_STACK_info` for two live examples. */ @@ -185,18 +199,8 @@ loop: else if(i == &stg_MSG_UPD_TSO_FLAG_info){ MessageUpdTSOFlag *u = (MessageUpdTSOFlag*) m; - // We must check that the current owner of the thread is still this capability. - // See Note [TSO owner may change in between Msg being sent and received] - Capability *owner = RELAXED_LOAD(&u->tso->cap); - if (owner != cap) { - sendMessage(cap, owner, (Message *)u); - return; - } - - if (u->set) { u->tso->flags |= u->flag; } - else { u->tso->flags &= ~u->flag; } - - return; + StgTSO *tso = RELAXED_LOAD(&u->tso); + updThreadFlag(cap, tso, u->flag, u->set); } else { ===================================== rts/Threads.c ===================================== @@ -379,23 +379,25 @@ migrateThread (Capability *from, StgTSO *tso, Capability *to) sets or unsets a flag in a given TSO ------------------------------------------------------------------------- */ -static void -updThreadFlag(Capability *from, StgTSO *tso, StgWord32 flag, StgBool set); - void setThreadFlag(Capability *from, StgTSO *tso, StgWord32 flag) { - updThreadFlag(from, tso, flag, 1); + updThreadFlag(from, tso, flag, true); } void unsetThreadFlag(Capability *from, StgTSO *tso, StgWord32 flag) { - updThreadFlag(from, tso, flag, 0); + updThreadFlag(from, tso, flag, false); } -static void -updThreadFlag(Capability *from, StgTSO *tso, StgWord32 flag, StgBool set /* true=set, false=unset */) +void +updThreadFlag(Capability *from USED_IF_THREADS, StgTSO *tso, StgWord32 flag, StgBool set /* true=set, false=unset */) { #if defined(THREADED_RTS) + // If we're the current owner of the thread we want to modify, do it. + // Otherwise, we must forward the message to the actual owner. + // When executing the upd message, we check again that we're still the TSO + // owner (which may have changed since the message was queued on this cap.) + // See Note [TSO owner may change in between Msg being sent and received] Capability *tso_owner = RELAXED_LOAD(&tso->cap); if (from != tso_owner) { MessageUpdTSOFlag *msg; @@ -407,8 +409,6 @@ updThreadFlag(Capability *from, StgTSO *tso, StgWord32 flag, StgBool set /* true sendMessage(from, tso_owner, (Message*)msg); return; } -#else - (void)from; // unused in non-threaded case #endif if (set) { ===================================== rts/Threads.h ===================================== @@ -21,6 +21,7 @@ void migrateThread (Capability *from, StgTSO *tso, Capability *to); void setThreadFlag (Capability *from, StgTSO *tso, StgWord32 flag); void unsetThreadFlag (Capability *from, StgTSO *tso, StgWord32 flag); +void updThreadFlag (Capability *from, StgTSO *tso, StgWord32 flag, StgBool set); // Wakes up a thread on a Capability (probably a different Capability // from the one held by the current Task). View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/16112a3c4be485846bc2146d8527e904... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/16112a3c4be485846bc2146d8527e904... You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications: https://gitlab.haskell.org/-/profile/notifications | Help: https://gitlab.haskell.org/help