[commit: ghc] master: Fix an assertion that could randomly fail (ce13a9a)

[git at git.haskell.org]

Repository : ssh://git@git.haskell.org/ghc

On branch  : master
Link       : http://ghc.haskell.org/trac/ghc/changeset/ce13a9a9f57d61170837532948fed8bc1924a7ab/ghc

>---------------------------------------------------------------

commit ce13a9a9f57d61170837532948fed8bc1924a7ab
Author: Simon Marlow <marlowsd at gmail.com>
Date:   Thu Aug 4 15:57:37 2016 +0100

    Fix an assertion that could randomly fail
    
    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


>---------------------------------------------------------------

ce13a9a9f57d61170837532948fed8bc1924a7ab
 rts/Capability.c | 3 +++
 rts/Capability.h | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/rts/Capability.c b/rts/Capability.c
index f2220f0..681797b 100644
--- a/rts/Capability.c
+++ b/rts/Capability.c
@@ -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;
 
diff --git a/rts/Capability.h b/rts/Capability.h
index 6779624..8e0288b 100644
--- a/rts/Capability.h
+++ b/rts/Capability.h
@@ -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);