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

[James Vega]

Bram,

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.

Thanks,
-- 
James
GPG Key: 1024D/61326D40 2003-09-02 James Vega <jamessan at debian.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: select_eintr.diff
Type: text/x-diff
Size: 732 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-vim-maintainers/attachments/20110906/7238e6db/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-vim-maintainers/attachments/20110906/7238e6db/attachment.pgp>