diff options
author | Simon Marlow <marlowsd@gmail.com> | 2016-08-04 15:57:37 +0100 |
---|---|---|
committer | Simon Marlow <marlowsd@gmail.com> | 2016-08-05 10:13:28 +0100 |
commit | ce13a9a9f57d61170837532948fed8bc1924a7ab (patch) | |
tree | f442352b3fa558e8b8de8328bc8351995ffd9cc9 | |
parent | ca7e1ad346362ba68b430550110e74421b19193f (diff) | |
download | haskell-ce13a9a9f57d61170837532948fed8bc1924a7ab.tar.gz |
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
-rw-r--r-- | rts/Capability.c | 3 | ||||
-rw-r--r-- | rts/Capability.h | 7 |
2 files changed, 7 insertions, 3 deletions
diff --git a/rts/Capability.c b/rts/Capability.c index f2220f0651..681797b409 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 6779624360..8e0288b15e 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); |