Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPython kernel connections are unpredictable #977

Closed
spyder-bot opened this issue Feb 17, 2015 · 7 comments
Closed

IPython kernel connections are unpredictable #977

spyder-bot opened this issue Feb 17, 2015 · 7 comments

Comments

@spyder-bot
Copy link
Collaborator

From jed.lud...@gmail.com on 2012-03-26T18:11:13Z

What steps will reproduce the problem?

  1. Launch spyder
    1. Select "Interpreters...Start a new IPython kernel" What is the expected output? What do you see instead? A new kernel is, in fact, created, but the associated front end that is also created does not always attach to the right instance of the kernel. Note the process IDs in the attached screenshot. You'll see three separate JSON files referenced, all tied to process IDs. PID 6496 is actually the PID of the python interpreter that Spyder itself is running in. PID 3800 is the one that the front end should be attaching to but isn't. PID 8796 is actually the PID to which the front end attached even though it says that it attached to 6496 (that sounds bizarre, I know, but it's real). I can confirm that more than one IPython kernel is being created because I can attach to them with new front ends, and defining variables in one does not result in bindings in the other.

Once I get to the point displayed in the screenshot, it is possible to attach a new front end to PID 3800, and everything works correctly in that front end.

This behavior is not consistent. Sometimes the connection happens correctly, but many other times it does not. Sometimes one of the python processes that gets created ends up running after Spyder is shutdown, too, leaving behind a zombie.

I've tried to step through the code where the launch of the IPython kernel occurs, but I'm not sure my multi-threaded debugging skills are up to the task, particularly on code that's still a bit unfamiliar. Hopefully my detailed report can guide to right person to a solution. Please use labels and text to provide additional information. Spyder 2.2.0dev revision 1acb2f1d8324 Python 2.7.2
Windows 7
PyQt4 4.8.5

Attachment: IPython Kernel Connections.PNG

Original issue: http://code.google.com/p/spyderlib/issues/detail?id=977

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-03-26T16:19:56Z

By the way, the same behavior happens with PySide 1.1.0. I don't suspect the Qt bindings to be related to this issue.

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-03-26T22:54:09Z

After a bit more experimentation, it appears that introducing a little delay into the IPython kernel process start up appears to resolve the issue. The attached patch is a workaround and is clearly a dirty hack, but it produces reliable connection to the correct, single kernel instance. Maybe it gives additional clues as to the real root cause.

As an aside, is there a particular reason why the default IPython start up options have "python" in the string as shown here? https://code.google.com/p/spyderlib/source/browse/spyderlib/plugins/externalconsole.py#1114 Shouldn't this line be just "--pylab=inline"?

Attachment: ipython_patch.diff

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-03-29T22:11:24Z

I've isolated the root cause. A proposed solution is attached.

When we attempt to launch a new IPython kernel, the pythonshell process startup code blocks until the new external process has started here: https://code.google.com/p/spyderlib/source/browse/spyderlib/widgets/externalshell/pythonshell.py#502 But that's not sufficient with an IPython kernel since the startup code has to fully initialize the IPython kernel before the kernel can really accept connections: https://code.google.com/p/spyderlib/source/browse/spyderlib/widgets/externalshell/startup.py#30 The monitor thread communicates the alleged existence of a kernel as soon as it can find the ipythonkernel variable in the global namespace of the spawned external process: https://code.google.com/p/spyderlib/source/browse/spyderlib/widgets/externalshell/monitor.py#208 But the ipythonkernel variable exists before the kernel is totally initialized, and what often happens is that the connection_file string that gets sent out as the "data" variable is actually empty (not None, but u'') because the whole connection file creation process has yet to execute. That doesn't happen until this line completes: https://code.google.com/p/spyderlib/source/browse/spyderlib/widgets/externalshell/startup.py#32 The result is that the front end initialization process sometimes finds the right kernel connection file and sometimes it doesn't. It all depends on random delays in the OS. There must be some additional code inside the IPython Qt front end console that tries to spawn another kernel if it can't find one already running in the process your trying to connect to, and that's why you sometimes get the front end connecting to the wrong process. This all goes away with this fix.

So instead of just checking for the existence of ipythonkernel, we need another state variable that will indicate that the kernel has completed initialization. Once we've reached that point, the monitor can safely communicate that the kernel is ready for connections. I've added this state variable to the IPython startup code and the monitor thread.

I've tested this with both PyQt and PySide bindings on Windows 7, and it works well. Multiple kernels can be launched, and multiple front ends can be attached to each.

I've assigned this to Pierre since he's worked the most on the IPython kernel integration. I think he'd be able to provide the best QA on the proposed solution.

Status: Started
Owner: pierre.raybaut

Attachment: ipython_patch_better.diff

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-03-29T22:16:31Z

I should have added that the line numbers in the links to the code above apply at revision 6133f1ce134f .

@spyder-bot
Copy link
Collaborator Author

From pierre.raybaut on 2012-03-30T13:58:13Z

Thanks Jed for the fix and for the detailed explanations. I'll take a look at it ASAP.

@spyder-bot
Copy link
Collaborator Author

From pierre.raybaut on 2012-03-31T04:23:03Z

This issue was updated by revision 218fa962b17c .

Jed, actually, it is not necessary to modify the monitor script as this issue
can be simply solved by assigning the ipythonkernel reference after the
IPython kernel has been initialized. Thanks for debugging all this.

Please mark the issue as "Verified" once you have tested my fix.

Status: Fixed

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-03-31T07:25:39Z

Well that makes perfect sense :). After debugging this for so long I was blinded by an earlier attempt to set ipythonkernel after the start() method was called which, of course, does nothing at all since start() blocks forever. It was a day or so later that I was reading the IPython initialize() code and experimenting with the extra state variable and didn't make the final jump to realizing that your approach is the next logical step in simplifying the fix.

Status: Verified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant