reactor.callWhenRunning(reactor.stop); reactor.run() doesn't work on glib2reactor

[radix]

callWhenRunning generally works, but for some reason you cannot effectively shut down the reactor with it in glib2reactor. It's clearly being invoked and getting the reactor into the "I am not running" state, but reactor.run() doesn't return.

Python 2.4.4 (#2, Jan  3 2008, 14:46:51) 
[GCC 4.2.3 20071223 (prerelease) (Ubuntu 4.2.2-4ubuntu3)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from twisted.internet.glib2reactor import install
>>> install()
>>> from twisted.internet import reactor
>>> reactor.callWhenRunning(reactor.stop)
('startup', ('after', <bound method Glib2Reactor.stop of <twisted.internet.glib2reactor.Glib2Reactor object at 0xb7d5f0cc>>, (), {}))
>>> reactor.run()

If I hit ctrl+c here, I get a "can't stop reactor that isn't running" exception.

[radix]

Looks pretty obvious; this reactor translates 'crash' to gobject.MainLoop.quit and starts with MainLoop.run. reactor.run calls BaseReactor.startRunning, which fires the startup trigger, which executes the callWhenRunning calls, thus causing MainLoop.quit to be called before MainLoop.run is called.

[radix]

(In [22392]) Branching to 'gtk2-callWhenRunning-stop-3011'

[radix]

Ok, ready for review. The fix is stupid, but it was TDDed and everything works hooray.

I'm sad that I couldn't write this test such that it would run against all reactor classes, but that was too hard to do in 20 minutes.

[exarkun]

Boring stuff:

• Put a test-case-name on gtk2reactor.py
• update copyright date on gtk2reactor.py
• put a copyright on test_gtk2reactor.py (and on the new __init__.py?)
• In the test method, youSuck could have a better name
• test_gtk2reactor.py will fail to import if the gtk2 dependencies aren't available.
• I thought we didn't like should in test method docstrings?

Less boring:

• reactor.callLater(0.1, ...) is sad making. Using a delay of 0 instead of 0.1 makes this better, I think (although maybe still not awesome). A delay of 0 would let you make the test method docstring more precise as well. Instead of immediately, it could say something like without processing any timed events. I'd like to tack or network events on there too, but then the test would have to set up some network stuff (actually that wouldn't be too hard, it could put a pipe with bytes in it into the reactor with addReader and notice if doRead gets called - but maybe it's still not a good idea).
• The test leaks file descriptors. Apparently reactors don't clean up their wakers when they shut down. Since the waker uses pipes, nothing else is going to close them due to GC or anything.

[radix]

I've made these changes. I've explicitly cleaned the waker in the unit test so as to not leak FDs, but I didn't solve the systemic problem because it was getting too unwieldy. I've created a ticket for it at #3063.

I've run the tests on winxp32-py2.5-select, debian-py2.4-reactors, and debian-py2.3-select. They pass.

PLS REVIEW

[exarkun]

Looks good. I'd add a comment about how it'd be better if the test weren't gtk2reactor-specific (since the behavior isn't supposed to be). Merge at your pleasure.

[radix]

(In [22744]) Merge gtk2-callWhenRunning-stop-3011

reactor.callWhenRunning(reactor.stop) when using glib2 or gtk2 reactor now works as documented.

Author: radix Reviewer: exarkun Fixes #3011