On Fri, 3 Sep 2021 19:00:46 +0900
Takashi Yano wrote:
> On Thu, 2 Sep 2021 21:35:21 +0200
> Corinna Vinschen wrote:
> > 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.
> 
> Your code seems that read() returns only the partial data even
> if the pipe stil has more data. Is this by design?
> 
> This happes in both blocking and non-blocking case.

The patch attached seems to fix the issue.

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