diff options
author | Ben Gamari <ben@smart-cactus.org> | 2022-11-10 19:35:00 -0500 |
---|---|---|
committer | Marge Bot <ben+marge-bot@smart-cactus.org> | 2022-12-23 19:09:30 -0500 |
commit | 8093264a448940133fc2f1899bab8757eabe4b85 (patch) | |
tree | ed5baf36346f0c1a4747554f02da926e2725f313 | |
parent | 26837523f5a9cdb33a206f8b22eea88ef75c874b (diff) | |
download | haskell-8093264a448940133fc2f1899bab8757eabe4b85.tar.gz |
nonmoving: Fix races in collector status tracking
Mark a number of accesses to do with tracking of the status of the
concurrent collection thread as atomic. No interesting races here,
merely necessary to satisfy TSAN.
-rw-r--r-- | rts/sm/GC.c | 2 | ||||
-rw-r--r-- | rts/sm/NonMoving.c | 15 |
2 files changed, 10 insertions, 7 deletions
diff --git a/rts/sm/GC.c b/rts/sm/GC.c index 19cb0fe31d..c91d795756 100644 --- a/rts/sm/GC.c +++ b/rts/sm/GC.c @@ -355,7 +355,7 @@ GarbageCollect (uint32_t collect_gen, deadlock_detect_gc = deadlock_detect; #if defined(THREADED_RTS) - if (major_gc && RtsFlags.GcFlags.useNonmoving && concurrent_coll_running) { + if (major_gc && RtsFlags.GcFlags.useNonmoving && RELAXED_LOAD(&concurrent_coll_running)) { /* If there is already a concurrent major collection running then * there is no benefit to starting another. * TODO: Catch heap-size runaway. diff --git a/rts/sm/NonMoving.c b/rts/sm/NonMoving.c index 130da95c3e..24ab3cbba4 100644 --- a/rts/sm/NonMoving.c +++ b/rts/sm/NonMoving.c @@ -748,11 +748,12 @@ void nonmovingStop(void) { if (! RtsFlags.GcFlags.useNonmoving) return; #if defined(THREADED_RTS) - if (mark_thread) { + if (RELAXED_LOAD(&mark_thread)) { debugTrace(DEBUG_nonmoving_gc, "waiting for nonmoving collector thread to terminate"); ACQUIRE_LOCK(&concurrent_coll_finished_lock); waitCondition(&concurrent_coll_finished, &concurrent_coll_finished_lock); + ACQUIRE_LOCK(&nonmoving_collection_mutex); } #endif } @@ -927,7 +928,7 @@ void nonmovingCollect(StgWeak **dead_weaks, StgTSO **resurrected_threads) #if defined(THREADED_RTS) // We can't start a new collection until the old one has finished // We also don't run in final GC - if (concurrent_coll_running || getSchedState() > SCHED_RUNNING) { + if (RELAXED_LOAD(&concurrent_coll_running) || getSchedState() > SCHED_RUNNING) { return; } #endif @@ -1001,13 +1002,15 @@ void nonmovingCollect(StgWeak **dead_weaks, StgTSO **resurrected_threads) // a major GC, because that's what we do when exiting scheduler (see // exitScheduler()). if (getSchedState() == SCHED_RUNNING) { - concurrent_coll_running = true; + RELAXED_STORE(&concurrent_coll_running, true); nonmoving_write_barrier_enabled = true; debugTrace(DEBUG_nonmoving_gc, "Starting concurrent mark thread"); - if (createOSThread(&mark_thread, "non-moving mark thread", + OSThreadId thread; + if (createOSThread(&thread, "non-moving mark thread", nonmovingConcurrentMark, mark_queue) != 0) { barf("nonmovingCollect: failed to spawn mark thread: %s", strerror(errno)); } + RELAXED_STORE(&mark_thread, thread); } else { nonmovingConcurrentMark(mark_queue); } @@ -1246,12 +1249,12 @@ finish: exitMyTask(); // We are done... - mark_thread = 0; + RELAXED_STORE(&mark_thread, 0); stat_endNonmovingGc(); // Signal that the concurrent collection is finished, allowing the next // non-moving collection to proceed - concurrent_coll_running = false; + RELAXED_STORE(&concurrent_coll_running, false); signalCondition(&concurrent_coll_finished); RELEASE_LOCK(&nonmoving_collection_mutex); #endif |