SIGIO with si_code==POLL_HUP on read pipe FD with no writers?

* SIGIO with si_code==POLL_HUP on read pipe FD with no writers?
@ 2022-09-05 22:03 Jason Vas Dias
  2022-09-08  1:54 ` Jason Vas Dias
  0 siblings, 1 reply; 2+ messages in thread
From: Jason Vas Dias @ 2022-09-05 22:03 UTC (permalink / raw)
  To: David Howells; +Cc: linux-kernel, jason.vas.dias


Good day -

    To the last committer & maintainers of 'linux/fs/pipe.c' :

    Why isn't a SIGIO signal with POLL_HUP in 'si_code'
    raised on an O_ASYNC, F_SETOWN{,_EX} pid F_SETSIG
    signal owning pipe read file descriptor ?

    All that happens when the write end of a pipe is
    closed is that a SIGIO gets raised with the
    (struct siginfo* parameter)->si_code set
    to 1 ( POLL_IN ) , and then 
     ioctl( fd, FIONREAD, &sz)
    then returns with sz==0 for that fd ;
    a read() on that fd would then return 0.

    Looking at pipe.c, the situation of no pipe writers
    is detected and revents is set to contain EPOLLHUP
    ONLY in pipe_poll(), not pipe_read() .

    pipe_read() (in version 5.19) DOES detect the
    no writers situation :

    fs/pipe.c, @line 255:
	for (;;) {
		/* Read ->head with a barrier vs post_one_notification() */
        ...
    @line 341:
		if (!pipe->writers)
			break;
        ...

    It would be quite easy to add after the pipe_read() loop quits a clause as in
    pipe_poll() , @ line 677:
	mask = 0;
	if (filp->f_mode & FMODE_READ) {
		if (!pipe_empty(head, tail))
			mask |= EPOLLIN | EPOLLRDNORM;
		if (!pipe->writers && filp->f_version != pipe->w_counter)
			mask |= EPOLLHUP;
	}
   
    which does something like :

	if ( !pipe->writers )
		kill_fasync(&pipe->fasync_readers, SIGIO, POLL_HUP);

    
    It is not nice to have to GUESS that just because 
        ioctl(fd, FIONREAD, &sz) 
    returns with sz==0 immediately after a POLL_IN event,
    that the pipe in fact has no writers, because the
    signal could be blocked when the ioctl call happens.

    And if one happens not to try to read 0 bytes from the pipe,
    then one would never know that no writers exist on it, and
    could pause() infinitely waiting for a signal.
    Or why should I have to put the FD into O_NONBLOCK mode
    (which mine was not in) and attempt a read to return
    0 bytes, when I know 0 bytes are available to read ?
    
    OR, maybe in pipe_write(), @ line 595 where it does :

    	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
        
    (which is probably where the FINAL POLL_IN signal originates)     
    it could instead do:
        kill_fasync(&pipe->fasync_readers, SIGIO, 
                    ((ret==0) && (pipe->fasync_writers <= 1))
                    ? POLL_HUP
                    : POLL_IN
                   );

     It seems there are several easy ways to fix this and
     I believe that it would make processes wanting to
     read pipes using SIGIO much more robust & simple to code.

     Processes would still be able to rely on read()s returning
     0 in this case, but please, why can't SIGIO using processes
     also get a definitive SIGIO with si_code==POLL_HUP, not POLL_IN ?

     There appears to be similar logic that does send
     a final POLL_HUP SIGIO when the remote write end of
     a readable socket closes - why not for pipes ?

     And the sigaction manual page states:
     "
       The  following values can be placed in si_code for a SIGIO/SIGPOLL sig‐
       nal:

           POLL_IN
                  Data input available.

           POLL_OUT
                  Output buffers available.

           POLL_MSG
                  Input message available.

           POLL_ERR
                  I/O error.

           POLL_PRI
                  High priority input available.

           POLL_HUP
                  Device disconnected.
     "
     which suggests that these events should be raised for all devices -
     it does not mention any special cases for pipe file descriptors,
     so readers would reasonably expect a POLL_HUP event to be sent
     on a read end of a pipe with no writers.
 
     Please do something about this - 
     or would a patch from me that fixes this ever be
     likely to be considered ?

Thanks for any responses & Best Regards,
Jason Vas Dias

     
^ permalink raw reply	[flat|nested] 2+ messages in thread