From: Nathaniel W. Turner <[email protected]> This addresses the case where a Ctrl-C is intended for a process running in the chroot; before, schroot would always exit without cleaning up (the most visible symptom being left-over bind-mounts in /var/lib/schroot/mount). Now, cleanup occurs, and an appropriate error message is displayed ("E: Child terminated by signal ‘Interrupt’").
This patch does not cover all the cases described by bug 477770, but I think it is a step in the right direction. Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=477770 Signed-off-by: Nathaniel W. Turner <[email protected]> --- sbuild/sbuild-session.cc | 52 +++++++++++++++++++++++++++++++++++++++++++++- sbuild/sbuild-session.h | 16 ++++++++++++++ 2 files changed, 67 insertions(+), 1 deletions(-) diff --git a/sbuild/sbuild-session.cc b/sbuild/sbuild-session.cc index 9d83ca1..685a0bd 100644 --- a/sbuild/sbuild-session.cc +++ b/sbuild/sbuild-session.cc @@ -184,6 +184,7 @@ namespace } volatile bool sighup_called = false; + volatile bool sigint_called = false; volatile bool sigterm_called = false; /** @@ -199,6 +200,29 @@ namespace } /** + * Handle the SIGINT signal. + * + * @param ignore the signal number. + * + * We explicitly do nothing with SIGINT, and rely on the exit status of child + * processes to determine if we should care. We want to make sure any child + * process (which will also have received SIGINT) has exited before we do + * anything, and some child processes (for example, emacs) may expect SIGINT + * during normal operation. See http://www.cons.org/cracauer/sigint.html for + * a good discussion of SIGINT handling. + */ + void + sigint_handler (int ignore) + { + /* + * Allows us to detect if an interrupted waitpid() was interrupted due to + * SIGINT or something else. We may also want to use this at exit time to + * see if we should re-kill ourselves with SIGINT. + */ + sigint_called = true; + } + + /** * Handle the SIGTERM signal. * * @param ignore the signal number. @@ -242,6 +266,7 @@ session::session (std::string const& service, session_id(), force(false), saved_sighup_signal(), + saved_sigint_signal(), saved_sigterm_signal(), saved_termios(), termios_ok(false), @@ -571,6 +596,8 @@ session::run_impl () { sighup_called = false; set_sighup_handler(); + sigint_called = false; + set_sigint_handler(); sigterm_called = false; set_sigterm_handler(); @@ -695,6 +722,7 @@ session::run_impl () catch (error const& e) { clear_sigterm_handler(); + clear_sigint_handler(); clear_sighup_handler(); /* If a command was not run, but something failed, the exit @@ -705,6 +733,7 @@ session::run_impl () } clear_sigterm_handler(); + clear_sigint_handler(); clear_sighup_handler(); } @@ -1268,6 +1297,14 @@ session::wait_for_child (pid_t pid, while (1) { + /* + * If we (the parent process) gets SIGHUP or SIGTERM, pass this signal on + * to the child (once). Note that we do not handle SIGINT this way, + * because when a user presses Ctrl-C, the SIGINT is sent to all + * processes attached to that TTY (so the child will already have gotten + * it). In any case, once the child gets the signal, we just have to + * continue waiting for it to exit. + */ if ((sighup_called || sigterm_called) && !child_killed) { if (sighup_called) @@ -1290,7 +1327,7 @@ session::wait_for_child (pid_t pid, if (waitpid(pid, &status, 0) == -1) { - if (errno == EINTR && (sighup_called || sigterm_called)) + if (errno == EINTR && (sighup_called || sigterm_called || sigint_called)) continue; // Kill child and wait again. else throw error(CHILD_WAIT, strerror(errno)); @@ -1305,6 +1342,7 @@ session::wait_for_child (pid_t pid, sigterm_called = false; throw error(SIGNAL_CATCH, strsignal(SIGTERM)); } + /* no need to handle the SIGINT case here; it is handled correctly below */ else break; } @@ -1372,6 +1410,18 @@ session::clear_sighup_handler () } void +session::set_sigint_handler () +{ + set_signal_handler(SIGINT, &this->saved_sigint_signal, sigint_handler); +} + +void +session::clear_sigint_handler () +{ + clear_signal_handler(SIGINT, &this->saved_sigint_signal); +} + +void session::set_sigterm_handler () { set_signal_handler(SIGTERM, &this->saved_sigterm_signal, sigterm_handler); diff --git a/sbuild/sbuild-session.h b/sbuild/sbuild-session.h index 888fbfe..9179a29 100644 --- a/sbuild/sbuild-session.h +++ b/sbuild/sbuild-session.h @@ -417,6 +417,20 @@ namespace sbuild clear_sighup_handler (); /** + * Set the SIGINT handler. + * + * An error will be thrown on failure. + */ + void + set_sigint_handler (); + + /** + * Restore the state of SIGINT prior to setting the handler. + */ + void + clear_sigint_handler (); + + /** * Set the SIGTERM handler. * * An error will be thrown on failure. @@ -474,6 +488,8 @@ namespace sbuild bool force; /// Signal saved while sighup handler is set. struct sigaction saved_sighup_signal; + /// Signal saved while sigint handler is set. + struct sigaction saved_sigint_signal; /// Signal saved while sigterm handler is set. struct sigaction saved_sigterm_signal; /// Saved terminal settings. -- 1.7.0.4 -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]

