Opened 12 years ago

Closed 10 years ago

Last modified 7 months ago

#1235 defect closed fixed (fixed)

Change wxreactor to use threadedselectreactor

Reported by: Jean-Paul Calderone Owned by:
Priority: high Milestone:
Component: core Keywords:
Cc: Glyph, Cory Dodt, radix, Jean-Paul Calderone, itamarst, etrepum, Alejandro J. Cura, moof Branch:

Attachments (1)

wxdemo.txt (2.6 KB) - added by itamarst 11 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 Changed 12 years ago by Jean-Paul Calderone

The public API for wxreactor stays the same as the rest of the reactors, but is
implemented in terms of threadedselectreactor's interleave method plus wx's

Also, rename threadedselectreactor so that it is private, since it exposes the
wrong API for a reactor.

Also, figure out if Bob cares about cfreactor.  If not, deprecate/remove it.

Also, figure out if Bob's Cocoa example can be re-implemented without
non-framework calls to interleave().  If so, and if we remove the current
cfreactor, make a cfreactor that internally relies on threadedselectreactor
(just like wxreactor).

comment:2 Changed 12 years ago by etrepum

Kill cfreactor.  The cocoa examples are trivially refactored to use 
threadedselectreactor (if they aren't already).

Note that threadedselectreactor does have the reactor API also, but it's not at 
all interesting since using it as one ends up with a two threaded selectreactor.

Don't make a cfreactor that internally relies on threadedselectreactor.  
cfreactor *needs* the interleave API because Twisted *can not* start a Cocoa 
app.  cfreactor's run method actually was non-blocking in most situations, like 

comment:3 Changed 12 years ago by Glyph

Added #1241 (assigned to him) to reflect bob's concerns

comment:4 Changed 12 years ago by itamarst

It's not clear to me why the proposed tsr-based cfreactor can't
call the cf start loop command and have interleave() called in the appropriate
place (as wxreactor does now and will when it uses threadedselectreactor).
Wouldn't that have the same effect as users doing that manually?

comment:5 Changed 12 years ago by etrepum

Because you don't call a start loop command, the loop is already running by the 
time you have control.

comment:6 Changed 12 years ago by itamarst

Some issues with tsr: 
1. reactor shutdown events don't work at all without extra event loop specific work
2. the thread uses setDaemon(True), bleh

I have fixed these for my soon to be added tsr-using wxreactor.

comment:7 Changed 12 years ago by itamarst

in the new wxreactor the calls wxapp.MainLoop(), couldn't a
cfreactor run() call the appropriate CF function?

comment:8 Changed 12 years ago by etrepum

Not really.  It just can't work like that, tsr exists *because* the traditional 
twisted-on-the-outside approach is not appropriate in this case (and others).

comment:9 Changed 12 years ago by Glyph

Let's just get the wx issues resolved first, before we drag CF into it.  CF
seems to have a much more involved mainloop situation.

comment:10 Changed 12 years ago by itamarst

The re-implemented wxreactor I did using tsr is in /branches/itamar/branch-1235
- someone should take a look and merge over the weekend. If you can get it to
run tests that would be even better; at the very least you'd need to have a top
frame registered or something or the mainloop exits when all windows exit.
SetExitOnFramesClose(False) or whatever that API is called might prevent that
though, you could try that in the DummyApp that gets installed if the user doesn't.

comment:11 Changed 12 years ago by radix

Well, given that 1. nobody seems to be happy with this 2. tsr seems to be in a
nasty state, and 3. wx has been broken forever, I don't think I'm going to
consider this release-blocking.

comment:12 Changed 12 years ago by itamarst

I do think we should merge this for 2.1, the whole point of this is to keep
people from using the current tsr, by making wxreactor use it behind the scenes.
Actually that bit is already done, this just makes wxreactor-using-tsr suck less.

So, wxreactor-using-tsr is *already* in 2.1. By merging this we'll make it work

comment:13 Changed 12 years ago by itamarst

Oh, I see you released it. Now I have to write an email to the list sayig "the
correct way to use wx in 2.1 is using wxreactor but btw it's also has broken
signal handling and shutdown support." Can we have a 2.1.1? :)

comment:14 Changed 12 years ago by itamarst

Actually, my apologies, releasing without this was the right thing to do. I will
email the list a summary of our current status and what needs doing (by us and
by users) on Monday.

comment:15 Changed 12 years ago by moof

Tested on windows XP SP2, python 2.4.1, wxpython

* File/Close Closes
* The Close Box Closes
* Alt+F4 Closes
* Alt f x Closes
* Ctrl-C from the command console closes
* Ctrl-C with the wx window in focus does not close (which is, I assume,
correct, so you can get copy and pasteworkign normally)
* Ctrl-Z from the command console *Does Not Close.* I am unsure as to whether
this is a bug or not. It's *supposed* to send some sort of signal to python, but
whether python catches it as a KeyboardInterrupt is a different matter.
* Sending and End Task message from the Task Manager Applications tab closes
gracefully (shows the shutting down... message)
* Terminating the python process from the processes tab kills python without
shutting down gracefully. This is correct.

comment:16 Changed 12 years ago by moof

I forgot to add:

Hello World keeps printing throughout the use of the menus and dialog box.

comment:17 Changed 12 years ago by Cory Dodt

Tested on mac os X.3.

"hello, world" kept scrolling while the dialog was open.  Alt-Q and File>Exit
both trigger (2,1,0) output in the console.  Unfortunately ^C in the console
causes "^C" to appear in the console, and the program exits immediately.

comment:18 Changed 12 years ago by Alejandro J. Cura

Tested on Windows XP SP2, python 2.4.1 with wxPython2.6.1.0 then with wxPython
Everything ok, as reported by moof

Tested on Mandrake 10.1, python 2.4, wxPythonGTK
Everything works fine, but closing the window, both with Alt-f4 or the wm close
button does not work ok. It seems to hang up the reactor, because I see in the
log "shutting down in 0.3 seconds" immediately, but it takes a few seconds to
see "2..", "1...", "0..." (they show up together) then it hangs up and I need to
killall -1 python

comment:19 Changed 12 years ago by itamarst

I tested on Ubuntu with wxPython/wxWidgets 2.5.3 and it worked... so I'm not
sure what the problem with Mandrake is.

comment:20 Changed 12 years ago by itamarst

Another bug report: wxPython 2.4 has no wxEventLoop.

comment:21 Changed 12 years ago by Cory Dodt

I for one wouldn't care if wx 2.6 was required.

Changed 11 years ago by itamarst

Attachment: wxdemo.txt added

comment:22 Changed 11 years ago by itamarst

Comment and attachment from another tester, via email:

I have tested the svn version of and doc/core/example/
with Twisted 2.1. No problem with Windows (XP SP2) and Linux (Fedora Core 4)
both using wxPython
However there are problems with Mac (OS X 10.4.3) using wxPython
I have attached the trace I get on the Mac when closing with the close
button of the window manager.
When I quit with CTRL+C in the terminal and the Quit option of the menu, I get
the same trace except that 2 1 and 0 are displayed before the error. "Hello
world" is not printed continously when I move the mouse in the menu or when I
click ok in the dialog without releasing it.
Please let me know if I can be of any further help. Thank you.
Maxime Vernier

comment:23 Changed 11 years ago by Jean-Paul Calderone

Component: conch

Merged this branch forward to threaded-wxreactor-1235

comment:24 Changed 11 years ago by Jean-Paul Calderone

Component: conchcore

comment:25 Changed 10 years ago by itamarst

Keywords: review added
Owner: changed from itamarst to Jean-Paul Calderone

comment:26 Changed 10 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to itamarst

Tested on Linux and Windows and OS X. Linux and Windows seem to work well with this version. OS X has all manner of problems, but they are probably all due to bugs in wxPython/Widgets that we can't do anything about. This also isn't any worse than the previous version on OS X, so there's no reason to block the Linux/Win32 improvements on this.

There should be a list of acceptance tests for wx, since we don't have unit tests for this. currently serves as the basis for this - it should be split into two pieces, one for acceptance testing and one which is actually a demo for people who want to know how to use wx with Twisted and leaves out the various things which are primarily useful for determining whether or not wxreactor works. For the acceptance version, change the output so it is more obvious when events are not being processed.

Do some whitespace cleanup on whatever files were changed and need it. Remove the unnecessary change near the SystemExit handling in Also add docstrings to all the methods in

comment:27 Changed 10 years ago by Jean-Paul Calderone

Further investigation reveals a wx.Timer with a Notify method which calls ProcessPendingEvents on the App instance seems to cause Twisted events to be processed properly on OS X. Adding this to wxreactor (when running on OS X) would seem to make it work completely on all three platforms.

comment:28 Changed 10 years ago by itamarst

Description: modified (diff)
Keywords: review added
Owner: changed from itamarst to Jean-Paul Calderone

comment:29 Changed 10 years ago by itamarst

Description: modified (diff)

Review comments have been addressed on a branch; please test new OS X fix. I won't have time to merge this week since I'm out of town, so I'd appreciate it if someone else would merge before 2.5 if it passes review.

comment:30 Changed 10 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [19070]) Merge wxreactor-1235+1574+1688

Author: itamar Reviewer: exarkun Fixes #1235 Fixes #1574 Fixes #1688

This fixes numerous problems with wxreactor:

  • startup and shutdown events are now supported
  • signal handlers will not be installed if installSignalHandlers=False is passed to run
  • a manual acceptance test has been added so the level of workingness of wxreactor can be examined

This also fixes the problem where modal GUI actions (displaying menus, modal dialogs, resizing windows, etc) would block Twisted events from being processed. On Linux, Windows, and OS X Twisted events will now be serviced in these cases.

Additionally, _threadedselect no longer creates daemon threads and the wxdemo has been simplified by the removal of code which was only in support of acceptance testing (which is now a separate program).

comment:31 Changed 6 years ago by <automation>

Owner: Jean-Paul Calderone deleted

comment:32 Changed 7 months ago by hawkowl

Keywords: review removed

[mass edit] Removing review from closed tickets.

Note: See TracTickets for help on using tickets.