Okay, so twisted has a bunch of reactors. But, most of them aren't being tested. Currently being tested: Linux: select, poll, gtk, gtk2 OSX: select, cf Windows: select, win32event Currently untested platform/reactor combos that ought to work: Linux: glib2, qt, wx OSX: kq, qt, wx Windows: win32event, gtk, gtk2, glib2, qt, wx glib2reactor is really gtk2reactor, so, while it might be nice to test, it probably isn't very important, and the linux reactors test already takes a long time. However qtreactor really should be tested. Currently, cfreactor and IOCP reactor both seem to be in a state of very-brokenness. Itamar tells me wxreactor is also broken, but I have no knowledge of that. GTK and GTK2 reactors have some failures on BB, but I don't think they're in very bad shape really. Windows default reactor also currently has some failures, but I seem to recall it working right in the recent past (besides not being able to spawn processes). So: I propose removing cfreactor from OSX bb, removing IOCP from windows bb, adding win32eventreactor to windows bb, and qtreactor to linux bb. The reactors that we know are totally broken (As far as I know, the list is: IOCP, cf, wx) should be marked as such with warnings, and excluded from the release of twisted 2.0. James
On Wed, 17 Nov 2004 17:14:52 -0500, James Y Knight <foom@fuhm.net> wrote:
Okay, so twisted has a bunch of reactors. But, most of them aren't being tested.
Currently being tested: Linux: select, poll, gtk, gtk2 OSX: select, cf Windows: select, win32event
Currently untested platform/reactor combos that ought to work: Linux: glib2, qt, wx OSX: kq, qt, wx Windows: win32event, gtk, gtk2, glib2, qt, wx
glib2reactor is really gtk2reactor, so, while it might be nice to test, it probably isn't very important, and the linux reactors test already takes a long time. However qtreactor really should be tested.
Currently, cfreactor and IOCP reactor both seem to be in a state of very-brokenness. Itamar tells me wxreactor is also broken, but I have no knowledge of that. GTK and GTK2 reactors have some failures on BB, but I don't think they're in very bad shape really. Windows default reactor also currently has some failures, but I seem to recall it working right in the recent past (besides not being able to spawn processes).
So: I propose removing cfreactor from OSX bb, removing IOCP from windows bb, adding win32eventreactor to windows bb, and qtreactor to linux bb.
The reactors that we know are totally broken (As far as I know, the list is: IOCP, cf, wx) should be marked as such with warnings, and excluded from the release of twisted 2.0.
You may want to add Qt to the list of broken reactors. Quite some time ago, it had a place on the reactors buildslave. Due to gross breakage and a complete absence of interest in fixing it, it was removed. Since it is lacking a maintainer, I think it makes for a good candidate for some kind of deprecation in 2.0, for removal in a later release if no one steps up to maintain it. A similar case can probably be made for the other three reactors you mentioned: if no one plans to get them working, there's little point in keeping them around. I think the picture is somewhat better for IOCP and cf: their authors are still around, here and there, and there is actually a benefit to using them ;) I hope they can be fixed instead of removed. Wx is tough. It will be difficult to fix, and lacks a maintainer. On the other hand, wxWidgets and wxPython _should_ be getting much better in the medium-term future, and it's possible that the current difficults preventing wxreactor from working well will go away. In this case, the minimal effort on our part to update to new APIs (or whatever) is probably worth it, as wx is fairly popular. Of course, this does nothing to help the state it is in currently... Jp
On Thursday 18 November 2004 05:49, Jp Calderone wrote:
You may want to add Qt to the list of broken reactors. Quite some time ago, it had a place on the reactors buildslave. Due to gross breakage and a complete absence of interest in fixing it, it was removed.
Since it is lacking a maintainer
I wasnt aware of that. We are currently developing a PyQt application that uses Twisted, but using the a standard reactor in a seperate thread. That situation is less than satisfactory for us, so I may pick this up. Any other PyQt+Twisted users out there? -- Toby Dickenson
On Thu, 18 Nov 2004 08:14:31 +0000, Toby Dickenson <tdickenson@devmail.geminidataloggers.co.uk> wrote:
On Thursday 18 November 2004 05:49, Jp Calderone wrote:
You may want to add Qt to the list of broken reactors. Quite some time ago, it had a place on the reactors buildslave. Due to gross breakage and a complete absence of interest in fixing it, it was removed.
Since it is lacking a maintainer
I wasnt aware of that.
We are currently developing a PyQt application that uses Twisted, but using the a standard reactor in a seperate thread. That situation is less than satisfactory for us, so I may pick this up.
Any other PyQt+Twisted users out there?
Yes, I've used it in the past. I can see using it again considering that pygtk is not (yet) a satisfactory replacement on all platforms. I'm assuming qtreactor is non-trivial to fix. In that case I currently don't have the time nor motivation to do so. -Eric
On Nov 18, 2004, at 12:14 AM, James Y Knight wrote:
Currently, cfreactor and IOCP reactor both seem to be in a state of very-brokenness. Itamar tells me wxreactor is also broken, but I have no knowledge of that. GTK and GTK2 reactors have some failures on BB, but I don't think they're in very bad shape really. Windows default reactor also currently has some failures, but I seem to recall it working right in the recent past (besides not being able to spawn processes).
Very test-failness. I haven't seen a situation where cfreactor doesn't actually work in practice since it was changed to handle the GIL correctly. The way the reactor code is written (not very modularly) leaves only two options to make cfreactor cleaner: - have some naughty hacks in cfreactor [current situation] - subclass, refactor, or reimplement everything that deals with unix file descriptors and make some subtle changes so that they don't require said hacks [not interested in doing this] The problem is that the classes that use file descriptors make certain assumptions that are only true for the default (and probably poll) reactors. Particularly, from memory, they don't have to tell the reactor when they want to lose connection, and they do something very crazy in order to determine when a TCP connection is established. In the case of cfreactor, a wrapper object needs to live on top of the sockets so that the reactor will wake up on activity, and without proper notifications these objects can't be disposed of. I'm not so sure those hacks are why it fails tests, though. I believe the reason that cfreactor fails so many tests is because (a) reactor.run() doesn't necessarily block (a CFRunLoop may already be active) and (b) iteration is a big nasty hack, and nearly all the tests depend on (or at least they used to) reactor iteration having very specific and deterministic behavior. Anyway, I simply don't have time or need to maintain cfreactor, especially when it works for everything I've thrown at it. If you want to remove it, fine, I don't care that much, but I know a few people are using it. -bob
On Nov 18, 2004, at 3:58 AM, Bob Ippolito wrote:
Very test-failness. I haven't seen a situation where cfreactor doesn't actually work in practice since it was changed to handle the GIL correctly.
There are two serious issues I know about: 1) See "survive a re-entrant call to .resumeProducing, which happens under the cf reactor". That should definitely not happen. Unfortunately, there is no test case testing this at the moment. 2) Half-close support is unimplemented. James
On Nov 18, 2004, at 7:17 PM, James Y Knight wrote:
On Nov 18, 2004, at 3:58 AM, Bob Ippolito wrote:
Very test-failness. I haven't seen a situation where cfreactor doesn't actually work in practice since it was changed to handle the GIL correctly.
There are two serious issues I know about: 1) See "survive a re-entrant call to .resumeProducing, which happens under the cf reactor". That should definitely not happen. Unfortunately, there is no test case testing this at the moment.
If someone files a bug with a stack trace, I'm sure this would be easy to diagnose and fix. I have seen no such thing, so I can't fix it. It's probably resumeProducing calling doWrite calling resumeProducing, or something.. but I'm not going to attempt a fix-by-guess.
2) Half-close support is unimplemented.
There's three solutions to that: - Someone (other than me) can implement it. - Someone (other than me) can refactor the default file descriptor implementations so that cfreactor can be implemented cleaner and correctly share all this half-closing code. If this happens, I'll refactor cfreactor accordingly. - The half-close interface can be made optional. -bob
On Nov 18, 2004, at 3:58 AM, Bob Ippolito wrote:
The problem is that the classes that use file descriptors make certain assumptions that are only true for the default (and probably poll) reactors. Particularly, from memory, they don't have to tell the reactor when they want to lose connection, and they do something very crazy in order to determine when a TCP connection is established. In the case of cfreactor, a wrapper object needs to live on top of the sockets so that the reactor will wake up on activity, and without proper notifications these objects can't be disposed of.
Okay, I'm still unsure why this is necessary. Right now, the reactors just get told when it should send notifications for a socket being readable or writable. When a socket is closed, the reactor is told stopReading/stopWriting. For all other reactors, that causes all reactorstate associated with the socket to be disposed of. They just know the user no longer wants to see notifications, for whatever reason. That the reason is the socket closed doesn't matter. Why does the CF reactor have to care? And, if it _really does_, would it not be good enough to use a weakref to the Selectable with a callback? QTReactor seems to have a similar architecture, with objects that live on top of sockets, but it doesn't have the same crazy hacks. Of course, since it's also failing tests, I don't know if it's necessarily a good thing to point at. ;) James
On Nov 18, 2004, at 9:55 PM, James Y Knight wrote:
On Nov 18, 2004, at 3:58 AM, Bob Ippolito wrote:
The problem is that the classes that use file descriptors make certain assumptions that are only true for the default (and probably poll) reactors. Particularly, from memory, they don't have to tell the reactor when they want to lose connection, and they do something very crazy in order to determine when a TCP connection is established. In the case of cfreactor, a wrapper object needs to live on top of the sockets so that the reactor will wake up on activity, and without proper notifications these objects can't be disposed of.
Okay, I'm still unsure why this is necessary. Right now, the reactors just get told when it should send notifications for a socket being readable or writable. When a socket is closed, the reactor is told stopReading/stopWriting. For all other reactors, that causes all reactorstate associated with the socket to be disposed of. They just know the user no longer wants to see notifications, for whatever reason. That the reason is the socket closed doesn't matter. Why does the CF reactor have to care? And, if it _really does_, would it not be good enough to use a weakref to the Selectable with a callback?
It also calls (or at least used to call) stopReading and stopWriting immediately after a TCP client connection is made, and then calls startReading and startWriting again.. which causes the wrapper to get thrown away, so it might miss notifications in the meantime. I think it already uses weakrefs, but I was never confident that they would go away at the right time, and from recent python-dev threads, it doesn't sound like it's a good thing to depend on at all any time soon :) -bob
On Nov 18, 2004, at 6:29 PM, Bob Ippolito wrote:
It also calls (or at least used to call) stopReading and stopWriting immediately after a TCP client connection is made, and then calls startReading and startWriting again.. which causes the wrapper to get thrown away, so it might miss notifications in the meantime.
That shouldn't matter in a level-triggered system: it'll call back every time the socket is readable/writable, not just the first time. Thus, missing a notification because the user asked to not receive them for a while is okay.
I think it already uses weakrefs, but I was never confident that they would go away at the right time, and from recent python-dev threads, it doesn't sound like it's a good thing to depend on at all any time soon :)
I think all the problems were related to weakref objects being disposed of during GC. As long as all your weakrefs are strongly referenced from the module level, I believe they should be safe. James
On Wed, 2004-11-17 at 17:14 -0500, James Y Knight wrote:
The reactors that we know are totally broken (As far as I know, the list is: IOCP, cf, wx) should be marked as such with warnings, and excluded from the release of twisted 2.0.
Based on what Bob is saying the CF reactor is in same state as Qt, where it runs correctly but has issues with the tests, so I think we should keep it (and of course as with Qt investigate ways of fixing this). wx... dunno. It might work on some instances of wx backends. Or not. God only knows. IOCP: possibly this should be a separate project, so people can still download it, since even in its current state it's probably better than default win32 reactor in terms of scalability and will work correctly for a simple TCP server. Any comments, Pavel?
On Thu, 18 Nov 2004 12:04:57 -0500 Itamar Shtull-Trauring <itamar@itamarst.org> wrote:
IOCP: possibly this should be a separate project, so people can still download it, since even in its current state it's probably better than default win32 reactor in terms of scalability and will work correctly for a simple TCP server. Any comments, Pavel?
Sounds good to me. How would we deal with twisted.application.app.reactorTypes, though?
On Thu, 2004-11-18 at 09:51 -0800, Pavel Pergamenshchik wrote:
On Thu, 18 Nov 2004 12:04:57 -0500 Itamar Shtull-Trauring <itamar@itamarst.org> wrote:
IOCP: possibly this should be a separate project, so people can still download it, since even in its current state it's probably better than default win32 reactor in terms of scalability and will work correctly for a simple TCP server. Any comments, Pavel?
Sounds good to me. How would we deal with twisted.application.app.reactorTypes, though?
JP pointed out that we should probably use plugins.tml to register new reactors. There's an issue in the reactor somewhere.
On Thu, 2004-11-18 at 12:04 -0500, Itamar Shtull-Trauring wrote:
Based on what Bob is saying the CF reactor is in same state as Qt, where it runs correctly but has issues with the tests, so I think we should keep it (and of course as with Qt investigate ways of fixing this).
If these test failures are really not bad enough to keep things out of the release, why aren't they set to .todo, perhaps specifically in the case of the reactors which break them? In any event, a document clearly listing the limitations of all the reactors would be good. I vaguely remember we had one at some point. Is it up to date?
On Nov 18, 2004, at 12:04 PM, Itamar Shtull-Trauring wrote:
IOCP: possibly this should be a separate project, so people can still download it, since even in its current state it's probably better than default win32 reactor in terms of scalability and will work correctly for a simple TCP server. Any comments, Pavel?
Even if it isn't released in 2.0, I don't see any reason it can't stay in Twisted. If it is made to work after 2.0 release, we could release Twisted Core 2.1, with the main new feature being Pavel's awesome IOCP support. I'm not proposing to remove any reactors from SVN, just omit the currently non-working ones from the release tarball. But, as for it working right now for TCP: the buildlogs show a bunch of failures in twisted.mail.test.test_imap.NewFetchTestCase, which I assume is a "simple" (besides the IMAP part of course) TCP server, followed by the buildslave going belly-up. This seems to be consistent through all the buildlogs I looked at. http://www.twistedmatrix.com/buildbot/win32/builds/122/iocp/0 James
participants (8)
-
Bob Ippolito -
Eric Mangold -
Glyph Lefkowitz -
Itamar Shtull-Trauring -
James Y Knight -
Jp Calderone -
Pavel Pergamenshchik -
Toby Dickenson