Fix an assertion that could randomly fail
authorSimon Marlow <marlowsd@gmail.com>
Thu, 4 Aug 2016 14:57:37 +0000 (15:57 +0100)
committerSimon Marlow <marlowsd@gmail.com>
Fri, 5 Aug 2016 09:13:28 +0000 (10:13 +0100)
Summary:
ASSERT_THREADED_CAPABILITY_INVARIANTS was testing properties of the
returning_tasks queue, but that requires cap->lock to access safely.
This assertion would randomly fail if stressed enough.

Instead I've removed it from the catch-all
ASSERT_PARTIAL_CAPABILITIY_INVARIANTS and made it a separate assertion
only called under cap->lock.

Test Plan:
```
cd testsuite/tests/concurrent/should_run
make TEST=setnumcapabilities001 WAY=threaded1 EXTRA_HC_OPTS=-with-rtsopts=-DS CLEANUP=0
while true; do ./setnumcapabilities001.run/setnumcapabilities001 4 9 2000 || break; done
```

Reviewers: niteria, bgamari, ezyang, austin, erikd

Subscribers: thomie

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

GHC Trac Issues: #10860

rts/Capability.c
rts/Capability.h

index f2220f0..681797b 100644 (file)
@@ -213,6 +213,7 @@ newReturningTask (Capability *cap, Task *task)
     }
     cap->returning_tasks_tl = task;
     cap->n_returning_tasks++;
+    ASSERT_RETURNING_TASKS(cap,task);
 }
 
 STATIC_INLINE Task *
@@ -228,6 +229,7 @@ popReturningTask (Capability *cap)
     }
     task->next = NULL;
     cap->n_returning_tasks--;
+    ASSERT_RETURNING_TASKS(cap,task);
     return task;
 }
 #endif
@@ -507,6 +509,7 @@ releaseCapability_ (Capability* cap,
     task = cap->running_task;
 
     ASSERT_PARTIAL_CAPABILITY_INVARIANTS(cap,task);
+    ASSERT_RETURNING_TASKS(cap,task);
 
     cap->running_task = NULL;
 
index 6779624..8e0288b 100644 (file)
@@ -174,13 +174,15 @@ struct Capability_ {
   ASSERT(task->cap == cap);                                             \
   ASSERT_PARTIAL_CAPABILITY_INVARIANTS(cap,task)
 
+// This assert requires cap->lock to be held, so it can't be part of
+// ASSERT_PARTIAL_CAPABILITY_INVARIANTS()
 #if defined(THREADED_RTS)
-#define ASSERT_THREADED_CAPABILITY_INVARIANTS(cap,task)                  \
+#define ASSERT_RETURNING_TASKS(cap,task)                                \
   ASSERT(cap->returning_tasks_hd == NULL ?                              \
            cap->returning_tasks_tl == NULL && cap->n_returning_tasks == 0 \
          : 1);
 #else
-#define ASSERT_THREADED_CAPABILITY_INVARIANTS(cap,task) /* nothing */
+#define ASSERT_RETURNING_TASKS(cap,task) /* nothing */
 #endif
 
 // Sometimes a Task holds a Capability, but the Task is not associated
@@ -193,7 +195,6 @@ struct Capability_ {
             cap->run_queue_tl == END_TSO_QUEUE && cap->n_run_queue == 0 \
          : 1);                                                          \
   ASSERT(cap->suspended_ccalls == NULL ? cap->n_suspended_ccalls == 0 : 1); \
-  ASSERT_THREADED_CAPABILITY_INVARIANTS(cap,task);                      \
   ASSERT(myTask() == task);                                             \
   ASSERT_TASK_ID(task);