Merge pull request #25 from chriskuehl/background-support

Properly respond to job control signals

dumb-init.c

@@ -37,11 +37,62 @@ void forward_signal(int signum) {
     }
 }
 
+/*
+ * The dumb-init signal handler.
+ *
+ * The main job of this signal handler is to forward signals along to our child
+ * process(es). In setsid mode, this means signaling the entire process group
+ * rooted at our child. In non-setsid mode, this is just signaling the primary
+ * child.
+ *
+ * In most cases, simply proxying the received signal is sufficient. If we
+ * receive a job control signal, however, we should not only forward it, but
+ * also sleep dumb-init itself.
+ *
+ * This allows users to run foreground processes using dumb-init and to
+ * control them using normal shell job control features (e.g. Ctrl-Z to
+ * generate a SIGTSTP and suspend the process).
+ *
+ * The libc manual is useful:
+ * https://www.gnu.org/software/libc/manual/html_node/Job-Control-Signals.html
+ *
+ * When running in setsid mode, however, it is not sufficient to forward
+ * SIGTSTP/SIGTTIN/SIGTTOU in most cases. If the process has not added a custom
+ * signal handler for these signals, then the kernel will not apply default
+ * signal handling behavior (which would be suspending the process) since it is
+ * a member of an orphaned process group.
+ *
+ * Sadly this doesn't appear to be well documented except in the kernel itself:
+ * https://github.com/torvalds/linux/blob/v4.2/kernel/signal.c#L2296-L2299
+ *
+ * Forwarding SIGSTOP instead is effective, though not ideal; unlike SIGTSTP,
+ * SIGSTOP cannot be caught, and so it doesn't allow processes a change to
+ * clean up before suspending. In non-setsid mode, we proxy the original signal
+ * instead of SIGSTOP for this reason.
+*/
 void handle_signal(int signum) {
     DEBUG("Received signal %d.\n", signum);
+
+    if (
+        signum == SIGTSTP || // tty: background yourself
+        signum == SIGTTIN || // tty: stop reading
+        signum == SIGTTOU    // tty: stop writing
+    ) {
+        if (use_setsid) {
+            DEBUG("Running in setsid mode, so forwarding SIGSTOP instead.\n");
+            forward_signal(SIGSTOP);
+        } else {
+            DEBUG("Not running in setsid mode, so forwarding the original signal (%d).\n", signum);
             forward_signal(signum);
         }
 
+        DEBUG("Suspending self due to TTY signal.\n");
+        kill(getpid(), SIGSTOP);
+    } else {
+        forward_signal(signum);
+    }
+}
+
 void print_help(char *argv[]) {
     fprintf(stderr,
         "Usage: %s COMMAND [[ARG] ...]\n"

tests/lib/print_signals.py

@@ -11,10 +11,14 @@ import signal
 import sys
 import time
 
-from tests.lib.testing import CATCHABLE_SIGNALS
+
+CATCHABLE_SIGNALS = frozenset(
+    set(range(1, 32)) - set([signal.SIGKILL, signal.SIGSTOP, signal.SIGCHLD])
+)
 
 
 print_queue = []
+last_signal = None
 
 
 def unbuffered_print(line):
@@ -35,6 +39,12 @@ if __name__ == '__main__':
     # loop forever just printing signals
     while True:
         if print_queue:
-            unbuffered_print(print_queue.pop())
+            signum = print_queue.pop()
+            unbuffered_print(signum)
+
+            if signum == signal.SIGINT and last_signal == signal.SIGINT:
+                print('Received SIGINT twice, exiting.')
+                exit(0)
+            last_signal = signum
 
         time.sleep(0.01)

tests/lib/testing.py

@@ -1,10 +1,20 @@
+import re
 import signal
 
 from py._path.local import LocalPath
 
 
-CATCHABLE_SIGNALS = frozenset(
+# these signals cause dumb-init to suspend itself
-    set(range(1, 32)) - set([signal.SIGKILL, signal.SIGSTOP, signal.SIGCHLD])
+SUSPEND_SIGNALS = frozenset([
+    signal.SIGTSTP,
+    signal.SIGTTOU,
+    signal.SIGTTIN,
+])
+
+NORMAL_SIGNALS = frozenset(
+    set(range(1, 32))
+    - set([signal.SIGKILL, signal.SIGSTOP, signal.SIGCHLD])
+    - SUSPEND_SIGNALS
 )
 
 
@@ -32,3 +42,10 @@ def pid_tree(pid):
 def is_alive(pid):
     """Return whether a process is running with the given PID."""
     return LocalPath('/proc').join(str(pid)).isdir()
+
+
+def process_state(pid):
+    """Return a process' state, such as "stopped" or "running"."""
+    status = LocalPath('/proc').join(str(pid), 'status').read()
+    m = re.search('^State:\s+[A-Z] \(([a-z]+)\)$', status, re.MULTILINE)
+    return m.group(1)

tests/proxies_signals_test.py

@@ -5,11 +5,12 @@ import sys
 from subprocess import PIPE
 from subprocess import Popen
 
-from tests.lib.testing import CATCHABLE_SIGNALS
+from tests.lib.testing import NORMAL_SIGNALS
 from tests.lib.testing import pid_tree
 
 
 def test_prints_signals(both_debug_modes, both_setsid_modes):
+    """Ensure dumb-init proxies regular signals to its child."""
     proc = Popen(
         ('dumb-init', sys.executable, '-m', 'tests.lib.print_signals'),
         stdout=PIPE,
@@ -17,7 +18,7 @@ def test_prints_signals(both_debug_modes, both_setsid_modes):
 
     assert re.match(b'^ready \(pid: (?:[0-9]+)\)\n$', proc.stdout.readline())
 
-    for signum in CATCHABLE_SIGNALS:
+    for signum in NORMAL_SIGNALS:
         proc.send_signal(signum)
         assert proc.stdout.readline() == '{0}\n'.format(signum).encode('ascii')
 

tests/shell_background_test.py

@@ -0,0 +1,82 @@
+import os
+import re
+import sys
+import time
+from signal import SIGCONT
+from signal import SIGKILL
+from subprocess import PIPE
+from subprocess import Popen
+
+from tests.lib.testing import pid_tree
+from tests.lib.testing import process_state
+from tests.lib.testing import SUSPEND_SIGNALS
+
+
+def test_shell_background_support_setsid(both_debug_modes, setsid_enabled):
+    """In setsid mode, dumb-init should suspend itself and its children when it
+    receives SIGTSTP, SIGTTOU, or SIGTTIN.
+    """
+    proc = Popen(
+        ('dumb-init', sys.executable, '-m', 'tests.lib.print_signals'),
+        stdout=PIPE,
+    )
+    match = re.match(b'^ready \(pid: ([0-9]+)\)\n$', proc.stdout.readline())
+    pid = match.group(1).decode('ascii')
+
+    for signum in SUSPEND_SIGNALS:
+        # both dumb-init and print_signals should be running or sleeping
+        assert process_state(pid) in ['running', 'sleeping']
+        assert process_state(proc.pid) in ['running', 'sleeping']
+
+        # both should now suspend
+        proc.send_signal(signum)
+
+        for _ in range(1000):
+            time.sleep(0.001)
+            try:
+                assert process_state(proc.pid) == 'stopped'
+                assert process_state(pid) == 'stopped'
+            except AssertionError:
+                pass
+            else:
+                break
+        else:
+            raise RuntimeError('Timed out waiting for processes to stop.')
+
+        # and then both wake up again
+        proc.send_signal(SIGCONT)
+        assert (
+            proc.stdout.readline() == '{0}\n'.format(SIGCONT).encode('ascii')
+        )
+        assert process_state(pid) in ['running', 'sleeping']
+        assert process_state(proc.pid) in ['running', 'sleeping']
+
+    for pid in pid_tree(proc.pid):
+        os.kill(pid, SIGKILL)
+
+
+def test_prints_signals(both_debug_modes, setsid_disabled):
+    """In non-setsid mode, dumb-init should forward the signals SIGTSTP,
+    SIGTTOU, and SIGTTIN, and then suspend itself.
+    """
+    proc = Popen(
+        ('dumb-init', sys.executable, '-m', 'tests.lib.print_signals'),
+        stdout=PIPE,
+    )
+
+    assert re.match(b'^ready \(pid: (?:[0-9]+)\)\n$', proc.stdout.readline())
+
+    for signum in SUSPEND_SIGNALS:
+        assert process_state(proc.pid) in ['running', 'sleeping']
+        proc.send_signal(signum)
+        assert proc.stdout.readline() == '{0}\n'.format(signum).encode('ascii')
+        assert process_state(proc.pid) == 'stopped'
+
+        proc.send_signal(SIGCONT)
+        assert (
+            proc.stdout.readline() == '{0}\n'.format(SIGCONT).encode('ascii')
+        )
+        assert process_state(proc.pid) in ['running', 'sleeping']
+
+    for pid in pid_tree(proc.pid):
+        os.kill(pid, SIGKILL)