[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