Kim Gybels <kgybels@xxxxxxxxxxxx> writes: >> > In other words, you scolded Kim for something that this patch did not >> > introduce, but which was already there. > > I didn't feel scolded, just Junio raising a concern about maintainability of > the code. FWIW, I didn't mean to scold, either. Rather I was pointing out that the code already maintains the number of remaining children, which means that a more portable abstraction than poll(), if we desired to have one, would merely be one step away, as we already know that at least need that information to help Windows. > There is another issue with the existing code that this new > "xpoll" will need to take into account. If a SIGCHLD arrives > between the call to check_dead_children and poll, the poll will > not be interupted by it, resulting in the child not being reaped > until another child terminates or a client connects. Currently, > the effect is just a zombie process for a longer time, however, > the proposed patch (daemon: graceful shutdown of client > connection) relies on the cleanup to close the client connection. Good analysis. That consideration may mean that xpoll() as I suggested is useless as a possible more portable abstraction (or, "an abstraction that is implementable easily in both POSIX and non-POSIX world"), but I suspect we would still want to have an internal "portable" API that serves the purpose similar to how we wanted POSIX poll() to serve. The place the patch is touching is not the only place poll() is used in the codebase, and other places (and future ones we would add) may benefit from having one. Thanks for being constructive.
