Clean BG processes created by %%script on kernel exit

[tkf]

This patch does not work yet because __del__ method is not guaranteed to be called on exit. Are there any hook in kernel which is called when the kernel exits?

[takluyver]

The standard library atexit module can call functions when the
interpreter shuts down.

http://docs.python.org/library/atexit

[tkf]

After I added atexit.register(self.kill_bg_processes), IPython hangs when try to exit. I need to hit C-c to exit:

% ./ipython.py
Python 2.7.2+ (default, Oct  4 2011, 20:06:09) 
Type "copyright", "credits" or "license" for more information.

IPython 0.13.beta1 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: %%sh --bg
   ...: exec sleep 12345
   ...: 
Starting job # 0 in a separate thread.

In [2]: 
Do you really want to exit ([y]/n)? y
^CException KeyboardInterrupt in <module 'threading' from '/usr/lib/python2.7/threading.pyc'> ignored

Need more time to debug...

[tkf]

No, I don't need atexit for this behavior.
I guess atexit waits until all the threads are finished?

[takluyver]

I'm a bit puzzled by that. From what I can see, it should wait for non-daemon threads - but the only other thread we use is a daemon. And we use atexit in several other places without any problem.

[takluyver]

Ah, I see, backgroundjobs uses non-daemon threads. That's what's
causing problems for atexit, and also probably causing #1984.

[minrk]

Re: backgroundjobs / daemon: which makes more sense? It seems to me like they should be daemon, because if you start a few tasks in the background and then exit, those tasks should complete before the process ends, unless the exit is forceful (i.e. by a signal).

[takluyver]

Note that the behaviour you describe (waiting for them to finish) would mean they're not daemon.

Starting a background job with %script --bg will actually create a thread which is waiting for a subprocess. I'm not sure if killing the thread also kills the subprocess or not.

[tkf]

I didn't know about daemon property, but I guess using daemon threads mean that you need to do invoke any cleanup for these thread from the main thread, right? I think this is preferable for script magic, because we can use atexit.

If the question @minrk asked is to terminate BG processes or not, I think they should be terminated. I was running some quick server in notebook and after restarting the notebook kernel I could not restart the server because the port was already in use. So I needed to grep through the output of ps and kill the process to execute the code in notebook again.

[takluyver]

If it's running in a terminal, it could perhaps prompt on shutdown: "2
background tasks are still running. Wait for them to finish (y/[n])?"

[minrk]

But if you start a background process in bash, it lives forever.

I would expect one of the following:

1. background shell scripts behave like scripts backgrounded in the shell (i.e. totally detached)
2. background scripts are awaited on clean exit, because they are assumed to be part of regular computation.

daemon threads does neither of these.

[takluyver]

A quick test suggests that if the thread running the subprocess is a
daemon and you exit the process, the subprocess itself continues.

[minrk]

I think the confirmation dialog is not worth the complication. Let's go with daemon threads and force kill, as @takluyver suggested. It's not what I would expect, but it is the least likely to have unpleasant surprises.

[tkf]

In my zsh, job & is terminated when I exit (zsh warns once). I didn't know that in bash it's different. In zsh you need &! for bash's behavior.

@takluyver Yes, I think that's reasonable behavior.

@minrk I guess you need multiprocessing for 1. I think 2 is the current status and probably a good default.

But I think having terminate-on-exit option would be nice. What do you think? Or more in general, %onexit magic to register hook would be nice. So that you can do something like this:

In [1]:
%%sh --proc p
sleep long time

In [2]:
%onexit p.kill()

[takluyver]

I don't think we need an %onexit magic - using atexit.register is easy enough already.

[minrk]

I don't think there's any need to do anything complicated. Let's just change scripts to use daemon=True, and register kill with atexit. I agree with @takluyver that %onexit provides no benefit beyond atexit.register.

[tkf]

Ah, I missed the two messages before my post. If it's OK to kill forcefully, I will stick to that.

And if daemon thread works that way, right, atexit is fine.

[tkf]

I added atexit and it works almost fine. When I start ipython by ./ipython.py, and start subprocess as:

%sh --bg
exec sleep 123456

exiting ipython kills sleep. However, when I start ipython using kernel ./ipython.py console, exiting ipython is fine but sleep remains as detached process (PPID=1). I can still reproduce #1984.

[minrk]

On what system can you reproduce #1984? I've tried everything I can think of, and it always works fine.

[tkf]

I am using Ubuntu 11.10 (x86_64).

[tkf]

I noticed that after restarting kernel, old kernel becomes defunct. After sometime it is no longer there (doesn't show up in ps) and then I can restart kernel.

[minrk]

So this is working now?

[minrk]

And does it fix #1984 as well?

[tkf]

Yes, it fixes #1984.
BTW, it would be nice to have similar shutdown in console app.

[tkf]

I just checked that cleanup works in qtconsole if #1988 is merged.

[minrk]

Okay, then I think this should be ready to merge, right @takluyver?

The terminal console is a bit weird - Even know I don't really understand how it shuts down.

[minrk]

Right - the qtconsole was always better, because it did proper shutdown. That's where I got the fix from to use here.

[tkf]

I tweaked %killbgscripts magic a bit because it was too silent (and the dummy argument was ugly).

[minrk]

let's assign self.bg_processes = [] at the end of this, so that the list doesn't keep growing forever.

[tkf]

Good point. Fixed.

[takluyver]

It looks OK to me, but has someone tested on Windows? I know there are some differences in process control.

[minrk]

On Windows I do get the 'address in use' error, before and after this change when there is a running script subprocess (background or not). This is true for any running subprocess, regardless of the %%script magic. I expect this is true in 0.12 as well. If I recall, Windows is just slower at releasing ports, so restart_kernel might just want a sleep on Windows.

[minrk]

Since the Windows bugs are not a regression, I think this can be merged as-is and Windows can be addressed at a later point (perhaps not for 0.13). I think the %%script magic will be significantly less popular on Windows, anyway.

[minrk]

I will merge this, and retag 1984 as a Windows-specific bug, now that it's addressed on sensible platforms.

[tkf]

Thanks.