script: fix signalfd use

It's necessary to create signal-fd before fork() to get SIGCHLD,
because child could be faster than our code.

Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/lib/pty-session.c b/lib/pty-session.c
index 10eb462d28b4a7ed3358145f355d7197e43e6492..5486fd06883c8576b1f30426b1cb0953f1a9b442 100644 (file)
--- a/lib/pty-session.c
+++ b/lib/pty-session.c
@@ -128,11 +128,24 @@ void ul_pty_set_mainloop_time(struct ul_pty *pty, struct timeval *tv)
        }
 }
 
+static void pty_signals_cleanup(struct ul_pty *pty)
+{
+       if (pty->sigfd != -1)
+               close(pty->sigfd);
+       pty->sigfd = -1;
+
+       /* restore original setting */
+       sigprocmask(SIG_SETMASK, &pty->orgsig, NULL);
+}
+
 /* call me before fork() */
 int ul_pty_setup(struct ul_pty *pty)
 {
        struct termios slave_attrs;
-       int rc;
+       sigset_t ourset;
+       int rc = 0;
+
+       assert(pty->sigfd == -1);
 
        /* save the current signals setting */
        sigprocmask(0, NULL, &pty->orgsig);
@@ -141,11 +154,15 @@ int ul_pty_setup(struct ul_pty *pty)
                DBG(SETUP, ul_debugobj(pty, "create for terminal"));
 
                /* original setting of the current terminal */
-               if (tcgetattr(STDIN_FILENO, &pty->stdin_attrs) != 0)
-                       return -errno;
+               if (tcgetattr(STDIN_FILENO, &pty->stdin_attrs) != 0) {
+                       rc = -errno;
+                       goto done;
+               }
                ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&pty->win);
                /* create master+slave */
                rc = openpty(&pty->master, &pty->slave, NULL, &pty->stdin_attrs, &pty->win);
+               if (rc)
+                       goto done;
 
                /* set the current terminal to raw mode; pty_cleanup() reverses this change on exit */
                slave_attrs = pty->stdin_attrs;
@@ -160,9 +177,30 @@ int ul_pty_setup(struct ul_pty *pty)
                        tcgetattr(pty->slave, &slave_attrs);
                        slave_attrs.c_lflag &= ~ECHO;
                        tcsetattr(pty->slave, TCSANOW, &slave_attrs);
-               }
+               } else
+                       goto done;
+       }
+
+       sigfillset(&ourset);
+       if (sigprocmask(SIG_BLOCK, &ourset, NULL)) {
+               rc = -errno;
+               goto done;
        }
 
+       sigemptyset(&ourset);
+       sigaddset(&ourset, SIGCHLD);
+       sigaddset(&ourset, SIGWINCH);
+       sigaddset(&ourset, SIGALRM);
+       sigaddset(&ourset, SIGTERM);
+       sigaddset(&ourset, SIGINT);
+       sigaddset(&ourset, SIGQUIT);
+
+       if ((pty->sigfd = signalfd(-1, &ourset, SFD_CLOEXEC)) < 0)
+               rc = -errno;
+done:
+       if (rc)
+               ul_pty_cleanup(pty);
+
        DBG(SETUP, ul_debugobj(pty, "pty setup done [master=%d, slave=%d, rc=%d]",
                                pty->master, pty->slave, rc));
        return rc;
@@ -173,6 +211,8 @@ void ul_pty_cleanup(struct ul_pty *pty)
 {
        struct termios rtt;
 
+       pty_signals_cleanup(pty);
+
        if (pty->master == -1 || !pty->isterm)
                return;
 
@@ -434,7 +474,6 @@ static int handle_signal(struct ul_pty *pty, int fd)
 /* loop in parent */
 int ul_pty_proxy_master(struct ul_pty *pty)
 {
-       sigset_t ourset;
        int rc = 0, ret, eof = 0;
        enum {
                POLLFD_SIGNAL = 0,
@@ -451,22 +490,7 @@ int ul_pty_proxy_master(struct ul_pty *pty)
        /* We use signalfd and standard signals by handlers are blocked
         * at all
         */
-       sigfillset(&ourset);
-       if (sigprocmask(SIG_BLOCK, &ourset, NULL))
-               return -errno;
-
-       sigemptyset(&ourset);
-       sigaddset(&ourset, SIGCHLD);
-       sigaddset(&ourset, SIGWINCH);
-       sigaddset(&ourset, SIGALRM);
-       sigaddset(&ourset, SIGTERM);
-       sigaddset(&ourset, SIGINT);
-       sigaddset(&ourset, SIGQUIT);
-
-       if ((pty->sigfd = signalfd(-1, &ourset, SFD_CLOEXEC)) < 0) {
-               rc = -errno;
-               goto done;
-       }
+       assert(pty->sigfd >= 0);
 
        pfd[POLLFD_SIGNAL].fd = pty->sigfd;
        pty->poll_timeout = -1;
@@ -569,13 +593,7 @@ int ul_pty_proxy_master(struct ul_pty *pty)
                }
        }
 
-done:
-       if (pty->sigfd != -1)
-               close(pty->sigfd);
-       pty->sigfd = -1;
-
-       /* restore original setting */
-       sigprocmask(SIG_SETMASK, &pty->orgsig, NULL);
+       pty_signals_cleanup(pty);
 
        DBG(IO, ul_debug("poll() done [signal=%d, rc=%d]", pty->delivered_signal, rc));
        return rc;

diff --git a/term-utils/script.c b/term-utils/script.c
index d1f326d0744a1710426d79366b4b8a45c82e9cb2..8d0b45f646e2c10b9b27431f9beb6e6ffb46142c 100644 (file)
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -893,6 +893,10 @@ int main(int argc, char **argv)
                printf(_(".\n"));
        }
 
+#ifdef HAVE_LIBUTEMPTER
+       utempter_add_record(ul_pty_get_childfd(ctl.pty), NULL);
+#endif
+
        if (ul_pty_setup(ctl.pty))
                err(EXIT_FAILURE, _("failed to create pseudo-terminal"));
 
@@ -902,10 +906,6 @@ int main(int argc, char **argv)
         * We have terminal, do not use err() from now, use "goto done"
         */
 
-#ifdef HAVE_LIBUTEMPTER
-       utempter_add_record(ul_pty_get_childfd(ctl.pty), NULL);
-#endif
-
        switch ((int) (ctl.child = fork())) {
        case -1: /* error */
                warn(_("cannot create child process"));

diff --git a/tests/ts/script/buffering-race b/tests/ts/script/buffering-race
index 51421faeb676a8f9fcd6469082a7c4eaab54ffaf..b7203837e46a20c7eda36e59a726280a4566c6df 100755 (executable)
--- a/tests/ts/script/buffering-race
+++ b/tests/ts/script/buffering-race
@@ -20,7 +20,8 @@ ts_init "$*"
 
 ts_check_test_command "$TS_CMD_SCRIPT"
 
-SCRIPT_DEBUG=all ULPTY_DEBUG=all $TS_CMD_SCRIPT -c "echo Hallo World" /dev/null </dev/null >$TS_OUTPUT
+#SCRIPT_DEBUG=all ULPTY_DEBUG=all
+$TS_CMD_SCRIPT -c "echo Hallo World" /dev/null </dev/null >$TS_OUTPUT
 
 ts_finalize