Qt frontend shutdown behavior fixes and enhancements

[minrk]

Ready for review.

) fixed error when resetting pykernel (it still reset, but printed an error)
2) fixed issue where closing a frontend, even secondary ones, always shutdown the kernel
3) shutdown_reply now goes out on the pub socket, so all clients are notified
3.a) this means that all clients can (and do) refresh the screen when a reset is called, just like the master frontend
4) kernel can stay alive after consoles are shutdown, and can be shutdown by any frontend at any point
4.a) this means that a shutdown request from any frontend can close all open frontends and the kernel, even if the kernel is detached, leaving no processes zombified.
4.b) 4.a required that a 'reset' element be added to shutdown_request/reply messages to identify the difference between a real shutdown message and stage 1 of a reset.

[fperez]

Minor typo: http://github.com/ipython/ipython/pull/164#L0R373
reest -> reset

• http://github.com/ipython/ipython/pull/164#L1R34
  Should document what 'existing' means. Object constructors are probably one of those places where we do want to be fairly explicit about documentation.
• http://github.com/ipython/ipython/pull/164#L1R51
  leftover code?
• The close console/close all should have keyboard accelerators ('c' and 'a' would be my choices, Esc is already the de-facto accelerator for Cancel).
• The 'close console' doesn't seem to actually work in linux. I get:

Segmentation fault
dreamweaver[test]> Killed by parent poller!

And the kernel is completely gone. Did you test only on osx so far? This is on ubuntu 10.04, with zmq/pyzmq 2.0.8.

• http://github.com/ipython/ipython/pull/164#L4R321

This addition to the messaging should be documented in the spec, so we don't fall out of sync. Currently the spec only says:

Message type: shutdown_request:

content = {
}

The restart field is fine, let's just make sure the spec document stays valid.

Summary:

most are minor nits easy to fix, but we need to figure out the linux crash before merging.

Thanks a lot, great work! I'll be available over the weekend for testing if you want.

[minrk]

All issues addressed in intermediate commits except for the accelerators.
Sorry I hadn't tested on Linux - it turns out my strategy of closing just a Window was a bad practice, and only worked on OSX by coincidence. Much more official/supported approach used now that works on both.

As for the accelerators, I'll have to look this up, as I have no idea how to bind keys to buttons. I never looked at any Qt code before this week, so I'll have to do some searching, unless someone knows this one off the top of their head.

[fperez]

Great job, I just merged and closed it, thanks!

I hope you'll be able to add the accelerators back, I made #166 to track this.

Great work, this is excellent functionality.

[ellisonbg]

• The choices in the close dialog are not very clear. Maybe change the text
  to something like "which would you like to close:" and then have choices of
  ('Cancel','Console Only','Kernel and Console').
• How does this logic interplay with the --parent flag of ipkernel?

[minrk]

It's not actually 'Kernel and Console', it's Kernel and all Consoles; that's why it's Close All. What do you think?

I didn't touch anything to do with the --parent flag. What should that change?

It's a hard thing to make clear, but I can have more explanation in the dialog box:

What would you like to close? Just this Console (leaving the Kernel running), or the kernel and all connected Consoles?

< Cancel | Just Console | Close Everything >

It really corresponds directly to the Close (cmd-W) vs Quit (cmd-Q) model of general applications. i.e. Close this Window (and leave everything else) vs. Quit this Application (and close/shutdown everything). It's certainly less clear with the whole kernel/interprocess model.

On OSX, this model is actually quite standard, and might not need any dialog - it's not normal at all for hitting the close button on a window to terminate an application (in fact, it's sufficiently uncommon for an application to do this, it should be considered a bad native UI). However, that's not the case on other platforms, and a general issue when building cross-platform apps, not at all specific to us.

[ellisonbg]

Ahh, that is quite different, missed that. But then the current message is even more confusing. Does this really automatically close all frontends connected to that kernel? Do other frontends have any say so in the matter? In the future it is entirely possible that the qtconsole app could connect to multiple kernels (in tabs maybe). In that case, it really doesn't make sense to actually quit the other frontends. I guess I am thinking that it really should be:

Cancel | Console Only | Kernel and This Console

If ipkernel is started with the --parent flag, it starts a thread that watches for the parent pid. When it goes away, the thread kills the process. This is used to make sure the ipkernel dies if the parent goes away. Obviously, the "Close Console Only" option won't work when --parent is used.

It is much less clear, especially when other windows could be on other hosts.

[minrk]

Kernel and This Console isn't useful because other Consoles can't do anything once the Kernel goes away.

