[PATCH 0/1] Point out the interactions of signalfd with epoll & fork

[PATCH 0/1] Point out the interactions of signalfd with epoll & fork

[Andrew Clayton]

Hi Michael,

I was just hit by this interaction of signalfd with epoll & fork.
Basically if you add a signalfd file descriptor to epoll and then fork,
you won't receive notifications for sent signals,

Hopefully this addition to the man page will help avoid people head
scratching in the future.

I've added links and quotes to the sources for this problem in the
commit message.

Hopefully the text is OK. I don't really understand what is going on
underneath.

Cheers,
Andrew

Andrew Clayton (1):
  signalfd.2: Note about interactions with epoll & fork

 man2/signalfd.2 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

-- 
2.21.0

[PATCH 1/1] signalfd.2: Note about interactions with epoll & fork

[Andrew Clayton]

Using signalfd(2) with epoll(7) and fork(2) can lead to some
head scratching.

It seems that when a signalfd file descriptor is added to epoll you will
only get notifications for signals sent to the process that added the
file descriptor to epoll.

So if you have a signalfd fd registered with epoll and then call
fork(2), perhaps by way of daemon(3) for example. Then you will find
that you no longer get notifications for signals sent to the newly
forked process.

User kentonv on ycombinator[0] explained it thus

    "One place where the inconsistency gets weird is when you use
     signalfd with epoll. The epoll will flag events on the signalfd
     based on the process where the signalfd was registered with epoll,
     not the process where the epoll is being used. One case where this
     can be surprising is if you set up a signalfd and an epoll and then
     fork() for the purpose of daemonizing -- now you will find that
     your epoll mysteriously doesn't deliver any events for the signalfd
     despite the signalfd otherwise appearing to function as expected."

And another post from the same person[1].

And then there is this snippet from this kernel commit message[2]

    "If you share epoll fd which contains our sigfd with another process
     you should blame yourself. signalfd is "really special"."

So add a note to the man page that points this out where people will
hopefully find it sooner rather than later!

[0]: https://news.ycombinator.com/item?id=9564975
[1]: https://stackoverflow.com/questions/26701159/sending-signalfd-to-another-process/29751604#29751604
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d80e731ecab420ddcb79ee9d0ac427acbc187b4b

Signed-off-by: Andrew Clayton <andrew@digital-domain.net>
---
 man2/signalfd.2 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/man2/signalfd.2 b/man2/signalfd.2
index 497ee4cbd..a96ff6441 100644
--- a/man2/signalfd.2
+++ b/man2/signalfd.2
@@ -261,6 +261,23 @@ itself and the signals that are directed to the process
 (i.e., the entire thread group).
 (A thread will not be able to read signals that are directed
 to other threads in the process.)
+.SS epoll(7) semantics
+If you add a signalfd file descriptor to
+.BR epoll(7)
+then
+.BR epoll_wait(2)
+will only return events for signals received by the process that did
+the
+.BR epoll_ctl(2).
+If you then
+.BR fork(2),
+say by calling
+.BR daemon(3),
+then you will find that you don't get any notifications for sent
+signals. For this to work, you need to add the signalfd file
+descriptor to
+.BR epoll(7)
+after forking.
 .SH RETURN VALUE
 On success,
 .BR signalfd ()
-- 
2.21.0

Re: [PATCH 1/1] signalfd.2: Note about interactions with epoll & fork

[Michael Kerrisk (man-pages)]

Hello Andrew,

On 9/21/19 1:42 AM, Andrew Clayton wrote:
> Using signalfd(2) with epoll(7) and fork(2) can lead to some
> head scratching.
> 
> It seems that when a signalfd file descriptor is added to epoll you will
> only get notifications for signals sent to the process that added the
> file descriptor to epoll.
> 
> So if you have a signalfd fd registered with epoll and then call
> fork(2), perhaps by way of daemon(3) for example. Then you will find
> that you no longer get notifications for signals sent to the newly
> forked process.
> 
> User kentonv on ycombinator[0] explained it thus
> 
>     "One place where the inconsistency gets weird is when you use
>      signalfd with epoll. The epoll will flag events on the signalfd
>      based on the process where the signalfd was registered with epoll,
>      not the process where the epoll is being used. One case where this
>      can be surprising is if you set up a signalfd and an epoll and then
>      fork() for the purpose of daemonizing -- now you will find that
>      your epoll mysteriously doesn't deliver any events for the signalfd
>      despite the signalfd otherwise appearing to function as expected."
> 
> And another post from the same person[1].
> 
> And then there is this snippet from this kernel commit message[2]
> 
>     "If you share epoll fd which contains our sigfd with another process
>      you should blame yourself. signalfd is "really special"."
> 
> So add a note to the man page that points this out where people will
> hopefully find it sooner rather than later!
> 
> [0]: https://news.ycombinator.com/item?id=9564975
> [1]: https://stackoverflow.com/questions/26701159/sending-signalfd-to-another-process/29751604#29751604
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d80e731ecab420ddcb79ee9d0ac427acbc187b4b

