On 06/14/15 21:11, Laurent Bercot wrote: > On 14/06/2015 14:37, Olivier Brunel wrote: >> Because of the buffered IO, the possible scenario could occur: >> - netlink uevents (plural) occur, i.e. data ready on stdin >> - iopause triggered, handle_stdin() called. The first uevent is >> processed, child >> launched, we're waiting for a signal >> - SIGCHLD occurs, we're back to iopausing on stdin again, only it's >> not ready >> yet; Because we've read it all already and still have unprocessed data >> (uevents) on our own internal buffer (buffer_0) > > Right, thanks for the catch. I usually avoid that trap, but meh. > I committed a simpler change than your patch, please tell me if it fixes > things for you.
Haven't tested it, but I'm sure it'll "work". In fact I had done something similar at first, but changed it because I don't think this is quite correct. That is, in your test now you're using x[1] even though it might not have been used in the iopause call before, so while I guess this isn't random memory, it doesn't really feel right either. Moreover, imagine this very likely scenario: - uevent comes in, handle_stdin is called, children forked, and we're iopausing in x[0] (selfpipe) only. - SIGCHLD occurs, handle_signals is called, then you test on x[1] which should still be as it was last time it was actually used, so stating stdin is readable even though it isn't anymore... and we're gonna block in handle_stdin. Not that it's a big deal since at this point we were gonna iopause to wait for that fd to become readable anyways, since we're not expecting any signal (as we don't have children anymore). So in effect this works basically the same, but you're using memory (x[1]) that you probably shouldn't, and effectively trying to read (and blocking) in handle_stdin despite having an iopause in place. You might as well change your test to "if (cont) handle_stdin(...) ;" then, it'll surely work just as well. But it doesn't feel right, to me at least, since we have a loop w/ iopause, hence why I went with the bit longer (and maybe less elegant) version. -j