'Close All' does close everything. With the current setup, if you close the Kernel, all the other frontends are useless anyway, so they should be closed. As it was before, if you closed the kernel (from anywhere), you would have a bunch of dangling frontends that can't do anything, unless the main parent decided to restart it. If you closed the master frontend, then you left possibly several Windows without the ability to ever do anything, and no information on what happened. If we change the frontend so that it's useful after the Kernel goes away, then the handling of a shutdown signal should be different, and that's easy to change. For now, though, I don't see any reason to keep Consoles open that have no ability to do anything.

The shutdown notice is handled by the qtconsole object, so changing that logic when new functionality comes in will be easy (it's in FrontendWidget._handle_shutdown_reply())
http://github.com/ipython/ipython/blob/master/IPython/frontend/qt/console/frontend_widget.py#L365

What you said about --parent is not true - it absolutely does work. The parent process is still alive, it's just not drawing a Console. That was the whole point of adding this - closing the initial console without losing the kernel.

[ellisonbg]

Minimally if we want to keep the current logic, the other frontends should display a message with an "OK" button indicating that the kernel was shutdown. Having an app silently disappear would be quite confusing if you were not expecting it.

So how exactly do you keep the qt frontend that started the kernel alive but not drawing windows? Where is that code?

[minrk]

I simply flip the switch that tells Qt to not automatically quit when the last Window is closed if Console-Only is chosen on the central frontend:
http://github.com/ipython/ipython/blob/master/IPython/frontend/qt/console/ipythonqt.py#L70

I can change it so there is a warning, and the user has the option to cancel the close. For instance, if they wanted to save (or print!) what they have. I don't think this is really necessary, at least on localhost - since there's no chance there that they don't know it's going down. Perhaps only display the message when the other Consoles are not connected via localhost?

Previously, there was no mechanism for other Consoles to even know that the Kernel was shutdown (only that the heartbeat failed). But now we can handle the shutdown however we want.

If I open 3 Consoles on one kernel locally, do you really think I should get at least 2 'OK' buttons, just to let me shut it down?

I think of it this way: If a user has multiple windows connected to a Kernel, there should be an easy way to close them all, and it should not involve more than one dialog, and certainly not one dialog per window.

But if you are sharing a Kernel remotely, I can definitely imagine a disappearing window being quite distressing.

[ellisonbg]

I agree that the extra message/dialog is not needed on localhost, but I do think we want that on a remote frontend for saving/printing purposes.

[minrk]

Okay, so what we want to change is:

1: more explicit dialog, but not change the behavior.

2: on notice of shutdown:
if someone else shutdown:
if I'm not connected via localhost:
prompt to close
else:
close

Is this correct?

[minrk]

Do you want to review the new changes?

Now there is a growing number of cases:

Multiple windows on localhost: no prompts, all Console windows are equivalent - one resets, all reset; kernel shuts down, all close.

Windows not on localhost:
on shutdown: prompt to close
on reset: prompt to clear

Is this better?

[ellisonbg]

Min,

I think this looks good. Fernando, how does this look to you?

Brian

[minrk]

I just noticed another possible issue: Remote kernels have the ability to shutdown the kernel, and local consoles will be shutdown automatically without warning. This is because it's the connection to the kernel that gets checked for the warning, not the relationship with the sender (which is unknown).

The obvious solutions:

1. have the shutdown message identify the sender's location. This can actually be inferred if we use uuid.uuid1() for session IDs, because that includes a MAC address in uuid.node (last 12 bytes).
2. Prevent remote (non-localhost) Consoles from shutting down the Kernel - I'm not sure this is a bad idea, but it does restrict use cases. It is certainly the easiest answer.

[fperez]

The rest looks good.

As for the last point you raise, go with #2. Remote users already have a way to shut down the kernel if they want to: type exit(). So there's no need to have the remote console even attempt at shutting down the kernels, ever.

See, easy is right :)

[minrk]

I worked out the accelerators.

Current status:
Close All is default (Enter)
Cancel is Esc
'Just Console' is the only one that's got non-standard behavior. I bound it to the native 'close' key-combo, which on OSX is cmd-W, since the difference between 'just this' and 'all' is the same as the difference between close and quit in regular apps.

But now that I've figured it out, all three can be anything we want.

[fperez]

Merge branch 'minrk-keepkernel' into trunk

Improvements to the closing behavior of a Qt console regarding what
happens to the kernel and other consoles that may be active. Now,
there is fine control on who is closed and who remains active.

Closed by dc30bee (pull request).