On Sep  2 21:00, Corinna Vinschen wrote:
> On Sep  2 09:01, Ken Brown wrote:
> > On 9/2/2021 4:17 AM, Corinna Vinschen wrote:
> > > What if the readers never request more than, say, 50 or even 25% of the
> > > available buffer space?  Our buffer is 64K and there's no guarantee that
> > > any read > PIPE_BUF (== 4K) is atomic anyway.  This can work without
> > > having to check the other side of the pipe.  Something like this,
> > > ignoring border cases:
> > > 
> > > pipe::create()
> > > {
> > >     [...]
> > >     mutex = CreateMutex();
> > > }
> > > 
> > > pipe::raw_read(char *buf, size_t num_requested)
> > > {
> > >    if (blocking)
> > >      {
> > >        WFSO(mutex);
> > >        NtQueryInformationFile(FilePipeLocalInformation);
> > >        if (!fpli.ReadDataAvailable
> > > 	  && num_requested > fpli.InboundQuota / 4)
> > > 	num_requested = fpli.InboundQuota / 4;
> > >        NtReadFile(pipe, buf, num_requested);
> > >        ReleaseMutex(mutex);
> > >      }
> > > }
> > > 
> > > It's not entirely foolproof, but it should fix 99% of the cases.
> > 
> > I like it!
> > 
> > Do you think there's anything we can or should do to avoid a deadlock in the
> > rare cases where this fails?  The only thing I can think of immediately is
> > to always impose a timeout if select is called with infinite timeout on the
> > write side of a pipe, after which we report that the pipe is write ready.
> > After all, we've lived since 2008 with a bug that caused select to *always*
> > report write ready.
> 
> Indeed.  Hmm.  What timeout are you thinking of?  Seconds?  Minutes?
> 
> > Alternatively, we could just wait and see if there's an actual use case in
> > which someone encounters a deadlock.
> 
> Or that.  Fixing up select isn't too hard in that case, I guess.

It's getting too late again.  I drop off for tonight, but I attached
my POC code I have so far.  It also adds the snippets from my previous
patch which fixes stuff Takashi found during testing.  It also fixes
something which looks like a bug in raw_write:

-	  ptr = ((char *) ptr) + chunk;
+	  ptr = ((char *) ptr) + nbytes_now;

Incrementing ptr by chunk bytes while only nbytes_now have been written
looks incorrect.

As for the reader, it makes the # of bytes to read dependent on the
number of reader handles.  I don't know if that's such a bright idea,
but this can be changed easily.

Anyway, this runs all my testcases successfully but they are anything
but thorough.

Patch relativ to topic/pipe attached.  Would you both mind to take a
scrutinizing look?


Thanks,
Corinna