@@ -1364,12 +1364,24 @@ static int pump_io_round(struct io_pump *slots, int nr, struct pollfd *pfd)
continue;
if (io->type == POLLOUT) {
- ssize_t len = xwrite(io->fd,
- io->u.out.buf, io->u.out.len);
+ ssize_t len;
+
+ /*
+ * Don't use xwrite() here. It loops forever on EAGAIN,
+ * and we're in our own poll() loop here.
+ *
+ * Note that we lose xwrite()'s handling of MAX_IO_SIZE
+ * and EINTR, so we have to implement those ourselves.
+ */
+ len = write(io->fd, io->u.out.buf,
+ io->u.out.len <= MAX_IO_SIZE ?
+ io->u.out.len : MAX_IO_SIZE);
if (len < 0) {
- io->error = errno;
- close(io->fd);
- io->fd = -1;
+ if (errno != EINTR && errno != EAGAIN) {
+ io->error = errno;
+ close(io->fd);
+ io->fd = -1;
+ }
} else {
io->u.out.buf += len;
io->u.out.len -= len;
If xwrite() sees an EAGAIN response, it will loop forever until the write succeeds (or encounters a real error). This is due to ef1cf0167a (xwrite: poll on non-blocking FDs, 2016-06-26), with the idea that we won't be surprised by a descriptor unexpectedly set as non-blocking. But that will make things awkward when we do want a non-blocking descriptor, and a future patch will switch pipe_command() to using one. In that case, looping on EAGAIN is bad, because the process on the other end of the pipe may be waiting on us before doing another read() on the pipe, which would mean we deadlock. In practice we're not supposed to ever see EAGAIN here, since poll() will have just told us the descriptor is ready for writing. But our Windows emulation of poll() will always return "ready" for writing to a pipe descriptor! This is due to 94f4d01932 (mingw: workaround for hangs when sending STDIN, 2020-02-17). Our best bet in that case is to keep handling other descriptors, as any read() we do may allow the child command to make forward progress (i.e., its write() finishes, and then it read()s from its stdin, freeing up space in the pipe buffer). This means we might busy-loop between poll() and write() on Windows if the child command is slow to read our input, but it's much better than the alternative of deadlocking. In practice, this busy-looping should be rare: - for small inputs, we'll just write the whole thing in a single write() anyway, non-blocking or not - for larger inputs where the child reads input and then processes it before writing (e.g., gpg verifying a signature), we may make a few extra write() calls that get EAGAIN during the initial write, but once it has taken in the whole input, we'll correctly block waiting to read back the data. - for larger inputs where the child process is streaming output back (like a diff filter), we'll likewise see some extra EAGAINs, but most of them will be followed immediately by a read(), which will let the child command make forward progress. Of course it won't happen at all for now, since we don't yet use a non-blocking pipe. This is just preparation for when we do. Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: Jeff King <peff@peff.net> --- run-command.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)