Always run the OS X loop even if no windows. #10150

Merged
merged 1 commit into from Jan 13, 2017

Projects

None yet

3 participants

@Carreau
Member
Carreau commented Jan 12, 2017

Otherwise this can trigger infinite Python-Icon-In-Dock bouncing.
See #10137. I will guess that this is because application on OS X may
not have windows and still need to process events.

It may be that an alternative is to run the loop only once the first
time, but I'm unsure.


Ping @minrk : I have no clue what I am doing.

@Carreau Carreau Always run the OS X loop even if no windows.
Otherwise this can trigger infinite Python-Icon-In-Dock bouncing.
See #10137. I will guess that this is because application on OS X may
not have windows and still need to process events.

It may be that an alternative is to run the loop only once the first
time, but I'm unsure.

    at_least_once = False

    def inputhook(context):
	"""Inputhook for Cocoa (NSApp)"""
	NSApp = _NSApp()
	window_count = msg(
	    msg(NSApp, n('windows')),
	    n('count')
	)
	if not window_count and not at_least_once:
	    at_least_once = True
	    return
	_stop_on_read(context.fileno())
	msg(NSApp, n('run'))
	if not _triggered.is_set():
	    # app closed without firing callback,
	    # probably due to last window being closed.
	    # Run the loop manually in this case,
	    # since there may be events still to process (#9734)
	    CoreFoundation.CFRunLoopRun()

Closes #10137
260e988
@Carreau Carreau added this to the 5.2 milestone Jan 12, 2017
@minrk
Member
minrk commented Jan 12, 2017

Thanks for working on this one! I'll have a look tomorrow. I'll have to refresh my memory on the last few bugs we worked out on this stuff, in case this is trading a new bug for an old one.

@minrk
Member
minrk commented Jan 13, 2017

Appears to work, and isn't related to the previous bugfixes on this tricky one. Thanks!

@minrk minrk merged commit 5b96f50 into ipython:master Jan 13, 2017

4 checks passed

codecov/patch Coverage not affected when comparing efc9e76...260e988
Details
codecov/project 71.75% (+0.00%) compared to efc9e76
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Carreau
Member
Carreau commented Jan 13, 2017
@meeseeksdev meeseeksdev pushed a commit that referenced this pull request Jan 13, 2017
@minrk minrk + MeeseeksDev[bot] Backport PR #10150: Always run the OS X loop even if no windows.
Otherwise this can trigger infinite Python-Icon-In-Dock bouncing.
See  10137. I will guess that this is because application on OS X may
not have windows and still need to process events.

It may be that an alternative is to run the loop only once the first
time, but I'm unsure.

----

Ping  minrk : I have no clue what I am doing.
83d725b
@minrk
Member
minrk commented Jan 13, 2017

Cool! Can the auto-backport copy the milestone, as well?

@Carreau
Member
Carreau commented Jan 13, 2017

Cool! Can the auto-backport copy the milestone, as well?

It could, I added an issue.

@Carreau Carreau deleted the Carreau:fix-osx-bounce branch Jan 13, 2017
@Carreau Carreau added the backported label Jan 29, 2017
@asmeurer
Contributor
asmeurer commented Feb 7, 2017

I was using this inputhook in my own prompt_toolkit application. Before this patch, it worked just fine. After this patch, it causes my application to be completely unresponsive to all keyboard input.

@asmeurer
Contributor
asmeurer commented Feb 7, 2017

Your suggested code from the commit message works for me. I can't reproduce the original bouncing Dock issue with the Anaconda Python, so I can't say how it affects that.

@Carreau
Member
Carreau commented Feb 7, 2017

Hum, sorry about that. We would need to better understand all the interactions and when to do what. Event loop are hard.

@asmeurer
Contributor
asmeurer commented Feb 7, 2017

Oh, scratch that your commit message fix doesn't work (didn't save the file correctly 😳).

@Carreau
Member
Carreau commented Feb 7, 2017

Oh, scratch that your commit message fix doesn't work (didn't save the file correctly 😳).

it's ok, GitHub have a delete comment button if needed.

@asmeurer
Contributor
asmeurer commented Feb 7, 2017

Well I'm not ashamed of making a mistake. And it already sent the email, so it would just be confusing to you.

@minrk
Member
minrk commented Feb 7, 2017

@asmeurer do you have an example that doesn't behave right?

@asmeurer
Contributor
asmeurer commented Feb 8, 2017

Even a dead simple prompt-toolkit shell reproduces the issue:

#!/usr/bin/env pythonw

# Basic REPL
from traceback import print_exc

from prompt_toolkit.shortcuts import prompt, create_eventloop

from IPython.terminal.pt_inputhooks.osx import inputhook

_globals = _locals = globals().copy()

while True:
    command = prompt(">>> ", eventloop=create_eventloop(inputhook))
    try:
        res = eval(command, _globals, _locals)
        print(repr(res))
    except SyntaxError:
        try:
            exec(command, _globals, _locals)
        except BaseException as e:
            print_exc()
    except BaseException as e:
        print_exc()

With the latest IPython, the shell hangs at startup. It doesn't accept any input, and you have to kill it with kill.

If you use an IPython before this change, it works perfectly. Here's how I've been testing "perfectly", running this at the command line:

>>> import matplotlib
>>> matplotlib.interactive(True)
None
>>> import matplotlib.pylab as plt
>>> plt.plot([1, 2])
[<matplotlib.lines.Line2D object at 0x10e09db70>]
>>>

Things it should do:

  • The plot should popup immediately, with a line.
  • The prompt should not hang (it should show the next >>> immediately, before closing the plot).
  • The plot should be interactive (try clicking and using one of the toolbar buttons).
  • Closing the plot should work.

Ideally, I could just import this code directly from IPython (for now, I have copied the code without this change), although I understand that IPython only needs to support IPython. I realise that it works fine in IPython itself, but I'd like at least to understand why. Plus it sounds like you weren't really sure about this change to begin with, so maybe there is something wrong with it, even from the IPython side.

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