[PATCH v4.2] Cygwin: pipe: Switch pipe mode to blocking mode by defaut

Takashi Yano takashi.yano@nifty.ne.jp
Fri Sep 20 09:23:35 GMT 2024


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?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>


More information about the Cygwin-patches mailing list