Fix a bug which sometimes caused extra major GCs to be performed

A long-running GC would cause the timer signal to declare the system
to be idle, which would cause a major GC immediately following the
current GC.  This only happened with +RTS -N2 or greater.

3 files changed, 28 insertions, 18 deletions

diff --git a/rts/Schedule.c b/rts/Schedule.c
index 28e54f9516..47636a3ff4 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -1606,29 +1606,38 @@ delete_threads_and_gc:
     
     heap_census = scheduleNeedHeapProfile(rtsTrue);
 
+#if defined(THREADED_RTS)
+    debugTrace(DEBUG_sched, "doing GC");
+    // reset waiting_for_gc *before* GC, so that when the GC threads
+    // emerge they don't immediately re-enter the GC.
+    waiting_for_gc = 0;
+    GarbageCollect(force_major || heap_census, gc_type, cap);
+#else
+    GarbageCollect(force_major || heap_census, 0, cap);
+#endif
+
     if (recent_activity == ACTIVITY_INACTIVE && force_major)
     {
         // We are doing a GC because the system has been idle for a
         // timeslice and we need to check for deadlock.  Record the
         // fact that we've done a GC and turn off the timer signal;
         // it will get re-enabled if we run any threads after the GC.
-        //
-        // Note: this is done before GC, because after GC there might
-        // be threads already running (GarbageCollect() releases the
-        // GC threads when it completes), so we risk turning off the
-        // timer signal when it should really be on.
         recent_activity = ACTIVITY_DONE_GC;
         stopTimer();
     }
+    else
+    {
+        // the GC might have taken long enough for the timer to set
+        // recent_activity = ACTIVITY_INACTIVE, but we aren't
+        // necessarily deadlocked:
+        recent_activity = ACTIVITY_YES;
+    }
 
 #if defined(THREADED_RTS)
-    debugTrace(DEBUG_sched, "doing GC");
-    // reset waiting_for_gc *before* GC, so that when the GC threads
-    // emerge they don't immediately re-enter the GC.
-    waiting_for_gc = 0;
-    GarbageCollect(force_major || heap_census, gc_type, cap);
-#else
-    GarbageCollect(force_major || heap_census, 0, cap);
+    if (gc_type == PENDING_GC_PAR)
+    {
+        releaseGCThreads(cap);
+    }
 #endif
 
     if (heap_census) {
diff --git a/rts/sm/GC.c b/rts/sm/GC.c
index e44a310f45..ec9cd0749d 100644
--- a/rts/sm/GC.c
+++ b/rts/sm/GC.c
@@ -146,7 +146,6 @@ static nat  inc_running             (void);
 static nat  dec_running             (void);
 static void wakeup_gc_threads       (nat n_threads, nat me);
 static void shutdown_gc_threads     (nat n_threads, nat me);
-static void continue_gc_threads     (nat n_threads, nat me);
 
 #if 0 && defined(DEBUG)
 static void gcCAFs                  (void);
@@ -787,8 +786,6 @@ GarbageCollect (rtsBool force_major_gc,
   }
 #endif
 
-  continue_gc_threads(n_gc_threads, gct->thread_index);
-
   RELEASE_SM_LOCK;
 
   gct = saved_gct;
@@ -1144,14 +1141,17 @@ shutdown_gc_threads (nat n_threads USED_IF_THREADS, nat me USED_IF_THREADS)
 #endif
 }
 
-static void
-continue_gc_threads (nat n_threads USED_IF_THREADS, nat me USED_IF_THREADS)
+void
+releaseGCThreads (Capability *cap USED_IF_THREADS)
 {
 #if defined(THREADED_RTS)
+    nat n_threads = RtsFlags.ParFlags.nNodes;
+    nat me = cap->no;
     nat i;
     for (i=0; i < n_threads; i++) {
         if (i == me) continue;
-        if (gc_threads[i]->wakeup != GC_THREAD_WAITING_TO_CONTINUE) barf("continue_gc_threads");
+        if (gc_threads[i]->wakeup != GC_THREAD_WAITING_TO_CONTINUE) 
+            barf("releaseGCThreads");
         
         gc_threads[i]->wakeup = GC_THREAD_INACTIVE;
         ACQUIRE_SPIN_LOCK(&gc_threads[i]->gc_spin);
diff --git a/rts/sm/GC.h b/rts/sm/GC.h
index 5fb142f58f..366125d159 100644
--- a/rts/sm/GC.h
+++ b/rts/sm/GC.h
@@ -43,6 +43,7 @@ extern StgWord64 whitehole_spin;
 void gcWorkerThread (Capability *cap);
 void initGcThreads (void);
 void waitForGcThreads (Capability *cap);
+void releaseGCThreads (Capability *cap);
 
 #define WORK_UNIT_WORDS 128