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

[Carl Edquist]

On Fri, 9 Dec 2022, Arsen Arsenović wrote:

> > Similar to the situation here, i was seeing things annoyingly look like 
> > they are still 'alive' longer than they ought to be when providing 
> > input from the terminal.
>
> Huh, I never tried that, honestly.

Here is a simple example:

    exec 3<> /dev/tcp/example.com/80

    echo_crlf () { printf "%s\r\n" "$@"; }

    { echo_crlf "GET / HTTP/1.1"
      echo_crlf "Host: example.com"
      echo_crlf "Connection: close"
      echo_crlf ""
    } >&3

    cat <&3


In this example, i'm sending the input to the socket (on fd 3) directly 
using the 'printf' builtin and shell redirection, and i request the server 
close its side of the connection after this request.

But if i want to type the request by hand, i might do:

    sed -u 's/$/\r/' >&3

to read from the terminal, convert line terminators to CRLF, and write to 
the socket.  Only, similar to the situation with tee and pipes, sed here 
will not detect when the remote read end of the socket has closed, so it 
hangs waiting for more input.

> Did polling help your use-case?

Mostly no, as i kind of got into later last time.  It seems the semantics 
are different for sockets than pipes; for some reason it seems the socket 
doesn't behave like a broken pipe until after the remote read end shuts 
down and then another write is made to the local end.  Only after these 
does poll() seem to detect the broken-pipe -like state.

> > Oh interesting.  That wasn't on my radar at all.  I guess this means 
> > that when cross-compiling, the configure script is run on the 
> > cross-compiling host, rather than on the target platform; so any test 
> > programs in configure.ac with AC_RUN_IFELSE don't necessarily check the 
> > target platform functionality (?)
>
> Or worse, is unable to run at all (and always fails), if the binary is 
> for a different kernel or architecture.

Ok, so i guess for the sake of cross-compiling we are limited to 
compile/link checks.

> > Originally i had imagined (or hoped) that this broken-pipe detection 
> > could also be used for sockets (that was how the issue came up for me), 
> > but it seems the semantics for sockets are different than for pipes.
>
> This might require POLLPRI or POLLRDHUP or such.  Can you try with those 
> to the set of events in pollfd?

Oh interesting!  I had assumed these wouldn't help - POLLPRI is for OOB 
data, and, I had assumed POLLRDHUP was only for polling the read side of a 
socket (thus POLL*RD*HUP), to determine if the remote write end was 
shutdown.  But you are right, poll() returns with POLLRDHUP in revents as 
soon as the remote end is closed.  Thanks for the tip!

I assume the reason this works is, even though i have in mind that i am 
monitoring the socket as an output, the socket serves as an input also 
(it's open for RW).  So, if my interpretation is correct, POLLRDHUP is 
actually monitoring the local read side of the socket.  And so, poll() is 
actually detecting the remote write side getting shutdown.

This is not technically the same thing as monitoring the local output 
side, but if the remote end of the socket closes entirely (shutting down 
the remote read and write ends together), then that tells us about the 
local output side as well.

This definitely seems to work for the case i was playing with, though i'm 
not sure if it would behave as intended if the remote side only shut down 
its read or write end (but not both).

Also, my bits/poll.h seems to suggest this is a Linux extension:

#ifdef __USE_GNU
/* These are extensions for Linux.  */
# define POLLMSG    0x400
# define POLLREMOVE 0x1000
# define POLLRDHUP  0x2000
#endif


Anyway, this is all Good To Know.  I don't know what the semantics with 
poll() for sockets are supposed to be in the general case (beyond Linux), 
so i don't feel i'm in a good position to advocate for it in coreutils. 
But maybe someone who knows better can comment on this topic.

> > A quick note, this check only needs to be done a total of once per 
> > output, it shouldn't be done inside iopoll(), which would result in an 
> > additional redundant fstat() per read().
>
> Could this be handled by get_next_out?

Sure, either in that function or immediately after it gets called.  But 
also once for stdout before the while (n_outputs) loop.  Alternatively, 
allocate a file type array and populate it in the for loop that does all 
the fopen() calls.

Either way, the idea would be to then replace

        if (pipe_check)

with something like

        if (pipe_check && first_out_is_pipe)

or (with a file type array)

        if (pipe_check && S_ISFIFO (output_types[first_out]))


> > ... Also, i suspect that the pipe_check option can be disabled if the 
> > _input_ is a regular file (or block device), since (i think) these 
> > always show up as ready for reading.  (This check would only need to be 
> > done once for fd 0 at program start.)
>
> Yes, there's no point poll-driving those, since it'll be always 
> readable, up to EOF, and never hesitate to bring more data.  It might 
> just end up being a no-op if used in current form (but I haven't tried).

Well currently if the input is a regular file, poll() immediately returns 
POLLIN, along with any potential errors for the output.  But yes the net 
effective behavior is the same as if the poll() call had been skipped.