Ticket #3011 (closed defect: fixed )

Opened 1 year ago

Last modified 1 year ago

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

Reported by: radix Assigned to: radix
Type: defect Priority: normal
Milestone: Component: core
Keywords: Cc:
Branch: branches/gtk2-callWhenRunning-stop-3011 Author: radix
Launchpad Bug:

Description

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.

Attachments

Change History

  2008-01-24 22:26:55+00:00 changed by 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.

  2008-01-24 22:55:32+00:00 changed by radix

  • branch set to branches/gtk2-callWhenRunning-stop-3011
  • author set to radix

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

  2008-01-24 23:13:26+00:00 changed by radix

  • keywords set to review

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.

  2008-01-24 23:13:52+00:00 changed by radix

  • owner deleted

  2008-01-25 13:08:18+00:00 changed by exarkun

  • owner set to exarkun
  • status changed from new to assigned

  2008-01-25 13:44:44+00:00 changed by exarkun

  • keywords deleted
  • owner changed from exarkun to radix
  • status changed from assigned to new

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.

  2008-02-29 20:04:32+00:00 changed by radix

  • keywords set to review
  • owner changed from radix to exarkun

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

  2008-03-04 20:57:49+00:00 changed by exarkun

  • keywords deleted
  • owner changed from exarkun to radix

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.

  2008-03-04 21:41:35+00:00 changed by radix

  • status changed from new to closed
  • resolution set to fixed

(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

Note: See TracTickets for help on using tickets.