[PATCH v4.2] Cygwin: pipe: Switch pipe mode to blocking mode by defaut
Takashi Yano
takashi.yano@nifty.ne.jp
Fri Sep 20 14:09:17 GMT 2024
On Fri, 20 Sep 2024 22:24:14 +0900
Takashi Yano wrote:
> On Fri, 20 Sep 2024 18:23:35 +0900
> Takashi Yano wrote:
> > Hi Ken,
> >
> > On Wed, 18 Sep 2024 10:03:13 -0400
> > Ken Brown wrote:
> > > On 9/17/2024 2:22 PM, Ken Brown wrote:
> > > > On 9/17/2024 9:49 AM, Takashi Yano wrote:
> > > >>>> @@ -439,19 +396,45 @@ fhandler_pipe_fifo::raw_write (const void
> > > >>>> *ptr, size_t len)
> > > >>>> if (!len)
> > > >>>> return 0;
> > > >>>> - if (reader_closed ())
> > > >>>> + ssize_t avail = pipe_buf_size;
> > > >>>> + bool real_non_blocking_mode = false;
> > > >>>> + if (is_nonblocking ())
> > > >>>> {
> > > >>>> - set_errno (EPIPE);
> > > >>>> - raise (SIGPIPE);
> > > >>>> - return -1;
> > > >>>> + FILE_PIPE_LOCAL_INFORMATION fpli;
> > > >>>> + status = NtQueryInformationFile (get_handle (), &io, &fpli,
> > > >>>> sizeof fpli,
> > > >>>> + FilePipeLocalInformation);
> > > >>>> + if (NT_SUCCESS (status))
> > > >>>> + {
> > > >>>> + if (fpli.WriteQuotaAvailable != 0)
> > > >>>> + avail = fpli.WriteQuotaAvailable;
> > > >>>> + else /* WriteQuotaAvailable == 0 */
> > > >>>> + { /* Refer to the comment in select.cc:
> > > >>>> pipe_data_available(). */
> > > >>>> + /* NtSetInformationFile() in set_pipe_non_blocking(true)
> > > >>>> seems
> > > >>>> + to fail with STATUS_PIPE_BUSY if the pipe is not empty.
> > > >>>> + In this case, the pipe is really full if WriteQuotaAvailable
> > > >>>> + is zero. Otherwise, the pipe is empty. */
> > > >>>> + if (!((fhandler_pipe *)this)->set_pipe_non_blocking (true))
> > > >>>> + {
> > > >>>> + /* Full */
> > > >>>> + set_errno (EAGAIN);
> > > >>>> + return -1;
> > > >>>> + }
> > > >>>> + /* Restore the pipe mode to blocking. */
> > > >>>> + ((fhandler_pipe *)this)->set_pipe_non_blocking (false);
> > > >>>> + /* Pipe should be empty because reader is waiting the
> > > >>>> data. */
> > >
> > > One other thing that I missed in my first review. Is there a possible
> > > race condition here? What if the pipe is empty now but another writer
> > > fills the pipe before we try to write? Can that happen? If so, maybe
> > > it's safer to leave the pipe non-blocking instead of restoring it to
> > > blocking.
> >
> > Thanks!
> > Shouldn't we add mutex guard for raw_write() as well as raw_read()?
> >
> > I think I have addressed all the points you have raised. Could you
> > please check v5 patch?
>
> Bug in v5 has been fixed in v6.
v7: Improve error handling in raw_write()
--
Takashi Yano <takashi.yano@nifty.ne.jp>
More information about the Cygwin-patches
mailing list