This patch exists in the git-for-windows fork of msys2-runtime (which is
itself a fork of cygwin).  There have been complaints and requests to
apply this patch to msys2-runtime (such as
https://github.com/msys2/MSYS2-packages/issues/4962), but it makes more
sense to me to try to figure this out upstream, so everyone can benefit.
I did not write the patch, nor do I personally encounter the bug it is
intended to fix, so I can't really advocate for its approach, but the
commit message is pretty detailed as to the investigation that led to it.

This is the original patch from
https://github.com/git-for-windows/msys2-runtime/pull/75, if necessary I
can rebase it on cygwin's master branch, it does so cleanly (or you can
try git am -3, that tends to work for me in cases where straight git am
does not).

-- >8 --
>From cbe555e054cefeccd65250bb11dc56f82196301f Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schinde...@gmx.de>
Date: Thu, 10 Oct 2024 19:52:47 +0200
Subject: [PATCH] Fix SSH hangs

It was reported in https://github.com/git-for-windows/git/issues/5199
that as of v3.5.4, cloning or fetching via SSH is hanging indefinitely.

Bisecting the problem points to 555afcb2f3 (Cygwin: select: set pipe
writable only if PIPE_BUF bytes left, 2024-08-18). That commit's
intention seems to look at the write buffer, and only report the pipe as
writable if there are more than one page (4kB) available.

However, the number that is looked up is the number of bytes that are
already in the buffer, ready to be read, and further analysis
shows that in the scenario described in the report, the number of
available bytes is substantially below `PIPE_BUF`, but as long as they
are not handled, there is apparently a dead-lock.

Since the old logic worked, and the new logic causes a dead-lock, let's
essentially revert 555afcb2f3 (Cygwin: select: set pipe writable only if
PIPE_BUF bytes left, 2024-08-18).

Note: This is not a straight revert, as the code in question has been
modified subsequently, and trying to revert the original commit would
cause merge conflicts. Therefore, the diff looks very different from the
reverse diff of the commit whose logic is reverted.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
---
 winsup/cygwin/select.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index bc02c3f9d4..2c09b14d1c 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -776,7 +776,7 @@ peek_pipe (select_record *s, bool from_select)
        }
       ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT | PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
-      gotone += s->write_ready = (n >= PIPE_BUF);
+      gotone += s->write_ready = (n > 0);
       if (n < 0 && s->except_selected)
        gotone += s->except_ready = true;
     }
@@ -990,7 +990,7 @@ peek_fifo (select_record *s, bool from_select)
       ssize_t n = pipe_data_available (s->fd, fh, fh->get_handle (),
                                       PDA_SELECT | PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
-      gotone += s->write_ready = (n >= PIPE_BUF);
+      gotone += s->write_ready = (n > 0);
       if (n < 0 && s->except_selected)
        gotone += s->except_ready = true;
     }
@@ -1416,7 +1416,7 @@ peek_pty_slave (select_record *s, bool from_select)
     {
       ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT | PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
-      gotone += s->write_ready = (n >= PIPE_BUF);
+      gotone += s->write_ready = (n > 0);
       if (n < 0 && s->except_selected)
        gotone += s->except_ready = true;
     }

Reply via email to