Comment 8 for bug 141172

Revision history for this message
Andrew Bennetts (spiv) wrote :

We do disable SIGINT in the SSH subprocess, see the os_specific_subprocess_params in bzrlib/transport/ssh.py (to fix bug 5987, which is what John is referring to I think).

Perhaps we should keep a set of active SSH transports (maybe using weakrefs?), and have an almost top-level try/except handler for KeyboardInterrupt that sends SIGINT to them. Ideally we should be closing them directly as part of cleanups when unwinding the stack, but it seems reasonable to have insurance in case we don't clean up our internals perfectly.

In fact, we already have that set of weakrefs, bzrlib.transport.ssh._subproc_weakrefs, with callbacks that try to close those subprocesses when the last reference to that transport goes away. So if we are still leaving openssh subprocesses, either:

 * _close_ssh_subproc isn't strong enough (it doesn't send SIGINT, it just closes stdin and stdout of the process then waits)
 * the transport object isn't being garbage collected as bzr shuts down, which is probably fairly likely, especially considering the way we use os._exit (but could be possible even if we didn't)

So probably it is reasonable to change e.g. bzrlib.commands.exception_to_return_code to call _close_ssh_subproc or send SIGINT to those subprocesses if they are still live. The main trick would seem to be avoiding importing bzrlib.transport.ssh if it wasn't already imported.