Thanks for all of the detail in the commit message! That's funky! But I
suppose in the end it's not completely surprising, because of the fact
that epoll is really notifying events on the open file description,
and there may be multiple file descriptors that refer to that description
(as for example, after fork()). It is however, still surprising to users 
that you can read() from the FD, even though epoll doesn't say it's ready.


> Signed-off-by: Andrew Clayton <andrew@digital-domain.net>
> ---
>  man2/signalfd.2 | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/man2/signalfd.2 b/man2/signalfd.2
> index 497ee4cbd..a96ff6441 100644
> --- a/man2/signalfd.2
> +++ b/man2/signalfd.2
> @@ -261,6 +261,23 @@ itself and the signals that are directed to the process
>  (i.e., the entire thread group).
>  (A thread will not be able to read signals that are directed
>  to other threads in the process.)
> +.SS epoll(7) semantics
> +If you add a signalfd file descriptor to
> +.BR epoll(7)
> +then
> +.BR epoll_wait(2)
> +will only return events for signals received by the process that did
> +the
> +.BR epoll_ctl(2).
> +If you then
> +.BR fork(2),
> +say by calling
> +.BR daemon(3),
> +then you will find that you don't get any notifications for sent
> +signals. For this to work, you need to add the signalfd file
> +descriptor to
> +.BR epoll(7)
> +after forking.
>  .SH RETURN VALUE
>  On success,
>  .BR signalfd ()

Thanks for the proposed text. But I prefer to avoid mentioning 
daemon(3), which has a longstanding bug described in its manual
page). Also, I think we need to bring out the fact that the
signalfd still makes signals available for reading, even though
epoll does not say the FD is ready. Also, your text might
lead the reader to think that this sequence might be possible:

create signalfd
create epoll instance
add signalfd to epoll instance
fork()
       In child: add signalfd to epoll instance

That last step will of course give an EEXIST error from
epoll_ctl(), because  that FD is already in the epoll
instance.

So, I've applied your patch, but then done a fairly major rewrite,
so that the text now looks like:

   epoll(7) semantics
       If a process adds (via epoll_ctl(2)) a signalfd file descriptor to
       an epoll(7) instance, then epoll_wait(2) returns events  only  for
       signals  sent to that process.  In particular, if the process then
       uses fork() to create a child process, then the child will be able
       to  read(2)  signals  that  are sent to it using the signalfd file
       descriptor, but epoll_wait(2) will not indicate that the  signalfd
       file descriptor is ready.  In this scenario, a possible workaround
       is that after the fork(2), the child process can  close  the  sig‐
       nalfd  file  descriptor  that it inherited from the parent process
       and then create another signalfd file descriptor and add it to the
       epoll  instance.   Alternatively,  the  parent and the child could
       delay creating their  (separate)  signalfd  file  descriptors  and
       adding them to the epoll instance until after the call to fork(2).

Seem okay to you?

I also verified this weirness using the program shown below:

$ ./epoll_signalfd
PID of parent: 5661
PID of child:  5662
epoll_wait() returned 0
PID 5662: got signal 10
Successfully read signal, even though epoll_wait() didn't say FD was ready!

8x----8x----8x----8x----8x----8x----8x----8x----8x----8x----8x----8x----
/* epoll_signalfd.c */

#include <sys/signalfd.h>
#include <signal.h>
#include <sys/epoll.h>
#include <sys/wait.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#define errExit(msg)    do { perror(msg); exit(EXIT_FAILURE); \
                        } while (0)

