Hi Cyril,
A few comments below. First, I'd prefer not to add anything in
run_poll_loop() since we have to pass there a lot of times and it's on
the critical path. Instead I think we could create a task upon receipt
of SIGUSR1 that will either kill all connections or simply exit after
expiration (and emit a log). This can still be discussed, if the resulting
code is more fragile or more complex, but that's a general principle I
mention here.
On Thu, Mar 16, 2017 at 12:44:40AM +0100, Cyril Bonté wrote:
> @@ -1585,6 +1587,12 @@ static void run_poll_loop()
>
> /* Check if we can expire some tasks */
> next = wake_expired_tasks();
> + if (stopping && (grace != TICK_ETERNITY)) {
You can use tick_isset(grace) instead of comparing with TICK_ETERNITY.
> + if (tick_is_expired(grace, now_ms))
> + break;
> + if (next == TICK_ETERNITY || tick_is_expired(next,
> grace))
> + next = grace;
> + }
I could be wrong but I fear that it can cause some longer poll() pauses
because here you're postponing "next" if it was supposed to trigger
earlier. You should have used tick_first(next, grace), this would give
something like this :
if (unlikely(stopping)) {
if (tick_is_expired(grace, now_ms))
break;
next = tick_first(next, grace);
}
> /* stop when there's nothing left to do */
> if (jobs == 0)
> diff --git a/src/proxy.c b/src/proxy.c
> index c76d55d5..024b36fa 100644
> --- a/src/proxy.c
> +++ b/src/proxy.c
> @@ -925,6 +925,8 @@ void soft_stop(void)
> struct peers *prs;
>
> stopping = 1;
> + if (global.grace != TICK_ETERNITY)
> + grace = tick_add(now_ms, global.grace);
You can use the following which does these two operations :
grace = tick_add_ifset(now_ms, global.grace);
Cheers,
Willy
