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 <james...@debian.org>
diff --git a/src/os_unix.c b/src/os_unix.c --- a/src/os_unix.c +++ b/src/os_unix.c @@ -5057,6 +5057,7 @@ /* * Select on ready for reading and exceptional condition (end of file). */ +select_eintr: FD_ZERO(&rfds); /* calls bzero() on a sun */ FD_ZERO(&efds); FD_SET(fd, &rfds); @@ -5117,6 +5118,10 @@ # else ret = select(maxfd + 1, &rfds, NULL, &efds, tvp); # endif +# ifdef EINTR + if (ret == -1 && errno == EINTR) + goto select_eintr; +# endif # ifdef __TANDEM if (ret == -1 && errno == ENOTSUP) { @@ -5124,7 +5129,7 @@ FD_ZERO(&efds); ret = 0; } -#endif +# endif # ifdef FEAT_MZSCHEME if (ret == 0 && mzquantum_used) /* loop if MzThreads must be scheduled and timeout occurred */
signature.asc
Description: Digital signature