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.

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