Correctly pass along SIGINT to child processes.

Ever since we introduced bup newliner, signal handling has been a little
screwy.  The problem is that ctrl-c is passed to *all* processes in the
process group, not just the parent, so everybody would start terminating at
the same time, with very messy results.

Two results were particularly annoying: git.PackWriter()'s destructor
wouldn't always get called (so half-finished packs would be lost instead of
kept so we don't need to backup the same stuff next time) and bup-newliner
would exit, so the stdout/stderr of a process that *did* try to clean up
would be lost, usually resulting in EPIPE, which killed the proces while
attempting to clean up.

The fix is simple: when starting a long-running subprocess, give it its own
session by calling os.setsid().  That way ctrl-c is only sent to the
toplevel 'bup' process, who can forward it as it should.

Next, fix bup's signal forwarding to actually forward the same signal as it
received, instead of always using SIGTERM.

diff --git a/lib/bup/client.py b/lib/bup/client.py
index 7b3e7c35700dbc914f5301c35e872f42e43082a1..a5762ae4b652c4f552beae6d92ee593a09fa8941 100644 (file)
--- a/lib/bup/client.py
+++ b/lib/bup/client.py
@@ -39,12 +39,16 @@ class Client:
                    """ % escapedir
             argv = ['ssh', host, '--', cmd.strip()]
             #log('argv is: %r\n' % argv)
+        def setup():
+            if fixenv:
+                fixenv()
+            os.setsid()
         (self.host, self.dir) = (host, dir)
         self.cachedir = git.repo('index-cache/%s'
                                  % re.sub(r'[^@\w]', '_', 
                                           "%s:%s" % (host, dir)))
         try:
-            self.p = p = Popen(argv, stdin=PIPE, stdout=PIPE, preexec_fn=fixenv)
+            self.p = p = Popen(argv, stdin=PIPE, stdout=PIPE, preexec_fn=setup)
         except OSError, e:
             raise ClientError, 'exec %r: %s' % (argv[0], e), sys.exc_info()[2]
         self.conn = conn = Conn(p.stdout, p.stdin)

diff --git a/main.py b/main.py
index 4d8ea1f3547260dd293590de64ae4317eadff301..50eb24454154b216287563e1ad5c773d27bb24b9 100755 (executable)
--- a/main.py
+++ b/main.py
@@ -93,6 +93,7 @@ def force_tty():
     if fix_stdout or fix_stderr:
         amt = (fix_stdout and 1 or 0) + (fix_stderr and 2 or 0)
         os.environ['BUP_FORCE_TTY'] = str(amt)
+    os.setsid()  # make sure ctrl-c is sent just to us, not to child too
 
 if fix_stdout or fix_stderr:
     realf = fix_stderr and 2 or 1
@@ -108,15 +109,18 @@ else:
 
 
 class SigException(Exception):
-    pass
+    def __init__(self, signum):
+        self.signum = signum
+        Exception.__init__(self, 'signal %d received' % signum)
 def handler(signum, frame):
-    raise SigException('signal %d received' % signum)
+    raise SigException(signum)
 
 signal.signal(signal.SIGTERM, handler)
 signal.signal(signal.SIGINT, handler)
 
 ret = 95
 p = None
+killsig = signal.SIGTERM
 try:
     try:
         p = subprocess.Popen([subpath(subcmd)] + argv[2:],
@@ -126,10 +130,12 @@ try:
         log('%s: %s\n' % (subpath(subcmd), e))
         ret = 98
     except SigException, e:
+        log('\nbup: %s\n' % e)
+        killsig = e.signum
         ret = 94
 finally:
     if p and p.poll() == None:
-        os.kill(p.pid, signal.SIGTERM)
+        os.kill(p.pid, killsig)
         p.wait()
     if n:
         n.stdin.close()