Fix failure in setnumcapabilities001 (#12728)

The value of enabled_capabilities can change across a call to
requestSync(), and we were erroneously using an old value, causing
things to go wrong later.  It manifested as an assertion failure, I'm
not sure whether there are worse consequences or not, but we should
get this fix into 8.0.2 anyway.

The failure didn't happen for me because it only shows up on machines
with fewer than 4 processors, due to the new logic to enable -qn
automatically.  I've bumped the test parameter 8 to make it more
likely to exercise that code.

Test Plan: Ran setnumcapabilities001 many times

Reviewers: niteria, austin, erikd, rwbarton, bgamari

Reviewed By: bgamari

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2617

GHC Trac Issues: #12728

1 files changed, 26 insertions, 27 deletions

diff --git a/rts/Schedule.c b/rts/Schedule.c
index 3cbfc0eba0..06db3fe81f 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -1562,35 +1562,14 @@ scheduleDoGC (Capability **pcap, Task *task USED_IF_THREADS,
         gc_type = SYNC_GC_SEQ;
     }
 
-    // If -qn is not set and we have more capabilities than cores, set the
-    // number of GC threads to #cores.  We do this here rather than in
-    // normaliseRtsOpts() because here it will work if the program calls
-    // setNumCapabilities.
-    n_gc_threads = RtsFlags.ParFlags.parGcThreads;
-    if (n_gc_threads == 0 && enabled_capabilities > getNumberOfProcessors()) {
-        n_gc_threads = getNumberOfProcessors();
-    }
-
-    if (gc_type == SYNC_GC_PAR && n_gc_threads > 0) {
-        need_idle = stg_max(0, enabled_capabilities - n_gc_threads);
-    } else {
-        need_idle = 0;
-    }
-
     // In order to GC, there must be no threads running Haskell code.
-    // Therefore, the GC thread needs to hold *all* the capabilities,
-    // and release them after the GC has completed.
-    //
-    // This seems to be the simplest way: previous attempts involved
-    // making all the threads with capabilities give up their
-    // capabilities and sleep except for the *last* one, which
-    // actually did the GC.  But it's quite hard to arrange for all
-    // the other tasks to sleep and stay asleep.
+    // Therefore, for single-threaded GC, the GC thread needs to hold *all* the
+    // capabilities, and release them after the GC has completed.  For parallel
+    // GC, we synchronise all the running threads using requestSync().
     //
-
-    /*  Other capabilities are prevented from running yet more Haskell
-        threads if pending_sync is set. Tested inside
-        yieldCapability() and releaseCapability() in Capability.c */
+    // Other capabilities are prevented from running yet more Haskell threads if
+    // pending_sync is set. Tested inside yieldCapability() and
+    // releaseCapability() in Capability.c
 
     PendingSync sync = {
         .type = gc_type,
@@ -1602,6 +1581,26 @@ scheduleDoGC (Capability **pcap, Task *task USED_IF_THREADS,
         SyncType prev_sync = 0;
         rtsBool was_syncing;
         do {
+            // If -qn is not set and we have more capabilities than cores, set
+            // the number of GC threads to #cores.  We do this here rather than
+            // in normaliseRtsOpts() because here it will work if the program
+            // calls setNumCapabilities.
+            //
+            n_gc_threads = RtsFlags.ParFlags.parGcThreads;
+            if (n_gc_threads == 0 &&
+                enabled_capabilities > getNumberOfProcessors()) {
+                n_gc_threads = getNumberOfProcessors();
+            }
+
+            // This calculation must be inside the loop because
+            // enabled_capabilities may change if requestSync() below fails and
+            // we retry.
+            if (gc_type == SYNC_GC_PAR && n_gc_threads > 0) {
+                need_idle = stg_max(0, enabled_capabilities - n_gc_threads);
+            } else {
+                need_idle = 0;
+            }
+
             // We need an array of size n_capabilities, but since this may
             // change each time around the loop we must allocate it afresh.
             idle_cap = (rtsBool *)stgMallocBytes(n_capabilities *