Re: wait_reading_process_ouput hangs in certain cases (w/ patches)

[Matthias Dahl]

Hello Paul...

On 13/11/17 20:44, Paul Eggert wrote:

> It cannot be for the entire process lifetime, since it's an unsigned
> integer that is designed to wrap around on overflow. So it sounds like
> you want a counter modulo 2**N for the entire process lifetime, and
> we're discussing what value of N is large enough for the purposes of
> wait_reading_process_output. Since EMACS_UINT might be only 32 bits, and
> it's eminently reasonable for a process to read more than 2**32 bytes
> during its lifetime, this is not a theoretical issue.
> 
> So: why must the counter contain more than a small number of bits (16,
> say)? I'm not seeing the scenario. I'm asking not because I think it's
> important to save a few bits in the counter: I'm asking because I want
> to understand the change and the reasoning behind it, so that I can be
> sure it's fixing the bug.

I am well aware that EMACS_UINT can be 32 bits wide and that the counter
could and will eventually wrap around, given the right process usage.

The scenario my patch(es) fix is the following: We have a wait_... call
with a waitproc set. Timers and filters run which can by themselves call
us recursively again, triggering read back from our own waitproc whilst
we would never notice it and stall waiting for it if no new data becomes
available.

Thus, with the current simple implementation, there is only one corner
case that will miss such read back nevertheless: If during an active
call to wait_... all recursive calls happen to read exactly 2**32 (or
whatever bit depths EMACS_UINT is) bytes back, then we will miss it
completely and stall.
If we read more than that, our calculation is off and we report less
bytes read back through got_some_output which is pretty much irrelevant.

Since I did not want to make any random assumptions about an upper limit
I decided to be on the safe side with >= 32 bits which, yes, is probably
overkill but arbitrarily setting a lower limit which might even wrap
around more often without any real gain, is also not the best thing to
do in my opinion.

Besides that, I also opted for the "bytes read during process lifetime"
metric because I thought it could turn out useful in the future while
debugging. A more synthetic metric like chunks read or in-flight bytes
read, not so much, imho.

What do you say?

So long,
Matthias

-- 
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu