Bug#627376: [patch] Ensure all of child process' output is read

[Bram Moolenaar]

James -

> As described below, when 'noshelltemp' is set Vim sometimes doesn't read
> all of the output from the read pipe of a child process.  This can be
> particularly problematic when the child process is being used as a
> filter, BufReadCmd, etc. since it can cause unnoticed data loss.
> 
> On Fri, May 20, 2011 at 03:07:35AM -0500, Alan Curry wrote:
> > This is a timing-sensitive bug, hard to reproduce. But after running into it
> > a few times, I finally came up with a simple test case that does the wrong
> > thing consistently for me. I hope this procedure makes it possible for others
> > to see it too:
> > 
> > Start up vim. :set noshelltmp. Insert the following long line into the edit
> > buffer:
> > 
> > sleep 3;printf '%s\n' xxxxxxx000 xxxxxxx001 xxxxxxx002 xxxxxxx003 xxxxxxx004 xxxxxxx005 xxxxxxx006 xxxxxxx007 xxxxxxx008 xxxxxxx009 xxxxxxx010 xxxxxxx011 xxxxxxx012 xxxxxxx013 xxxxxxx014 xxxxxxx015 xxxxxxx016 xxxxxxx017 xxxxxxx018
> > 
> > Run !!sh on that line. The command's output is truncated, with only a couple
> > of x's on the last line instead of xxxxxxx018. If you don't see the bug, try
> > a few more times, run a CPU-hogging program at the same time, and strace the
> > vim process. Anything that shakes up the timing can help bring out the bug.
> 
> From what I can tell, this is being caused by os_unix.c:RealWaitForChar's
> select getting interrupted after the child process has been wait()ed
> (i.e., the wait_pid == pid check on line 4622 is true).  Because of
> that, we never loop around again to finish performing the read.
> 
> The attached patch handles the problem by calling select again if it
> returned due to an interrupt.  Handling EINTR has to be done by jumping
> back to before the FD_SET calls, since the fd_sets are invalid after an
> interrupt.
> 
> I also took a look at the other uses of select(2) in the codebase and,
> from what I could tell, none of them are subject to the same error
> scenario.  They all seem to be either used as a granular sleep or are in
> a path where they'll get called again anyway.

I can't reproduce it.  My system probably is too fast.

I do understand that this can happen.  I suppose the loop won't happen
because MAY_LOOP is not defined.

Your solution appears to be OK, except when msec is set.  Then it may
wait too long.  A check for msec fixes that.  But it looks like 10 msec
is used for the call.  Even when that time has passed we still want to
check for characters being available.  Well, waiting too long is not
that bad.

- Bram

-- 
An easy way to determine if you have enough teamwork to be doomed is simply to
measure how long it takes from the time you decide to go to lunch together
until the time you actually eat.
				(Scott Adams - The Dilbert principle)

 /// Bram Moolenaar -- Bram at Moolenaar.net -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org        ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///