[Twisted-Python] pending twisted fixes
Hello, These are the pending fixes for twisted that I'm maintaining on my tree. On the client side to still allow 1.3.0 I've a file called twisted_bugs.py and I import from there instead of twisted.* depending on twisted.copyright.version. I've found very easy to fix a single bug locally by cut-and-pasting a twisted function and by fixing it locally while still not altering the 2.0 behaviour, it's pratically a local 2.0 backport which lives in its own file. Perhaps this would have been a better approach than an upgrade to 2.0 in the rogers.com incident. Hotfixes seems quite easy this way and sure less invasive than a full upgrade (i.e. no risk of API breakage and no annoyance to the sysadm). This is just a reminder just in case other incident happens that seems to require 2.0 upgrade. from twisted.copyright import version twisted_bug_workaround = False if '1.3.0' in version: twisted_bug_workaround = True from twisted.internet import reactor # to stop it if not twisted_bug_workaround: from twisted.protocols.basic import Int32StringReceiver from twisted.internet.protocol import ReconnectingClientFactory else: from cpushare.twisted_bugs import Int32StringReceiver from cpushare.twisted_bugs import ReconnectingClientFactory Full working example of the 1.3.0 hotfixes is available in the download section of my site. The transport.disconnecting was said not to be available for all twisted transports, but then it should be added to the others instead of my rejecting my patch with that argument (those basic.py protocols are broken without this fix, so it's better that they fail with a visible exception than subtly at runtime), or another method should be implemented to know if the transport has disconnected already. Keep up the great work, thanks! Index: Twisted/twisted/protocols/basic.py =================================================================== --- Twisted/twisted/protocols/basic.py (revision 13717) +++ Twisted/twisted/protocols/basic.py (working copy) @@ -166,9 +166,23 @@ """ return error.ConnectionLost('Line length exceeded') +class PauseProducer(object): + paused = False + def pauseProducing(self): + self.paused = True + self.transport.pauseProducing() -class LineReceiver(protocol.Protocol): + def resumeProducing(self): + self.paused = False + self.transport.resumeProducing() + self.dataReceived('') + + def stopProducing(self): + self.paused = True + self.transport.stopProducing() + +class LineReceiver(protocol.Protocol, PauseProducer): """A protocol that receives lines and/or raw data, depending on mode. In line mode, each line that's received becomes a callback to @@ -188,7 +202,6 @@ __buffer = '' delimiter = '\r\n' MAX_LENGTH = 16384 - paused = False def clearLineBuffer(self): """Clear buffered data.""" @@ -279,21 +292,8 @@ """ return self.transport.loseConnection() - def pauseProducing(self): - self.paused = True - self.transport.pauseProducing() - def resumeProducing(self): - self.paused = False - self.dataReceived('') - self.transport.resumeProducing() - - def stopProducing(self): - self.paused = True - self.transport.stopProducing() - - -class Int32StringReceiver(protocol.Protocol): +class Int32StringReceiver(protocol.Protocol, PauseProducer): """A receiver for int32-prefixed strings. An int32 string is a string prefixed by 4 bytes, the 32-bit length of @@ -314,7 +314,7 @@ """Convert int32 prefixed strings into calls to stringReceived. """ self.recvd = self.recvd + recd - while len(self.recvd) > 3: + while len(self.recvd) > 3 and not self.paused and not self.transport.disconnecting: length ,= struct.unpack("!i",self.recvd[:4]) if length > self.MAX_LENGTH: self.transport.loseConnection() @@ -331,7 +331,7 @@ self.transport.write(struct.pack("!i",len(data))+data) -class Int16StringReceiver(protocol.Protocol): +class Int16StringReceiver(protocol.Protocol, PauseProducer): """A receiver for int16-prefixed strings. An int16 string is a string prefixed by 2 bytes, the 16-bit length of @@ -351,7 +351,7 @@ """Convert int16 prefixed strings into calls to stringReceived. """ self.recvd = self.recvd + recd - while len(self.recvd) > 1: + while len(self.recvd) > 1 and not self.paused and not self.transport.disconnecting: length = (ord(self.recvd[0]) * 256) + ord(self.recvd[1]) if len(self.recvd) < length+2: break Index: Twisted/twisted/lore/latex.py =================================================================== --- Twisted/twisted/lore/latex.py (revision 13717) +++ Twisted/twisted/lore/latex.py (working copy) @@ -104,7 +104,7 @@ baseLevel = 0 try: - diaHack = not not os.popen('which dia').read() + diaHack = not not os.popen('which dia 2>/dev/null').read() except: # That's a no, then. diaHack = 0
Andrea Arcangeli wrote:
These are the pending fixes for twisted that I'm maintaining on my tree.
Thanks for reporting these, especially the fix backports. I guess Twisted is a big enough project now that we need to start doing maintenance release trees!
Hmm. True enough. I've seen this problem before but not thought too hard about it. Considering it now, I think the right solution is to add a 'disconnecting' attribute to ITransport, and fix all current Twisted ITransport implementations to include it. I know that this is about to become an issue for me with Q2Q. (Also, should we maybe have a distinction between IStreamTransport and IDatagramTransport, rather than just ITCPTransport/IUDPTransport? or at least some attributes to introspect, like ".reliable" and ".ordered"?)
On Thu, May 12, 2005 at 01:02:14AM -0400, Glyph Lefkowitz wrote:
You're very welcome! Thanks for the great twisted work.
Twisted is a big enough project now that we need to start doing maintenance release trees!
So far my project has been more experimental than twisted, so on the server side I'm surfing the SVN wave of all projects I depend on ;). Lack of maintainance release is not a big deal for me as long as SVN trunk remains usable in semi-production. I keep all installed versions in a backup directory, so I can downgrade to the previous binaries of all packages in a few seconds if something breaks. On the client side hotfixing has been so easy to add that perhaps a more official list of all potential serious bugs might be enough too. So if one is affected he can apply the hotfix without even requiring the end user to upgrade (i.e. avoiding the annoyance from the admin at the rogers incident). Infact I was surprised about the rogers decision of rewriting in pure python. I would understand pure C/C++ to boost performance and increase the developer base, but pure python isn't going to be better than python+twisted. I guess they're more used to thread programming than async driven programming, but thread programming is generally more complex than async programming. And async programming is quite close to thread programming, it's simpler because it's like if we were to hold implicitly a big lock before rescheduling the next context. On a related note, I wonder how complex it would be to really support multiple threads listening in epoll and requiring each context to take its locks, so then different contexts could scale in smp. This would break all apps, but it could be an optional mode that the app enables only if it supports multithreading. But anyway I don't need it right now, this is just a random idea, performance is more than good enough, if there's a performance problem it's only in the interfaces and apparently that has been partly addressed by taking zope.interfaces.
This sounds fine with me ;)
Abstracting this bit at the highlevel sounds good too. .stream may also be a good candidate. .reliable and .ordered are two of the properties that a stream has, the syscall uses SOCK_STREAM too. Thanks!
Andrea Arcangeli wrote:
These are the pending fixes for twisted that I'm maintaining on my tree.
Thanks for reporting these, especially the fix backports. I guess Twisted is a big enough project now that we need to start doing maintenance release trees!
Hmm. True enough. I've seen this problem before but not thought too hard about it. Considering it now, I think the right solution is to add a 'disconnecting' attribute to ITransport, and fix all current Twisted ITransport implementations to include it. I know that this is about to become an issue for me with Q2Q. (Also, should we maybe have a distinction between IStreamTransport and IDatagramTransport, rather than just ITCPTransport/IUDPTransport? or at least some attributes to introspect, like ".reliable" and ".ordered"?)
On Thu, May 12, 2005 at 01:02:14AM -0400, Glyph Lefkowitz wrote:
You're very welcome! Thanks for the great twisted work.
Twisted is a big enough project now that we need to start doing maintenance release trees!
So far my project has been more experimental than twisted, so on the server side I'm surfing the SVN wave of all projects I depend on ;). Lack of maintainance release is not a big deal for me as long as SVN trunk remains usable in semi-production. I keep all installed versions in a backup directory, so I can downgrade to the previous binaries of all packages in a few seconds if something breaks. On the client side hotfixing has been so easy to add that perhaps a more official list of all potential serious bugs might be enough too. So if one is affected he can apply the hotfix without even requiring the end user to upgrade (i.e. avoiding the annoyance from the admin at the rogers incident). Infact I was surprised about the rogers decision of rewriting in pure python. I would understand pure C/C++ to boost performance and increase the developer base, but pure python isn't going to be better than python+twisted. I guess they're more used to thread programming than async driven programming, but thread programming is generally more complex than async programming. And async programming is quite close to thread programming, it's simpler because it's like if we were to hold implicitly a big lock before rescheduling the next context. On a related note, I wonder how complex it would be to really support multiple threads listening in epoll and requiring each context to take its locks, so then different contexts could scale in smp. This would break all apps, but it could be an optional mode that the app enables only if it supports multithreading. But anyway I don't need it right now, this is just a random idea, performance is more than good enough, if there's a performance problem it's only in the interfaces and apparently that has been partly addressed by taking zope.interfaces.
This sounds fine with me ;)
Abstracting this bit at the highlevel sounds good too. .stream may also be a good candidate. .reliable and .ordered are two of the properties that a stream has, the syscall uses SOCK_STREAM too. Thanks!
participants (4)
-
Andrea Arcangeli -
Bob Ippolito -
Glyph Lefkowitz -
Robin Bryce