[PATCH v2 0/6] fix pipe_command() deadlock

[Jeff King <peff@peff.net>]

On Wed, Aug 17, 2022 at 01:39:32AM -0400, Jeff King wrote:

> I wouldn't be opposed to that, in the sense that it's supposed to be a
> black box to the caller, and it's relatively small in size. But I think
> we're pretty close to having something usable without that, so I'd like
> to pursue a smaller fix in the interim.

So here's what I came up with. Most of the changes are done as
preparatory steps, so I believe the final commit should just work out of
the box for you, as well as on Windows CI (which I just kicked off).

Here's a breakdown of the patches:

  [1/6]: compat: add function to enable nonblocking pipes
  [2/6]: nonblock: support Windows

This is my compat wrapper from v1 pulled into a separate function, and
then your Windows implementation on top.

  [3/6]: git-compat-util: make MAX_IO_SIZE define globally available
  [4/6]: pipe_command(): avoid xwrite() for writing to pipe

This fixes the "loop forever" problem of using xwrite(), under the
assumption that fixing our Windows poll() emulation is too hard. If we
did fix it, then we could revert these (though keeping them can be
thought of as a defensive measure against future bugs; busy-looping is
less bad than deadlocking).

I _think_ the amount of busy-looping on Windows will be OK here. Any
case that wouldn't deadlock currently is unaffected, and the cases that
are will probably not be too wasteful. More discussion in the commit
message.

  [5/6]: pipe_command(): handle ENOSPC when writing to a pipe

A hack around the ENOSPC problem you found. We could replace this with
your mingw_write() patch, but it looked like that needed some corner
cases to be smoothed a bit.

  [6/6]: pipe_command(): mark stdin descriptor as non-blocking

The actual fix, largely as before.

I don't love patches 3-5, but it seems like a practical path forward.
I'd prefer to go with a minimal fix that removes the existing deadlock,
and then we can do more cleanups on top. Thanks so much for all of your
investigation and testing so far!

 Makefile                   |  1 +
 compat/nonblock.c          | 50 ++++++++++++++++++++++++++++++++++++++
 compat/nonblock.h          |  9 +++++++
 git-compat-util.h          | 22 +++++++++++++++++
 run-command.c              | 33 +++++++++++++++++++++----
 t/t3701-add-interactive.sh | 13 ++++++++++
 wrapper.c                  | 22 -----------------
 7 files changed, 123 insertions(+), 27 deletions(-)
 create mode 100644 compat/nonblock.c
 create mode 100644 compat/nonblock.h

-Peff