From 6d31658583b97f84313cc7837a0eaeb4060022a7 Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Thu, 13 Nov 2008 15:57:30 +0000 Subject: [PATCH] Fix an extremely subtle deadlock bug on x86_64 The recent_activity flag was an unsigned int, but we sometimes do a 64-bit xchg() on it, which overwrites the next word in memory. This happened to contain the sched_state flag, which is used to control the orderly shutdown of the system. If the xchg() happened during shutdown, the scheduler would get confused and deadlock. Don't you just love C? --- rts/Schedule.c | 8 +++++--- rts/Schedule.h | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/rts/Schedule.c b/rts/Schedule.c index 63e2e99..499cf77 100644 --- a/rts/Schedule.c +++ b/rts/Schedule.c @@ -92,13 +92,15 @@ rtsBool blackholes_need_checking = rtsFalse; /* flag that tracks whether we have done any execution in this time slice. * LOCK: currently none, perhaps we should lock (but needs to be * updated in the fast path of the scheduler). + * + * NB. must be StgWord, we do xchg() on it. */ -nat recent_activity = ACTIVITY_YES; +volatile StgWord recent_activity = ACTIVITY_YES; /* if this flag is set as well, give up execution - * LOCK: none (changes once, from false->true) + * LOCK: none (changes monotonically) */ -rtsBool sched_state = SCHED_RUNNING; +volatile StgWord sched_state = SCHED_RUNNING; /* This is used in `TSO.h' and gcc 2.96 insists that this variable actually * exists - earlier gccs apparently didn't. diff --git a/rts/Schedule.h b/rts/Schedule.h index d76f42b..c3334e6 100644 --- a/rts/Schedule.h +++ b/rts/Schedule.h @@ -97,7 +97,7 @@ void initThread(StgTSO *tso, nat stack_size); #define SCHED_INTERRUPTING 1 /* ^C detected, before threads are deleted */ #define SCHED_SHUTTING_DOWN 2 /* final shutdown */ -extern rtsBool RTS_VAR(sched_state); +extern volatile StgWord RTS_VAR(sched_state); /* * flag that tracks whether we have done any execution in this time slice. @@ -113,7 +113,7 @@ extern rtsBool RTS_VAR(sched_state); * INACTIVE to DONE_GC happens under sched_mutex. No lock required * to set it to ACTIVITY_YES. */ -extern nat recent_activity; +extern volatile StgWord recent_activity; /* Thread queues. * Locks required : sched_mutex -- 1.7.10.4