Duncan Coutts pushed to branch wip/dcoutts/issue-26717 at Glasgow Haskell Compiler / GHC Commits: 3ea1f237 by Duncan Coutts at 2026-07-21T13:16:35+01:00 Fixup name of old .async_result, it's now async_reqID That bit changed in master after the MR was first written. TODO: fixup into previous commit after code review - - - - - c43b9929 by Duncan Coutts at 2026-07-21T13:17:30+01:00 Define constants for the existing stg_threadStatuszh return codes The stg_threadStatuszh reuses the internal tso->why_blocked codes but also extends them with a couple previously magic values. This is awkward since we need to know what those magic values are so we don't accidentally use those values to mean something else. By pulling a definition up to where the why_blocked codes are defined we will be able to avoid mistakenly assining those codes some meaning (or just changing the BlockedThreadComplete, BlockedThreadKilled code if necessary). - - - - - 4aca39e4 by Duncan Coutts at 2026-07-21T13:20:52+01:00 Extend the tso->why_blocked encoding to indicate block_info closures We use some bit tricks to cheaply and generically test if a tso->why_blocked tag implies that the corresponding tso->block_info will contain a non-trivial valid closure (i.e. not just block_info.unused set to END_TSO_QUEUE). In particular we arrange for most why_blocked values to naturally have a distinguishing bit, but for the BlockedOn{Read,Write,Delay} cases, they can come in either non-closure or closure forms. We allow an additional bit to distinguish these cases. The non-closure forms are only from legacy I/O managers: select and win32-legacy. So this extra bit mechanism will be able to be retired once the legacy I/O managers are themselves retired. This means in a few places we need to untag the why_blocked value before inspecting it, but in most places we do not. - - - - - 9c55e3c2 by Duncan Coutts at 2026-07-21T13:20:55+01:00 Use BlockInfoForceNonClosure in the select I/O manager - - - - - e4e4a32c by Duncan Coutts at 2026-07-21T13:20:55+01:00 Use BlockInfoForceNonClosure in the win32-legacy I/O manager for the BlockedOn{Read,Write} since these use the non-heap allocated StgAsyncIOResult. - - - - - eeb795a2 by Duncan Coutts at 2026-07-21T13:20:55+01:00 Enforce the why_blocked and block_info rules in checkTSO We now check the cases wher IsBlockInfoClosure should hold, the cases that are supposed to use block_info.unused == END_TSO_QUEUE, and which cases are allowed to use BlockInfoForceNonClosure. This partially enforces the use of why_blocked as a tag for the block_info union. We could be stricter and check for the correct expected info table for the closure cases. - - - - - 37cfcd73 by Duncan Coutts at 2026-07-21T13:20:55+01:00 Use IsBlockInfoClosure to simplify several tests In GC and generic traversal we need to know if we should look at the block_info.closure or not. Now we can do just that using a cheap bit test on the why_blocked tag. This fixes issue 26717, where the problem was that some GC modes did not know when to look at block_info.closure, because the poll I/O manager uses a closure for BlockedOn{Read,Write} while the select I/O manager uses a non-closure. Now this information is in the why_blocked tag itself. - - - - - 0afedcf5 by Duncan Coutts at 2026-07-21T13:20:55+01:00 Remove the now-unused scavengeTSOIOManager The GC no longer has to delegate to the I/O manager, since it can use IsBlockInfoClosure to decide things itself. - - - - - 51b04410 by Duncan Coutts at 2026-07-21T13:20:55+01:00 Remove duplicate assertion - - - - - a553a482 by Duncan Coutts at 2026-07-21T13:20:55+01:00 Follow atomic access rules more consistently for tso->why_blocked The rule is this: store block_info *before* why_blocked store why_blocked using store release load why_blocked using load acquire load block_info *after* why_blocked This is a an atomic store release / load acquire pair and (if the reads are in a separate thread to the writes, and the read receives the value stored) then this guarantees a full "happens before" relationship of these stores and loads. In some cases, we do not need a full load acquire, because we don't read the block_info at all and so do not need any ordering. In this case we just need an atomic relaxed load. This was being followed in most places, but not all. If there's good reason in any case that we don't need atomic access, then we should document that in a comment. In the absence of that I think it's easier to follow the rule everywhere. - - - - - 6fa2d9be by Duncan Coutts at 2026-07-21T13:20:55+01:00 Add a changelog entry - - - - - 573b54eb by Duncan Coutts at 2026-07-21T13:20:55+01:00 Fix two non-atomic uses of ->why_blocked spotted by Cheng Shao during code review - - - - - 40dc6744 by Duncan Coutts at 2026-07-21T13:20:55+01:00 Fix the category of BlockedOnDoProc in checkTSO It is not in the category where block_info.unused is set, but in the category where the thing is not a closure at all. It is the odd-one-out by being unconditional, so does not use BlockInfoForceNonClosure. TODO: squash into commit: "Enforce the why_blocked and block_info rules in checkTSO" - - - - - 20 changed files: - + changelog.d/T26716 - rts/IOManager.c - rts/IOManager.h - rts/Messages.c - rts/PrimOps.cmm - rts/RaiseAsync.c - rts/RaiseAsync.h - rts/Schedule.c - rts/Threads.c - rts/TraverseHeap.c - rts/include/rts/Constants.h - rts/include/rts/storage/TSO.h - rts/posix/Poll.c - rts/posix/Select.c - rts/posix/Timeout.c - rts/sm/Compact.c - rts/sm/NonMovingMark.c - rts/sm/Sanity.c - rts/sm/Scav.c - rts/win32/AsyncMIO.c Changes: ===================================== changelog.d/T26716 ===================================== @@ -0,0 +1,15 @@ +section: rts +synopsis: Fix a use-after-free bug in the poll I/O manager +issues: #26716 #26717 +mrs: !15519 +description: { + Experimental work on ASAN support for GHC (MR !15168) revealed a + use-after-free bug when using the combination of the new poll I/O + manager with the compacting GC. The ultimate cause is that a TSO's + `block_info` (used by I/O managers and many other parts of the RTS) + is sometimes a GC pointer and sometimes not, but without a consistent + and easy-to-follow rule for when this is the case. The solution has + been to clean up and enforce that the TSO's `why_blocked` enumeration + is a proper tag for the `block_info`, and to use an encoding that + determines precisely when the `block_info` is a pointer or not. +} ===================================== rts/IOManager.c ===================================== @@ -589,41 +589,6 @@ void markCapabilityIOManager(evac_fn evac, void *user, CapIOManager *iomgr) } -void scavengeTSOIOManager(StgTSO *tso) -{ - switch (iomgr_type) { - - /* case IO_MANAGER_SELECT: - * BlockedOn{Read,Write} uses block_info.fd - * BlockedOnDelay uses block_info.target - * both of these are not GC pointers, so there is nothing to do. - */ - -#if defined(IOMGR_ENABLED_POLL) - case IO_MANAGER_POLL: - /* BlockedOn{Read,Write} uses block_info.aiop - * BlockedOnDelay uses block_info.timeout - * both of these are heap allocated, so we can do the same in all - * cases, which is why we can use the generic block_info.closure. - */ - evacuate(&tso->block_info.closure); - break; -#endif - - /* case IO_MANAGER_WIN32_LEGACY: - * BlockedOn{Read,Write,DoProc} uses block_info.async_reqID - * which is a plain integer, so nothing to scavenge. - */ - - default: - /* All the other I/O managers do not use I/O-related why_blocked - * reasons, so there are no cases to handle. - */ - break; - } -} - - /* Declared in rts/IOInterface.h. Used only by the MIO threaded I/O manager on * Unix platforms. */ @@ -824,16 +789,17 @@ bool syncIOWaitReady(CapIOManager *iomgr, #if defined(IOMGR_ENABLED_SELECT) case IO_MANAGER_SELECT: { - StgWord why_blocked = rw == IORead ? BlockedOnRead : BlockedOnWrite; + StgThreadWhyBlocked why_blocked = (rw == IORead ? BlockedOnRead + : BlockedOnWrite) + | BlockInfoForceNonClosure; tso->block_info.fd = fd; - RELEASE_STORE(&tso->why_blocked, why_blocked); appendToIOBlockedQueue(iomgr, tso); + RELEASE_STORE(&tso->why_blocked, why_blocked); return true; } #endif #if defined(IOMGR_ENABLED_POLL) case IO_MANAGER_POLL: - ASSERT(tso->why_blocked == NotBlocked); return syncIOWaitReadyPoll(iomgr, tso, rw, fd); #endif default: @@ -890,8 +856,8 @@ bool syncDelay(CapIOManager *iomgr, StgTSO *tso, HsInt us_delay) { LowResTime target = getDelayTarget(us_delay); tso->block_info.target = target; - RELEASE_STORE(&tso->why_blocked, BlockedOnDelay); insertIntoSleepingQueue(iomgr, tso, target); + RELEASE_STORE(&tso->why_blocked, BlockedOnDelay | BlockInfoForceNonClosure); return true; } #endif @@ -911,8 +877,8 @@ bool syncDelay(CapIOManager *iomgr, StgTSO *tso, HsInt us_delay) * simplifies matters, so set the status to OnDoProc and put the * delayed thread on the blocked_queue. */ - RELEASE_STORE(&tso->why_blocked, BlockedOnDoProc); appendToIOBlockedQueue(iomgr, tso); + RELEASE_STORE(&tso->why_blocked, BlockedOnDoProc); return true; } #endif @@ -928,6 +894,7 @@ void syncDelayCancel(CapIOManager *iomgr, StgTSO *tso) switch (iomgr_type) { #if defined(IOMGR_ENABLED_SELECT) case IO_MANAGER_SELECT: + ASSERT(tso->why_blocked == (BlockedOnDelay | BlockInfoForceNonClosure)); removeThreadFromQueue(iomgr->cap, &iomgr->sleeping_queue, tso); break; #endif ===================================== rts/IOManager.h ===================================== @@ -311,11 +311,6 @@ void exitIOManager(bool wait_threads); void markCapabilityIOManager(evac_fn evac, void *user, CapIOManager *iomgr); -/* GC hook: scavenge I/O related tso->block_info. Used by scavengeTSO. - */ -void scavengeTSOIOManager(StgTSO *tso); - - /* Several code paths are almost identical between read and write paths. In * such cases we use a shared code path with an enum to say which we're doing. */ ===================================== rts/Messages.c ===================================== @@ -267,7 +267,8 @@ uint32_t messageBlackHole(Capability *cap, MessageBlackHole *msg) // NB. we check to make sure that the owner is not the same as // the current thread, since in that case it will not be on // the run queue. - if (owner->why_blocked == NotBlocked && owner->id != msg->tso->id) { + if (RELAXED_LOAD(&owner->why_blocked) == NotBlocked && + owner->id != msg->tso->id) { promoteInRunQueue(cap, owner); } @@ -328,7 +329,8 @@ uint32_t messageBlackHole(Capability *cap, MessageBlackHole *msg) msg->tso->id, owner->id); // See above, #3838 - if (owner->why_blocked == NotBlocked && owner->id != msg->tso->id) { + if (RELAXED_LOAD(&owner->why_blocked) == NotBlocked && + owner->id != msg->tso->id) { promoteInRunQueue(cap, owner); } ===================================== rts/PrimOps.cmm ===================================== @@ -1145,12 +1145,12 @@ stg_threadStatuszh ( gcptr tso ) // contents of block_info too, then we'd have to do some synchronisation. if (what_next == ThreadComplete) { - ret = 16; // NB. magic, matches up with GHC.Conc.threadStatus + ret = BlockedThreadComplete; // NB. magic, matches up with GHC.Conc.threadStatus } else { if (what_next == ThreadKilled) { - ret = 17; + ret = BlockedThreadKilled; } else { - ret = why_blocked; + ret = UntagWhyBlocked(why_blocked); } } @@ -2319,7 +2319,8 @@ stg_asyncReadzh ( W_ fd, W_ is_sock, W_ len, W_ buf ) StgTSO_block_info(CurrentTSO) = reqID; ASSERT(StgTSO_why_blocked(CurrentTSO) == NotBlocked::I32); - %release StgTSO_why_blocked(CurrentTSO) = BlockedOnRead::I32; + %release StgTSO_why_blocked(CurrentTSO) = BlockedOnRead::I32 + | BlockInfoForceNonClosure::I32; ccall appendToIOBlockedQueue(Capability_iomgr(MyCapability()) "ptr", CurrentTSO "ptr"); @@ -2339,7 +2340,8 @@ stg_asyncWritezh ( W_ fd, W_ is_sock, W_ len, W_ buf ) StgTSO_block_info(CurrentTSO) = reqID; ASSERT(StgTSO_why_blocked(CurrentTSO) == NotBlocked::I32); - %release StgTSO_why_blocked(CurrentTSO) = BlockedOnWrite::I32; + %release StgTSO_why_blocked(CurrentTSO) = BlockedOnWrite::I32 + | BlockInfoForceNonClosure::I32; ccall appendToIOBlockedQueue(Capability_iomgr(MyCapability()) "ptr", CurrentTSO "ptr"); ===================================== rts/RaiseAsync.c ===================================== @@ -233,7 +233,6 @@ throwTo (Capability *cap, // the Capability we hold uint32_t throwToMsg (Capability *cap, MessageThrowTo *msg) { - StgWord status; StgTSO *target = ACQUIRE_LOAD(&msg->target); Capability *target_cap; @@ -268,9 +267,9 @@ check_target: return THROWTO_BLOCKED; } - status = ACQUIRE_LOAD(&target->why_blocked); + StgThreadWhyBlocked why_blocked = ACQUIRE_LOAD(&target->why_blocked); - switch (status) { + switch (UntagWhyBlocked(why_blocked)) { case NotBlocked: { if ((target->flags & TSO_BLOCKEX) == 0) { @@ -370,8 +369,9 @@ check_target: // we have the MVar, let's check whether the thread // is still blocked on the same MVar. - if ((target->why_blocked != BlockedOnMVar - && target->why_blocked != BlockedOnMVarRead) + StgThreadWhyBlocked why_blocked_still = ACQUIRE_LOAD(&target->why_blocked); + if (( why_blocked_still != BlockedOnMVar + && why_blocked_still != BlockedOnMVarRead) || target->block_info.mvar != mvar) { unlockClosure((StgClosure *)mvar, info); goto retry; @@ -490,7 +490,7 @@ check_target: goto retry; default: - barf("throwTo: unrecognised why_blocked (%d)", target->why_blocked); + barf("throwTo: unrecognised why_blocked (%d)", why_blocked); } barf("throwTo"); } @@ -667,7 +667,7 @@ removeFromMVarBlockedQueue (StgTSO *tso) static void removeFromQueues(Capability *cap, StgTSO *tso) { - switch (tso->why_blocked) { + switch (UntagWhyBlocked(ACQUIRE_LOAD(&tso->why_blocked))) { case NotBlocked: case ThreadMigrating: @@ -721,8 +721,8 @@ removeFromQueues(Capability *cap, StgTSO *tso) } done: - RELAXED_STORE(&tso->why_blocked, NotBlocked); appendToRunQueue(cap, tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); } /* ----------------------------------------------------------------------------- @@ -1105,9 +1105,9 @@ done: IF_DEBUG(sanity, checkTSO(tso)); // wake it up - if (tso->why_blocked != NotBlocked) { - tso->why_blocked = NotBlocked; + if (RELAXED_LOAD(&tso->why_blocked) != NotBlocked) { appendToRunQueue(cap,tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); } return tso; ===================================== rts/RaiseAsync.h ===================================== @@ -56,7 +56,7 @@ void awakenBlockedExceptionQueue (Capability *cap, StgTSO *tso); INLINE_HEADER int interruptible(StgTSO *t) { - switch (t->why_blocked) { + switch (UntagWhyBlocked(t->why_blocked)) { case BlockedOnMVar: case BlockedOnSTM: case BlockedOnMVarRead: ===================================== rts/Schedule.c ===================================== @@ -181,7 +181,7 @@ static void truncateRunQueue(Capability *cap); static StgTSO *popRunQueue (Capability *cap); static inline EventThreadStatus eventlogThreadStatus(StgThreadReturnCode ret_code); -static inline EventThreadStatus eventlogThreadStatusBlocked(StgWord why_blocked); +static inline EventThreadStatus eventlogThreadStatusBlocked(StgThreadWhyBlocked why_blocked); /* --------------------------------------------------------------------------- Main scheduling loop. @@ -531,7 +531,7 @@ run_thread: #endif if (ret == ThreadBlocked) { - uint16_t why_blocked = ACQUIRE_LOAD(&t->why_blocked); + StgThreadWhyBlocked why_blocked = ACQUIRE_LOAD(&t->why_blocked); EventThreadStatus status = eventlogThreadStatusBlocked(why_blocked); StgWord32 status_detail = 0; if (why_blocked == BlockedOnBlackHole) { @@ -1074,7 +1074,7 @@ schedulePostRunThread (Capability *cap, StgTSO *t) // // and a is never equal to b given a consistent view of memory. // - if (t -> trec != NO_TREC && t -> why_blocked == NotBlocked) { + if (t -> trec != NO_TREC && RELAXED_LOAD(&t->why_blocked) == NotBlocked) { if (!stmValidateNestOfTransactions(cap, t -> trec, true)) { debugTrace(DEBUG_sched | DEBUG_stm, "trec %p found wasting its time", t); @@ -2508,9 +2508,9 @@ suspendThread (StgRegTable *reg, bool interruptible) tso->block_info.unused = END_TSO_QUEUE; if (interruptible) { - tso->why_blocked = BlockedOnCCall_Interruptible; + RELEASE_STORE(&tso->why_blocked, BlockedOnCCall_Interruptible); } else { - tso->why_blocked = BlockedOnCCall; + RELEASE_STORE(&tso->why_blocked, BlockedOnCCall); } // Hand back capability @@ -2568,16 +2568,25 @@ resumeThread (void *task_) tso = incall->suspended_tso; incall->suspended_tso = NULL; incall->suspended_cap = NULL; + + // we set why_blocked previously in suspendThread + ASSERT(tso->why_blocked == BlockedOnCCall || + tso->why_blocked == BlockedOnCCall_Interruptible); + // we will modify tso->_link IF_NONMOVING_WRITE_BARRIER_ENABLED { updateRemembSetPushClosure(cap, (StgClosure *)tso->_link); } tso->_link = END_TSO_QUEUE; + // but no need to modify tso->block_info.prev as coincidentally + // it has the value we want already (since in suspendThread we set + // tso->block_info.unused to END_TSO_QUEUE for BlockedOnCCall). + ASSERT(tso->block_info.prev == END_TSO_QUEUE); traceEventRunThread(cap, tso); /* Reset blocking status */ - tso->why_blocked = NotBlocked; + RELEASE_STORE(&tso->why_blocked, NotBlocked); if ((tso->flags & TSO_BLOCKEX) == 0) { // avoid locking the TSO if we don't have to @@ -2950,8 +2959,9 @@ deleteThread (StgTSO *tso) // The TSO must be on the run queue of the Capability we own, or // we must own all Capabilities. - if (tso->why_blocked != BlockedOnCCall && - tso->why_blocked != BlockedOnCCall_Interruptible) { + StgThreadWhyBlocked why_blocked = RELAXED_LOAD(&tso->why_blocked); + if (why_blocked != BlockedOnCCall && + why_blocked != BlockedOnCCall_Interruptible) { throwToSingleThreaded(tso->cap,tso,NULL); } } @@ -2962,10 +2972,12 @@ deleteThread_(StgTSO *tso) { // for forkProcess only: // like deleteThread(), but we delete threads in foreign calls, too. - if (tso->why_blocked == BlockedOnCCall || - tso->why_blocked == BlockedOnCCall_Interruptible) { + StgThreadWhyBlocked why_blocked = RELAXED_LOAD(&tso->why_blocked); + if (why_blocked == BlockedOnCCall || + why_blocked == BlockedOnCCall_Interruptible) { tso->what_next = ThreadKilled; appendToRunQueue(tso->cap, tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); } else { deleteThread(tso); } @@ -3355,7 +3367,7 @@ resurrectThreads (StgTSO *threads) // Wake up the thread on the Capability it was last on cap = tso->cap; - switch (tso->why_blocked) { + switch (UntagWhyBlocked(RELAXED_LOAD(&tso->why_blocked))) { case BlockedOnMVar: case BlockedOnMVarRead: /* Called by GC - sched_mutex lock is currently held. */ @@ -3427,7 +3439,7 @@ static inline EventThreadStatus eventlogThreadStatus(StgThreadReturnCode ret_cod return thread_stop_code[ret_code]; } -static inline EventThreadStatus eventlogThreadStatusBlocked(StgWord why_blocked) +static inline EventThreadStatus eventlogThreadStatusBlocked(StgThreadWhyBlocked why_blocked) { - return thread_blocked_code[why_blocked]; + return thread_blocked_code[UntagWhyBlocked(why_blocked)]; } ===================================== rts/Threads.c ===================================== @@ -291,7 +291,7 @@ tryWakeupThread (Capability *cap, StgTSO *tso) } #endif - switch (ACQUIRE_LOAD(&tso->why_blocked)) + switch (UntagWhyBlocked(ACQUIRE_LOAD(&tso->why_blocked))) { case BlockedOnMVar: case BlockedOnMVarRead: @@ -335,8 +335,8 @@ tryWakeupThread (Capability *cap, StgTSO *tso) unblock: // just run the thread now, if the BH is not really available, // we'll block again. - tso->why_blocked = NotBlocked; appendToRunQueue(cap,tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); // We used to set the context switch flag here, which would // trigger a context switch a short time in the future (at the end @@ -368,7 +368,7 @@ migrateThread (Capability *from, StgTSO *tso, Capability *to) // ThreadMigrating tells the target cap that it needs to be added to // the run queue when it receives the MSG_TRY_WAKEUP. tso->block_info.unused = END_TSO_QUEUE; - tso->why_blocked = ThreadMigrating; + RELEASE_STORE(&tso->why_blocked, ThreadMigrating); tso->cap = to; tryWakeupThread(from, tso); } @@ -876,7 +876,7 @@ loop: // save why_blocked here, because waking up the thread destroys // this information - StgWord why_blocked = ACQUIRE_LOAD(&tso->why_blocked); + StgThreadWhyBlocked why_blocked = ACQUIRE_LOAD(&tso->why_blocked); ASSERT(why_blocked == BlockedOnMVarRead || why_blocked == BlockedOnMVar); ASSERT(tso->block_info.mvar == mvar); @@ -949,7 +949,7 @@ end: void printThreadBlockage(StgTSO *tso) { - switch (ACQUIRE_LOAD(&tso->why_blocked)) { + switch (UntagWhyBlocked(ACQUIRE_LOAD(&tso->why_blocked))) { #if defined(mingw32_HOST_OS) case BlockedOnDoProc: debugBelch("is blocked on proc (request: %" FMT_Word ")", tso->block_info.async_reqID); @@ -1046,7 +1046,7 @@ printAllThreads(void) debugBelch("other threads:\n"); for (g = 0; g < RtsFlags.GcFlags.generations; g++) { for (t = generations[g].threads; t != END_TSO_QUEUE; t = next) { - if (t->why_blocked != NotBlocked) { + if (RELAXED_LOAD(&t->why_blocked) != NotBlocked) { printThreadStatus(t); } next = t->global_link; ===================================== rts/TraverseHeap.c ===================================== @@ -1242,15 +1242,12 @@ inner_loop: traversePushClosure(ts, (StgClosure *) tso->blocked_exceptions, c, sep, child_data); traversePushClosure(ts, (StgClosure *) tso->bq, c, sep, child_data); traversePushClosure(ts, (StgClosure *) tso->trec, c, sep, child_data); - switch (ACQUIRE_LOAD(&tso->why_blocked)) { - case BlockedOnMVar: - case BlockedOnMVarRead: - case BlockedOnBlackHole: - case BlockedOnMsgThrowTo: + + StgThreadWhyBlocked why_blocked = ACQUIRE_LOAD(&tso->why_blocked); + if (IsBlockInfoClosure(why_blocked) && why_blocked != NotBlocked) { + // The NotBlocked case uses block_info.prev as a TSO back link. + // Do not follow in that case or we'll get into a loop. traversePushClosure(ts, tso->block_info.closure, c, sep, child_data); - break; - default: - break; } goto loop; } ===================================== rts/include/rts/Constants.h ===================================== @@ -253,6 +253,33 @@ * block_info union. The comment for each tag below says which member * of the block_info union is used. * + * We also use the why_blocked to determine if the block_info contains + * a closure or not. There are three classes of tag: + * 1. why_blocked tags where block_info is always a closure; + * 2. why_blocked tags where block_info is never a closure; + * 3. why_blocked tags where block_info is sometimes a closure; + * + * We use the following encoding scheme for the three classes above: + * 1. the tag value has bits 3 and 4 unset (values 0..7); + * 2. the tag value has bit 3 set (values 8..15); and + * 3. the tag value has bit 4 set when it is not a closure and unset + * when it is a closure. + * + * This scheme makes it cheap and simple to check if the GC needs to + * look at the block_info.closure. + * + * The reason for the encoding using 2 marker bits rather than 1 is + * that it minimises the cases in the code that need to use or check + * the tag bits. The only tags in class 3 are BlockedOn{Read,Write + * Delay} which are used by in-RTS I/O managers, and the only ones that + * need to use block_info members that are not a closure are the legacy + * I/O managers select and win32-legacy. So when these I/O managers are + * removed then we can simplify the encoding. + */ +#define BlockInfoForceNonClosure 16 +#define UntagWhyBlocked(why) ((why) & 15) +#define IsBlockInfoClosure(why) (((why) & 24) == 0) +/* * In the threaded RTS there is an invariant that the block_info union * is always a valid GC closure. To ensure this, the tags that use * block_info.unused, always set it to END_TSO_QUEUE. The non-closure @@ -269,21 +296,31 @@ #define BlockedOnMVarRead 2 /* Uses block_info.mvar */ #define BlockedOnBlackHole 3 /* Uses block_info.bh */ #define BlockedOnMsgThrowTo 4 /* Uses block_info.throwto */ -#define BlockedOnRead 5 /* Uses block_info.aiop or uses .fd or - .async_result */ -#define BlockedOnWrite 6 /* Uses block_info.aiop or uses .fd or - .async_result */ -#define BlockedOnDelay 7 /* Uses block_info.timeout or +#define BlockedOnRead 5 /* Uses block_info.aiop + or with BlockInfoForceNonClosure + uses .fd or .async_reqID */ +#define BlockedOnWrite 6 /* Uses block_info.aiop + or with BlockInfoForceNonClosure + uses .fd or .async_reqID */ +#define BlockedOnDelay 7 /* Uses block_info.timeout + or with BlockInfoForceNonClosure uses .target */ #define BlockedOnSTM 8 /* Uses block_info.unused */ #define BlockedOnCCall 9 /* Uses block_info.unused */ #define BlockedOnCCall_Interruptible 10 /* Uses block_info.unused - * Same as BlockedOnCCall but permits - * killing the worker thread */ + * Same as BlockedOnCCall but permits + * killing the worker thread */ #define ThreadMigrating 11 /* Uses block_info.unused */ -#define BlockedOnDoProc 12 /* Uses block_info.async_result - * used by win32-legacy I/O manager */ +#define BlockedOnDoProc 12 /* Uses block_info.async_reqID */ + +/* Reserved values, not values that why_blocked currently use. They + * are used in primop stg_threadStatuszh and must not overlap with + * other why_blocked status values. They could be changed, if the + * threadStatus in ghc-internal is updated too. + */ +#define BlockedThreadComplete 16 +#define BlockedThreadKilled 17 /* Next available non-closure why_blocked tag numbers are: 13,14,15 * For more closure tag numbers, shift up all the non-closure ones ===================================== rts/include/rts/storage/TSO.h ===================================== @@ -30,6 +30,15 @@ typedef StgWord64 StgThreadID; #define tsoLocked(tso) ((tso)->flags & TSO_LOCKED) +/* Type for the tso->why_blocked field. See values in Constants.h. + * + * The StgThreadWhyBlocked type could be 8-bits, but for reasons + * unclear it is currently 32-bits. Previous comments here claimed + * that the smallest atomic type on AArch64 is 32-bits, but this is + * false. + */ +typedef StgWord32 StgThreadWhyBlocked; + /* * Type returned after running a thread. Values of this type * include HeapOverflow, StackOverflow etc. See Constants.h for the @@ -98,7 +107,21 @@ typedef union { * have the reason in the why_blocked field of the TSO, and some * further info (such as the closure the thread is blocked on, or the * file descriptor if the thread is waiting on I/O) in the block_info - * field. + * field. See Constants.h for the why_blocked values. + * + * The why_blocked field must be updated atomically. The protocol for + * updating block_info and why_blocked fields together is as follows: + * + * Writes: + * - first write block_info (normal non-atomic write) + * - then write why_blocked with an atomic *store release* + * + * Reads: + * - first read why_blocked with an atomic *load acquire* + * - then read block_info (normal non-atomic read) + * + * Read of only why_blocked without block_info: + * - read why_blocked with an atomic *relaxed load* */ typedef struct StgTSO_ { @@ -148,11 +171,7 @@ typedef struct StgTSO_ { StgWord16 what_next; // Values defined in Constants.h StgWord32 flags; // Values defined in Constants.h - /* - * N.B. why_blocked only has a handful of values but must be atomically - * updated; the smallest width which AArch64 supports for is 32-bits. - */ - StgWord32 why_blocked; // Values defined in Constants.h + StgThreadWhyBlocked why_blocked; // Values defined in Constants.h StgTSOBlockInfo block_info; // Barrier provided by why_blocked StgThreadID id; StgWord32 saved_errno; ===================================== rts/posix/Poll.c ===================================== @@ -183,8 +183,9 @@ bool syncIOWaitReadyPoll(CapIOManager *iomgr, StgTSO *tso, aiop->notify.tso = tso; aiop->notify_type = NotifyTSO; aiop->live = &stg_ASYNCIO_LIVE0_closure; - tso->why_blocked = rw == IORead ? BlockedOnRead : BlockedOnWrite; tso->block_info.aiop = aiop; + RELEASE_STORE(&tso->why_blocked, rw == IORead ? BlockedOnRead + : BlockedOnWrite); return asyncIOWaitReadyPoll(iomgr, aiop, rw, fd); } @@ -299,10 +300,9 @@ static void notifyIOCompletion(CapIOManager *iomgr, StgAsyncIOOp *aiop) * cap because the tso was not on the run queue of any cap and * so is not subject to thread migration. */ - StgTSO *tso = aiop->notify.tso; - tso->why_blocked = NotBlocked; - tso->_link = END_TSO_QUEUE; + StgTSO *tso = aiop->notify.tso; pushOnRunQueue(iomgr->cap, tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); } /* For the TSO case, the aiop was only reachable from the TSO * itself, and thus it is now no longer be reachable at all. ===================================== rts/posix/Select.c ===================================== @@ -138,11 +138,10 @@ static bool wakeUpSleepingThreads (CapIOManager *iomgr, LowResTime now) break; } iomgr->sleeping_queue = tso->_link; - RELAXED_STORE(&tso->why_blocked, NotBlocked); - tso->_link = END_TSO_QUEUE; IF_DEBUG(scheduler, debugBelch("Waking up sleeping thread %" FMT_StgThreadID "\n", tso->id)); pushOnRunQueue(iomgr->cap,tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); flag = true; } return flag; @@ -311,7 +310,7 @@ awaitCompletedTimeoutsOrIOSelect(CapIOManager *iomgr, bool wait) * So the (int) cast should be removed across the code base once * GHC requires a version of FreeBSD that has that change in it. */ - switch (ACQUIRE_LOAD(&tso->why_blocked)) { + switch (UntagWhyBlocked(ACQUIRE_LOAD(&tso->why_blocked))) { case BlockedOnRead: { int fd = tso->block_info.fd; @@ -449,7 +448,7 @@ awaitCompletedTimeoutsOrIOSelect(CapIOManager *iomgr, bool wait) int fd; enum FdState fd_state = RTS_FD_IS_BLOCKING; - switch (tso->why_blocked) { + switch (UntagWhyBlocked(ACQUIRE_LOAD(&tso->why_blocked))) { case BlockedOnRead: fd = tso->block_info.fd; @@ -488,9 +487,8 @@ awaitCompletedTimeoutsOrIOSelect(CapIOManager *iomgr, bool wait) IF_DEBUG(scheduler, debugBelch("Waking up blocked thread %" FMT_StgThreadID "\n", tso->id)); - tso->why_blocked = NotBlocked; - tso->_link = END_TSO_QUEUE; pushOnRunQueue(iomgr->cap,tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); break; case RTS_FD_IS_BLOCKING: if (prev == NULL) ===================================== rts/posix/Timeout.c ===================================== @@ -48,8 +48,8 @@ bool syncDelayTimeout(CapIOManager *iomgr, StgTSO *tso, HsInt us_delay) initElemTimeoutQueue(timeout, notify, NotifyTSO, iomgr->cap->r.rCCCS); ASSERT(tso->why_blocked == NotBlocked); - tso->why_blocked = BlockedOnDelay; tso->block_info.timeout = timeout; + RELEASE_STORE(&tso->why_blocked, BlockedOnDelay); insertTimeoutQueue(&iomgr->timeout_queue, timeout, target); @@ -116,10 +116,9 @@ static void notifyTimeoutCompletion(CapIOManager *iomgr, StgTimeout *timeout) switch (timeout->notify_type) { case NotifyTSO: { - StgTSO *tso = timeout->notify.tso; - tso->why_blocked = NotBlocked; - tso->_link = END_TSO_QUEUE; + StgTSO *tso = timeout->notify.tso; pushOnRunQueue(iomgr->cap, tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); break; } case NotifyMVar: ===================================== rts/sm/Compact.c ===================================== @@ -468,16 +468,10 @@ thread_TSO (StgTSO *tso) thread_(&tso->_link); thread_(&tso->global_link); - switch (ACQUIRE_LOAD(&tso->why_blocked)) { - case BlockedOnMVar: - case BlockedOnMVarRead: - case BlockedOnBlackHole: - case BlockedOnMsgThrowTo: - case NotBlocked: + if (IsBlockInfoClosure(ACQUIRE_LOAD(&tso->why_blocked))) { + /* This also follows the block_info.prev back-link in + * the NotBlocked case, which may not be necessary. */ thread_(&tso->block_info.closure); - break; - default: - break; } thread_(&tso->blocked_exceptions); thread_(&tso->bq); ===================================== rts/sm/NonMovingMark.c ===================================== @@ -1055,16 +1055,10 @@ trace_tso (MarkQueue *queue, StgTSO *tso) if (tso->label != NULL) { markQueuePushClosure_(queue, (StgClosure *) tso->label); } - switch (ACQUIRE_LOAD(&tso->why_blocked)) { - case BlockedOnMVar: - case BlockedOnMVarRead: - case BlockedOnBlackHole: - case BlockedOnMsgThrowTo: - case NotBlocked: + if (IsBlockInfoClosure(ACQUIRE_LOAD(&tso->why_blocked))) { + /* This also follows the block_info.prev back-link in + * the NotBlocked case, which may not be necessary. */ markQueuePushClosure_(queue, tso->block_info.closure); - break; - default: - break; } } ===================================== rts/sm/Sanity.c ===================================== @@ -779,13 +779,45 @@ checkTSO(StgTSO *tso) info == &stg_WHITEHOLE_info); // used to happen due to STM doing // lockTSO(), might not happen now - if ( tso->why_blocked == BlockedOnMVar - || tso->why_blocked == BlockedOnMVarRead - || tso->why_blocked == BlockedOnBlackHole - || tso->why_blocked == BlockedOnMsgThrowTo - || tso->why_blocked == NotBlocked - ) { + StgThreadWhyBlocked why_blocked = ACQUIRE_LOAD(&tso->why_blocked); + switch (why_blocked) { + case NotBlocked: + case BlockedOnMVar: + case BlockedOnMVarRead: + case BlockedOnBlackHole: + case BlockedOnMsgThrowTo: + case BlockedOnRead: + case BlockedOnWrite: + case BlockedOnDelay: + //TODO: we could be more specific and check BlockedOnMVar has an MVar, + // BlockedOnBlackHole has a message, BlockedOnRead has an AIOP etc. + ASSERT(IsBlockInfoClosure(why_blocked)); ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->block_info.closure)); + break; + + case BlockedOnSTM: + case BlockedOnCCall: + case BlockedOnCCall_Interruptible: + case ThreadMigrating: + ASSERT(!IsBlockInfoClosure(why_blocked)); + ASSERT(tso->block_info.unused == END_TSO_QUEUE); + break; + +#if !defined(THREADED_RTS) + // Only these three can use BlockInfoForceNonClosure + case BlockedOnRead | BlockInfoForceNonClosure: + case BlockedOnWrite | BlockInfoForceNonClosure: + case BlockedOnDelay | BlockInfoForceNonClosure: +#if defined(mingw32_HOST_OS) + case BlockedOnDoProc: +#endif + ASSERT(!IsBlockInfoClosure(why_blocked)); + break; +#endif + + default: + barf("checkTSO: strange tso->why_blocked: %d for TSO %" + FMT_StgThreadID " (%p)", why_blocked, tso->id, tso); } ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->bq)); ===================================== rts/sm/Scav.c ===================================== @@ -138,21 +138,9 @@ scavengeTSO (StgTSO *tso) evacuate((StgClosure **)&tso->label); } - switch (ACQUIRE_LOAD(&tso->why_blocked)) { - case BlockedOnMVar: - case BlockedOnMVarRead: - case BlockedOnBlackHole: - case BlockedOnMsgThrowTo: - case NotBlocked: + if (IsBlockInfoClosure(ACQUIRE_LOAD(&tso->why_blocked))) { evacuate(&tso->block_info.closure); - break; - case BlockedOnRead: - case BlockedOnWrite: - case BlockedOnDelay: - case BlockedOnDoProc: - scavengeTSOIOManager(tso); - break; - default: + } else { #if defined(THREADED_RTS) // in the THREADED_RTS, block_info.closure must always point to a // valid closure, because we assume this in throwTo(). In the @@ -160,7 +148,6 @@ scavengeTSO (StgTSO *tso) // BlockedOnRead/BlockedOnWrite) or a time value (BlockedOnDelay) ASSERT(tso->block_info.unused == END_TSO_QUEUE); #endif - break; } tso->dirty = gct->failed_to_evac; ===================================== rts/win32/AsyncMIO.c ===================================== @@ -304,7 +304,7 @@ start: for(tso = iomgr->blocked_queue_hd; tso != END_TSO_QUEUE; tso = tso->_link) { - switch(ACQUIRE_LOAD(&tso->why_blocked)) { + switch (UntagWhyBlocked(ACQUIRE_LOAD(&tso->why_blocked))) { case BlockedOnRead: case BlockedOnWrite: case BlockedOnDoProc: @@ -324,8 +324,6 @@ start: } // Terminates the run queue + this inner for-loop. - tso->_link = END_TSO_QUEUE; - tso->why_blocked = NotBlocked; // For stg_block_async frames (read/write/doProc), // write len and errCode directly to the stack. // For stg_block_noregs frames (delay), nothing @@ -335,14 +333,14 @@ start: tso->stackobj->sp[2] = (W_)errCode; } pushOnRunQueue(&MainCapability, tso); + RELEASE_STORE(&tso->why_blocked, NotBlocked); break; } break; - default: - if (tso->why_blocked != NotBlocked) { - barf("awaitRequests: odd thread state"); - } + case NotBlocked: break; + default: + barf("awaitRequests: odd thread state"); } prev = tso; View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/e76e9134ad3e6dcff1193bcf1979a21... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/e76e9134ad3e6dcff1193bcf1979a21... 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