Re: [PATCH 1/2] daemon: use timeout for uninterruptible poll

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 15, 2018, 22:16 UTC
Message-ID: <xmqqvacsyv1y.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <xmqq36zw16gv.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

> I think you identified the problem and diagnosed it correctly, but I
> find that the change proposed here introduces a severe layering
> violation.  The code is still calling what is called poll(), which
> should not have such a broken semantics.

I only mentioned a piece of fact (i.e. "the code calls poll() after the patch"), but I guess I should have made it clear what makes that a bad thing. Future readers of the code in daemon.c are required to be aware of the limitation of some poll() emulation; they cannot "optimize" out and made the code unware of the (non-)existence of remaining children, for example. When the callsite uses poll(), those who know how poll() ought to work won't be. The reason why the xpoll() I mentioned as a possible alternative would be better is because they will learn why we do not use normal poll() there and why we maintain and pass live_children (and those who cut and paste without understanding the existing code _will_ copy the calling site of xpoll(), which will automatically copy the need to maintain the number of remaining children ;-).