From 1ada18d287927d78d9dbdba062e2c882177f2d4f Mon Sep 17 00:00:00 2001
From: Ben Gamari <ben@smart-cactus.org>
Date: Fri, 27 Sep 2019 01:51:39 +0000
Subject: [PATCH] rts: Fix races in Pthread timer backend shudown
We can generally be pretty relaxed in the barriers here since the timer
thread is a loop.
---
rts/posix/itimer/Pthread.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/rts/posix/itimer/Pthread.c b/rts/posix/itimer/Pthread.c
index 860dc81cb516..6f86337ff512 100644
--- a/rts/posix/itimer/Pthread.c
+++ b/rts/posix/itimer/Pthread.c
@@ -84,11 +84,11 @@ static Time itimer_interval = DEFAULT_TICK_INTERVAL;
// Should we be firing ticks?
// Writers to this must hold the mutex below.
-static volatile bool stopped = false;
+static bool stopped = false;
// should the ticker thread exit?
// This can be set without holding the mutex.
-static volatile bool exited = true;
+static bool exited = true;
// Signaled when we want to (re)start the timer
static Condition start_cond;
@@ -119,7 +119,9 @@ static void *itimer_thread_func(void *_handle_tick)
}
#endif
- while (!exited) {
+ // Relaxed is sufficient: If we don't see that exited was set in one iteration we will
+ // see it next time.
+ while (!RELAXED_LOAD(&exited)) {
if (USE_TIMERFD_FOR_ITIMER) {
ssize_t r = read(timerfd, &nticks, sizeof(nticks));
if ((r == 0) && (errno == 0)) {
@@ -141,7 +143,8 @@ static void *itimer_thread_func(void *_handle_tick)
}
// first try a cheap test
- if (stopped) {
+ TSAN_ANNOTATE_BENIGN_RACE(&stopped, "itimer_thread_func");
+ if (RELAXED_LOAD(&stopped)) {
OS_ACQUIRE_LOCK(&mutex);
// should we really stop?
if (stopped) {
@@ -185,7 +188,7 @@ void
startTicker(void)
{
OS_ACQUIRE_LOCK(&mutex);
- stopped = 0;
+ RELAXED_STORE(&stopped, false);
signalCondition(&start_cond);
OS_RELEASE_LOCK(&mutex);
}
@@ -195,7 +198,7 @@ void
stopTicker(void)
{
OS_ACQUIRE_LOCK(&mutex);
- stopped = 1;
+ RELAXED_STORE(&stopped, true);
OS_RELEASE_LOCK(&mutex);
}
@@ -203,8 +206,8 @@ stopTicker(void)
void
exitTicker (bool wait)
{
- ASSERT(!exited);
- exited = true;
+ ASSERT(!SEQ_CST_LOAD(&exited));
+ SEQ_CST_STORE(&exited, true);
// ensure that ticker wakes up if stopped
startTicker();
--
GitLab