On Wed, 25 Jun 2025 17:56:35 +0200 (CEST)
Johannes Schindelin wrote:
> Hi Takashi,
>
> On Wed, 25 Jun 2025, Takashi Yano wrote:
>
> > On Wed, 25 Jun 2025 14:07:15 +0200 (CEST)
> > Johannes Schindelin wrote:
> > > Hi Takashi,
> > >
> > > On Wed, 25 Jun 2025, Takashi Yano wrote:
> > >
> > > > On Wed, 25 Jun 2025 19:55:34 +0900
> > > > Takashi Yano wrote:
> > > > >
> > > > > On Wed, 25 Jun 2025 09:38:17 +0200 (CEST)
> > > > > Johannes Schindelin wrote:
> > > > > >
> > > > > > On Wed, 25 Jun 2025, Johannes Schindelin wrote:
> > > > > >
> > > > > > > On Wed, 25 Jun 2025, Takashi Yano wrote:
> > > > > > >
> > > > > > > > I'd revise the patch as follows. Could you please test if the
> > > > > > > > following patch also solves the issue?
> > > > > > >
> > > > > > > Will do.
> > > > > >
> > > > > > For the record, in my tests, this fixed the hangs, too.
> > > > >
> > > > > Thanks for testing.
> > > > > However, I noticed that this patch changes the behavior Corinna was
> > > > > concerned about.
> > > >
> > > > The behaviour change can be checked using attached test case.
> > >
> > > I do not understand what this undocumented code is trying to demonstrate,
> > > not without any explanation.
> > >
> > > Could you rework it so that it becomes a proper test in the test suite
> > > that verifies that Cygwin behaves as desired, please?
> >
> > What the comment in the source code says:
> >
> > /* NtWriteFile returns success with # of bytes written == 0 if writing
> > on a non-blocking pipe fails because the pipe buffer doesn't have
> > sufficient space.
> >
> > POSIX requires
> > - A write request for {PIPE_BUF} or fewer bytes shall have the
> > following effect: if there is sufficient space available in the
> > pipe, write() shall transfer all the data and return the number
> > of bytes requested. Otherwise, write() shall transfer no data and
> > return -1 with errno set to [EAGAIN].
> >
> > - A write request for more than {PIPE_BUF} bytes shall cause one
> > of the following:
> >
> > - When at least one byte can be written, transfer what it can and
> > return the number of bytes written. When all data previously
> > written to the pipe is read, it shall transfer at least {PIPE_BUF}
> > bytes.
> >
> > - When no data can be written, transfer no data, and return -1 with
> > errno set to [EAGAIN]. */
> >
> > /* Independent of being blocking or non-blocking, if we're here,
> > the pipe has less space than requested. If the pipe is a
> > non-Cygwin pipe, just try the old strategy of trying a half
> > write. If the pipe has at
> > least PIPE_BUF bytes available, try to write all matching
> > PIPE_BUF sized blocks. If it's less than PIPE_BUF, try
> > the next less power of 2 bytes. This is not really the Linux
> > strategy because Linux is filling the pages of a pipe buffer
> > in a very implementation-defined way we can't emulate, but it
> > resembles it closely enough to get useful results. */
>
> I do not understand what part of that code comment refers to either
> documented behavior or to a thorough test you performed. To the contrary,
The point is that when avail < PIPE_BUF, wirte() doesn't fill all the
available space. This behaviour is a mimic of Linux.
With the patchs
https://github.com/git-for-windows/git/issues/5688#issuecomment-2996103559
https://cygwin.com/pipermail/cygwin-patches/2025q2/013897.html
write() fills all available pipe space. That breaks the code intent,
regardless of whether the behaviour is correct thing.
With cygwin 3.6.3, the result of check-non-blocking-write.c is
w:65536
r:2048
w:1
w:-1
w:-1
w:1024
w:512
w:256
w:128
w:64
w:32
w:16
w:8
w:4
w:2
w:-1
while the result with above patches is
w:65536
r:2048
w:1
w:2047
w:-1
w:-1
w:-1
w:-1
w:-1
w:-1
w:-1
w:-1
w:-1
w:-1
w:-1
w:-1
And with nga888's patch, the result is
w:65536
r:2048
w:1
w:-1
w:-1
w:1024
w:512
w:256
w:128
w:64
w:32
w:16
w:8
w:4
w:2
w:-1
where the intent of the code is kept.
--
Takashi Yano <[email protected]>
