On Tue, Oct 16, 2018 at 04:11:20PM +0200, Willy Tarreau wrote:
> Could you please apply the attached patch ? I'm going to merge it into 1.9
> and we'll backport it to 1.8 later.

And please add the attached one as well, which is specific to 1.8. I
suspect that different versions of compiler could emit inappropriate
code due to the threads_want_sync variable not being marked volatile.

In your case the issue would manifest itself if you're having heavy
server queueing ("maxconn" on the server lines) or if you're seeing
a lot of "server up/down" events.

Thanks,
Willy
>From a655363b34a1f09625bb3f1550dc8b2c9b19b045 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Tue, 16 Oct 2018 16:57:40 +0200
Subject: BUG/MEDIUM: threads: make sure threads_want_sync is marked volatile

The threads_want_sync variable is not volatile, which allows the compiler
to cache old copies of it for long parts of code and possibly optimize
some tests away. This could result in deadlocks when using heavy queue
activity or health check state changes.

There is no upstream commit for this fix because the sync point was
completely removed from 1.9. This fix is exclusively for 1.8.

Signed-off-by: Willy Tarreau <[email protected]>
---
 src/hathreads.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/hathreads.c b/src/hathreads.c
index 9dba4356e..a96cc7b8f 100644
--- a/src/hathreads.c
+++ b/src/hathreads.c
@@ -29,7 +29,7 @@ void thread_sync_io_handler(int fd)
 
 static HA_SPINLOCK_T sync_lock;
 static int           threads_sync_pipe[2] = {-1, -1};
-static unsigned long threads_want_sync = 0;
+volatile static unsigned long threads_want_sync = 0;
 volatile unsigned long threads_want_rdv_mask = 0;
 volatile unsigned long threads_harmless_mask = 0;
 volatile unsigned long all_threads_mask  = 1; // nbthread 1 assumed by default
-- 
2.14.4

Reply via email to