BUG/MINOR: signals/poller: ensure wakeup from signals

Add self-wake in signal_handler() to fix a race condition with a signal
coming in between checking signal_queue_len and entering polling sleep.

The changes in commit 43c891dda ("BUG/MINOR: signals/poller: set the
poller timeout to 0 when there are signals") were insufficient.

Move the signal_queue_len check from the poll implementations to
run_poll_loop() to keep that logic in one place.

The poll loops are terminated either by the parameter wake being set or
wake up due to a write to their poller_wr_pipe by wake_thread() in
signal_handler().

This fixes issue #1841.

Must be backported in every stable version.

diff --git a/src/ev_epoll.c b/src/ev_epoll.c
index 746a0d120af24685fc15b1e3eb19cf633218ff9a..880aa584d4b199aa940f40e824d246c8a708cc0c 100644 (file)
--- a/src/ev_epoll.c
+++ b/src/ev_epoll.c
@@ -222,11 +222,10 @@ static void _do_poll(struct poller *p, int exp, int wake)
        thread_idle_now();
        thread_harmless_now();
 
-       /* Now let's wait for polled events.
-        * Check if the signal queue is not empty in case we received a signal
-        * before entering the loop, so we don't wait MAX_DELAY_MS for nothing */
-       wait_time = (wake | signal_queue_len) ? 0 : compute_poll_timeout(exp);
+       /* Now let's wait for polled events. */
+       wait_time = wake ? 0 : compute_poll_timeout(exp);
        clock_entering_poll();
+
        do {
                int timeout = (global.tune.options & GTUNE_BUSY_POLLING) ? 0 : wait_time;
 
@@ -239,7 +238,7 @@ static void _do_poll(struct poller *p, int exp, int wake)
                }
                if (timeout || !wait_time)
                        break;
-               if (signal_queue_len || wake)
+               if (wake)
                        break;
                if (tick_isset(exp) && tick_is_expired(exp, now_ms))
                        break;

diff --git a/src/ev_evports.c b/src/ev_evports.c
index 7d3e8a8d77cb8ed5b934b3467cf09bc5ce2884ef..0a7df15d3b4da366bf91b6ce898513225698e842 100644 (file)
--- a/src/ev_evports.c
+++ b/src/ev_evports.c
@@ -178,10 +178,8 @@ static void _do_poll(struct poller *p, int exp, int wake)
        thread_idle_now();
        thread_harmless_now();
 
-       /* Now let's wait for polled events.
-        * Check if the signal queue is not empty in case we received a signal
-        * before entering the loop, so we don't wait MAX_DELAY_MS for nothing */
-       wait_time = (wake | signal_queue_len) ? 0 : compute_poll_timeout(exp);
+       /* Now let's wait for polled events. */
+       wait_time = wake ? 0 : compute_poll_timeout(exp);
        clock_entering_poll();
 
        do {
@@ -219,7 +217,7 @@ static void _do_poll(struct poller *p, int exp, int wake)
                        break;
                if (timeout || !wait_time)
                        break;
-               if (signal_queue_len || wake)
+               if (wake)
                        break;
                if (tick_isset(exp) && tick_is_expired(exp, now_ms))
                        break;

diff --git a/src/ev_kqueue.c b/src/ev_kqueue.c
index 1d6e91d5066331de47599d4db74ee677ebf8f98e..4796e680a44b517c6d9cea545237567f80b71dce 100644 (file)
--- a/src/ev_kqueue.c
+++ b/src/ev_kqueue.c
@@ -166,10 +166,8 @@ static void _do_poll(struct poller *p, int exp, int wake)
        }
        fd_nbupdt = 0;
 
-       /* Now let's wait for polled events.
-        * Check if the signal queue is not empty in case we received a signal
-        * before entering the loop, so we don't wait MAX_DELAY_MS for nothing */
-       wait_time = (wake | signal_queue_len) ? 0 : compute_poll_timeout(exp);
+       /* Now let's wait for polled events. */
+       wait_time = wake ? 0 : compute_poll_timeout(exp);
        fd = global.tune.maxpollevents;
        clock_entering_poll();
 
@@ -193,7 +191,7 @@ static void _do_poll(struct poller *p, int exp, int wake)
                }
                if (timeout || !wait_time)
                        break;
-               if (signal_queue_len || wake)
+               if (wake)
                        break;
                if (tick_isset(exp) && tick_is_expired(exp, now_ms))
                        break;

diff --git a/src/ev_poll.c b/src/ev_poll.c
index 6fefb3ab3fcb97c28f4ff3b203ff672b0692f8ed..e98630c7180797ba6e4a9aa83acc3a214a5efbab 100644 (file)
--- a/src/ev_poll.c
+++ b/src/ev_poll.c
@@ -204,10 +204,8 @@ static void _do_poll(struct poller *p, int exp, int wake)
                }
        }
 
-       /* Now let's wait for polled events.
-        * Check if the signal queue is not empty in case we received a signal
-        * before entering the loop, so we don't wait MAX_DELAY_MS for nothing */
-       wait_time = (wake | signal_queue_len) ? 0 : compute_poll_timeout(exp);
+       /* Now let's wait for polled events. */
+       wait_time = wake ? 0 : compute_poll_timeout(exp);
        clock_entering_poll();
        status = poll(poll_events, nbfd, wait_time);
        clock_update_date(wait_time, status);

diff --git a/src/haproxy.c b/src/haproxy.c
index 2eb84bd87b9ae0af340ba01bd73d843ae470e88e..8d6f5876650746ccf060b36e33ff057aed379f81 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2810,7 +2810,7 @@ void run_poll_loop()
                if (killed > 1)
                        break;
 
-               /* expire immediately if events are pending */
+               /* expire immediately if events or signals are pending */
                wake = 1;
                if (thread_has_tasks())
                        activity[tid].wake_tasks++;
@@ -2821,6 +2821,10 @@ void run_poll_loop()
                        if (thread_has_tasks()) {
                                activity[tid].wake_tasks++;
                                _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING);
+                       } else if (signal_queue_len) {
+                               /* this check is required after setting TH_FL_SLEEPING to avoid
+                                * a race with wakeup on signals using wake_threads() */
+                               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING);
                        } else
                                wake = 0;
                }

diff --git a/src/signal.c b/src/signal.c
index 5ec71429c2df8e41adf249276c3663c58fed2fc0..3d7a9a89897e05fbdeabe0f737449f380194b173 100644 (file)
--- a/src/signal.c
+++ b/src/signal.c
@@ -56,6 +56,9 @@ void signal_handler(int sig)
        signal_state[sig].count++;
        if (sig)
                signal(sig, signal_handler); /* re-arm signal */
+
+       /* If the thread is TH_FL_SLEEPING we need to wake it */
+       wake_thread(tid);
 }
 
 /* Call handlers of all pending signals and clear counts and queue length. The