GitLab

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).