summaryrefslogtreecommitdiff
path: root/rts/STM.c
diff options
context:
space:
mode:
authorSimon Marlow <marlowsd@gmail.com>2018-07-12 10:13:47 -0400
committerBen Gamari <ben@smart-cactus.org>2018-07-12 15:04:20 -0400
commit7fc418df856d9b58034eeec48915646e67a7a562 (patch)
tree656a7a8ae131122c4253bb6c490fb9e7db671346 /rts/STM.c
parent1a79270c72cfcd98d683cfe7b2c777d8dd353b78 (diff)
downloadhaskell-7fc418df856d9b58034eeec48915646e67a7a562.tar.gz
Fix deadlock between STM and throwTo
There was a lock-order reversal between lockTSO() and the TVar lock, see #15136 for the details. It turns out we can fix this pretty easily by just deleting all the locking code(!). The principle for unblocking a `BlockedOnSTM` thread then becomes the same as for other kinds of blocking: if the TSO belongs to this capability then we do it directly, otherwise we send a message to the capability that owns the TSO. That is, a thread blocked on STM is owned by its capability, as it should be. The possible downside of this is that we might send multiple messages to wake up a thread when the thread is on another capability. This is safe, it's just not very efficient. I'll try to do some experiments to see if this is a problem. Test Plan: Test case from #15136 doesn't deadlock any more. Reviewers: bgamari, osa1, erikd Reviewed By: osa1 Subscribers: rwbarton, thomie, carter GHC Trac Issues: #15136 Differential Revision: https://phabricator.haskell.org/D4956
Diffstat (limited to 'rts/STM.c')
-rw-r--r--rts/STM.c19
1 files changed, 1 insertions, 18 deletions
diff --git a/rts/STM.c b/rts/STM.c
index 058eec7409..abb44172dd 100644
--- a/rts/STM.c
+++ b/rts/STM.c
@@ -332,24 +332,7 @@ static void unpark_tso(Capability *cap, StgTSO *tso) {
// queues: it's up to the thread itself to remove it from the wait queues
// if it decides to do so when it is scheduled.
- // Unblocking a TSO from BlockedOnSTM is done under the TSO lock,
- // to avoid multiple CPUs unblocking the same TSO, and also to
- // synchronise with throwTo(). The first time the TSO is unblocked
- // we mark this fact by setting block_info.closure == STM_AWOKEN.
- // This way we can avoid sending further wakeup messages in the
- // future.
- lockTSO(tso);
- if (tso->why_blocked == BlockedOnSTM &&
- tso->block_info.closure == &stg_STM_AWOKEN_closure) {
- TRACE("unpark_tso already woken up tso=%p", tso);
- } else if (tso -> why_blocked == BlockedOnSTM) {
- TRACE("unpark_tso on tso=%p", tso);
- tso->block_info.closure = &stg_STM_AWOKEN_closure;
- tryWakeupThread(cap,tso);
- } else {
- TRACE("spurious unpark_tso on tso=%p", tso);
- }
- unlockTSO(tso);
+ tryWakeupThread(cap,tso);
}
static void unpark_waiters_on(Capability *cap, StgTVar *s) {