static void
signalTest(int sfd, int epfd)
{
    struct signalfd_siginfo fdsi;
    struct epoll_event rev;
    int ready;
    ssize_t s;

    usleep(50000);
    ready = epoll_wait(epfd, &rev, 1, 0);
    if (ready == -1)
        errExit("epoll_wait");

    printf("epoll_wait() returned %d\n", ready);

    s = read(sfd, &fdsi, sizeof(struct signalfd_siginfo));
    if (s != sizeof(struct signalfd_siginfo))
        errExit("read");

    printf("PID %ld: got signal %d\n", (long) getpid(), fdsi.ssi_signo);

    if (ready == 0 && s > 0)
        printf("Successfully read signal, even though epoll_wait() "
                "didn't say FD was ready!\n");
}

int
main(int argc, char *argv[])
{
    struct epoll_event ev;
    sigset_t mask;
    int sfd, epfd;

    sigfillset(&mask);
    sigdelset(&mask, SIGINT);

    if (sigprocmask(SIG_BLOCK, &mask, NULL) == -1)
        errExit("sigprocmask");

    sfd = signalfd(-1, &mask, SFD_NONBLOCK);
    if (sfd == -1)
        errExit("signalfd");

    epfd = epoll_create(5);
    if (epfd == -1)
        errExit("epoll_create");

    ev.data.fd = sfd;
    ev.events = EPOLLIN;
    if (epoll_ctl(epfd, EPOLL_CTL_ADD, sfd, &ev) == -1)
        errExit("epoll_ctl");

    switch (fork()) {
    case -1:
        errExit("fork");
    case 0:
        printf("PID of child:  %ld\n", (long) getpid());
        raise(SIGUSR1);
        signalTest(sfd, epfd);
        break;
    default:
        printf("PID of parent: %ld\n", (long) getpid());
        wait(NULL);
        break;
    }

    exit(EXIT_SUCCESS);
}
8x----8x----8x----8x----8x----8x----8x----8x----8x----8x----8x----8x----

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH 1/1] signalfd.2: Note about interactions with epoll & fork

[Andrew Clayton]

On Mon, 23 Sep 2019 16:49:29 +0200
"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> wrote:

> Hello Andrew,

Hi Michael,

Thanks for the extensive reply!

[...]
 
> Thanks for all of the detail in the commit message! That's funky! But I
> suppose in the end it's not completely surprising, because of the fact
> that epoll is really notifying events on the open file description,
> and there may be multiple file descriptors that refer to that description
> (as for example, after fork()). It is however, still surprising to users 
> that you can read() from the FD, even though epoll doesn't say it's ready.

Ah, I gotta remember there's a distinction between descriptors &
descriptions ;) 

[...]

> Thanks for the proposed text. But I prefer to avoid mentioning 
> daemon(3), which has a longstanding bug described in its manual
> page). Also, I think we need to bring out the fact that the
> signalfd still makes signals available for reading, even though

Yes, true.

> epoll does not say the FD is ready. Also, your text might
> lead the reader to think that this sequence might be possible:
> 
> create signalfd
> create epoll instance
> add signalfd to epoll instance
> fork()
>        In child: add signalfd to epoll instance
> 
> That last step will of course give an EEXIST error from
> epoll_ctl(), because  that FD is already in the epoll
> instance.

Yup.
 
> So, I've applied your patch, but then done a fairly major rewrite,
> so that the text now looks like:
> 
>    epoll(7) semantics
>        If a process adds (via epoll_ctl(2)) a signalfd file descriptor to
>        an epoll(7) instance, then epoll_wait(2) returns events  only  for
>        signals  sent to that process.  In particular, if the process then
>        uses fork() to create a child process, then the child will be able
>        to  read(2)  signals  that  are sent to it using the signalfd file
>        descriptor, but epoll_wait(2) will not indicate that the  signalfd
>        file descriptor is ready.  In this scenario, a possible workaround
>        is that after the fork(2), the child process can  close  the  sig‐
>        nalfd  file  descriptor  that it inherited from the parent process
>        and then create another signalfd file descriptor and add it to the
>        epoll  instance.   Alternatively,  the  parent and the child could
>        delay creating their  (separate)  signalfd  file  descriptors  and
>        adding them to the epoll instance until after the call to fork(2).
> 
> Seem okay to you?

Yes, looks good.
 
> I also verified this weirness using the program shown below:

Nice example.

[...]

Cheers,
Andrew