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 code comment merely states "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." without backing up that claim
with a reference.
That's not good. I cannot give my blessing to your code change because you
haven't yet given me any reason to be confident in it. I must therefore
suspect that there are faults in it, based on the story the commit history
of `winsup/cygwin/fhandler/pipe.cc` tells.
Ciao,
Johannes
