Hi Cyril, On Mon, Mar 20, 2017 at 11:19:37PM +0100, Cyril Bonté wrote: > > 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. > > I agree, while I modified the code, I didn't like having to add some new > tests in run_pool_loop(). Using a new task does not seem to make the code > too complex. It may even be clearer. > I already have something usable but I want to spend some more imes to test > some cases.
OK, do not hesitate to ask if you're unsure about certain parts. > > > + 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. > > I'm not sure, because "next" is modified only if it's greater than "grace". > The goal was exactly the contrary to wake up earlier if the grace time > triggers before "next". Ah yes indeed you're right. That's a good example of something not really easy to get at first glance that we have to be careful about. > But using a task, this is now obsolete and less error-prone :-) Definitely! Cheers, Willy
