GitLab

Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC

Commits:

13d38047

by Luite Stegeman at 2026-10-31T09:19:11-04:00

rts: tombstone Weak# copies that lose an evacuation race (#23160)

Mark the losing copy of a Weak# during an evacuation race in parallel
GC with stg_DEAD_WEAK_info. This ensures that the nonmoving GC
doesn't follow dangling pointers, leading to crashes.

Fixes #23160

5 changed files:

+ changelog.d/fix-nonmoving-weak-evacuation-race-23160
rts/sm/Evac.c
+ testsuite/tests/rts/T23160.hs
+ testsuite/tests/rts/T23160.stdout
testsuite/tests/rts/all.T

Changes:

changelog.d/fix-nonmoving-weak-evacuation-race-23160

 +section: rts
 +synopsis: Fix crash in the nonmoving GC caused by racing weak pointer evacuation
 +issues: #23160
 +mrs: !16522
 +description:
 +  When two threads compete for evacuating the same ``Weak#`` object, the
 +  losing thread would produces an object that is never put on a weak list.
 +  This resulted in pointers in the object getting stale, and the nonmoving
 +  GC crashing trying to follow them. This is fixed by immediately marking
 +  the losing weak object as dead.

rts/sm/Evac.c

@@ -265,6 +265,17 @@ copy_tag(StgClosure **p, const StgInfoTable *info,
                LDVW(to) = 0;
+             }
  #endif
 +            // The weak we just created is dead and doesn't appear on any
 +            // weak pointer list. This means that its value, finalizer and link
 +            // pointers will not be updated. However the nonmoving collector
 +            // can still see it (update remembered set, mutable list), so we must
 +            // tombstone it to avoid following stale pointers. See #23160.
++
 +            // The copy is still thread-private here so a plain store
 +            // suffices.
 +            if (INFO_PTR_TO_STRUCT(info)->type == WEAK) {
 +                ((StgClosure *)to)->header.info = &stg_DEAD_WEAK_info;
 +            }
              return evacuate(p); // does the failed_to_evac stuff
          } else {
              // This doesn't need to have RELEASE ordering since we are guaranteed

testsuite/tests/rts/T23160.hs

 +{-# LANGUAGE UnboxedTuples #-}
 +{-# LANGUAGE MagicHash #-}
++
 +-- Regression test for multiple threads competing for evacuating a Weak#
 +-- Based on the reproducer by Ben Gamari in #23160
++
++
 +import GHC.Exts
 +import GHC.Weak
 +import GHC.IO (IO(..))
 +import Control.Concurrent
 +import Control.Exception
 +import Control.Monad
 +import System.Mem
 +import System.Mem.Weak
++
 +data MBA = MBA (MutableByteArray# RealWorld)
++
 +newPinnedMBA :: Int -> IO MBA
 +newPinnedMBA (I# n) = IO $ \s ->
 +  case newPinnedByteArray# n s of
 +    (# s', mba #) -> (# s', MBA mba #)
++
 +mkMBAWeak :: MBA -> a -> IO (Weak a)
 +mkMBAWeak (MBA mba) v = IO $
 +    \s -> case mkWeakNoFinalizer# mba v s of
 +            (# s', w #) -> (# s', Weak w #)
++
 +forced :: String -> IO Int
 +forced a = evaluate (length a)
++
 +worker :: IO ()
 +worker = forever $ do
 +    xs <- mapM f [0..200000::Int]
 +    mapM_ (\w -> deRefWeak w >>= mapM_ forced) (take 1000 xs)
 +  where
 +    f n = do
 +      mba <- newPinnedMBA 10
 +      v <- forced (show n)
 +      mkMBAWeak mba (show (n + v))
++
 +main :: IO ()
 +main = do
 +  forkIO $ forever $ do
 +    threadDelay (100*1000)
 +    performMajorGC
++
 +  replicateM_ 10 (forkIO worker)
 +  threadDelay (25*1000*1000)
 +  putStrLn "OK"

testsuite/tests/rts/T23160.stdout

+OK

testsuite/tests/rts/all.T

@@ -767,3 +767,12 @@ test('T27526',
        extra_hc_opts('-fno-prof-auto -fprof-late -rtsopts'),
        extra_run_opts('+RTS -kc2k -kb256 -A64k -AL8k -RTS')],
       compile_and_run, [''])
++
 +test('T23160',
 +     [ req_target_smp
 +     , extra_ways(['nonmoving_thr_sanity'])
 +     , only_ways(['nonmoving_thr_sanity'])
 +     , extra_run_opts('+RTS -A1M -N4 -RTS')
 +     , unless(slow(), skip)
 +     ],
 +     compile_and_run, [''])