Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

[Carl Edquist]

On Fri, 2 Dec 2022, Carl Edquist wrote:

> > > Although tee has multiple outputs, you only need to monitor a single 
> > > output fd at a time.  Because the only case you actually need to catch 
> > > is when the final valid output becomes a broken pipe.  (So I don't 
> > > think it's necessary to poll(2) all the output fds together.)
> >
> > That is technically true, but I think coupling this to two FDs might 
> > prove a bit inelegant in implementation (since you'd have to decide 
> > which entry from an unorganized list with holes do you pick?  any of 
> > them could spontaneously go away), so I'm not sure the implementation 
> > would be cleaner that way.
>
> It'd be best to explain with a patch - I'll plan to send one later for a 
> concrete example.

Ok it's patch time.  Attached!  Feedback welcome.


A few notes:


Originally I had in mind to put the read() call inside the poll() loop. 
But if we keep this feature as an option, it felt it was a bit easier just 
to add the "if (pipe_check) {...}" block before the read().


The new iopoll() function is the core concept here for waiting on an 
[input, output] pair of fds - waiting for the input to become ready to 
read, or the output to have an error or become a broken pipe.


For Pádraig, I think the same function & approach here could be used in 
other filters (cat for example).  The stubborn part of me might say, for 
platforms that do not natively support poll(2), we could simply leave out 
support for this feature.  If that's not acceptable, we could add a 
select(2)-based fallback for platforms that do not have a native poll(2).


Unique to tee is its multiple outputs.  The new get_next_out() helper 
simply advances to select the next remaining (ie, not-yet-removed) output. 
As described last time, it's sufficient to track a single output at a 
time, and perhaps it even simplifies the implementation.  It also avoids 
the need for a malloc() for the pollfd array before every read().


I moved the somewhat complicated write-failure logic from tee_files() out 
to a new helper function, fail_output(), which now also gets called for 
broken pipes that we want to remove.


Note also that I make -P imply -p.  I think this is necessary, otherwise 
an output pipe becoming broken always produces an error.  But normally, an 
output pipe breaking does not necessarily produce an error, since EOF can 
arrive before any further input, and in that case no write is then 
attempted into the broken pipe.



Happy hacking / feedback welcome.


Thanks,
Carl

tee-pipe-check-iopoll.patch
Description: Text Data