[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