incompatibility with Twisted trunk due to setting self.transport.protocol

[Zooko]

​http://twistedmatrix.com/trac/ticket/5028

[davidsarah]

Twisted mailing list thread: ​http://twistedmatrix.com/pipermail/twisted-python/2011-April/023823.html

This should preferably be fixed before Twisted 11.1 is released, to stop it from affecting more users. Is there any chance that ​http://twistedmatrix.com/trac/ticket/3204 (supported API for changing the protocol) will be implemented by 11.1?

[Zooko]

Replying to davidsarah:

This should preferably be fixed before Twisted 11.1 is released, to stop it from affecting more users. Is there any chance that ​http://twistedmatrix.com/trac/ticket/3204 (supported API for changing the protocol) will be implemented by 11.1?

Glyph's mailing list message made it sound like it foolscap lovers would be the most likely implementors.

[Jean-Paul Calderone]

I think the fix will be quite simple (at least, one possible fix should be). However, there seem to be no Foolscap unit tests which fail against Twisted trunk@HEAD, so I can't easily test what I have in mind.

Here's a patch, though:

diff --git a/foolscap/negotiate.py b/foolscap/negotiate.py
index f74daba..13a21d8 100644
--- a/foolscap/negotiate.py
+++ b/foolscap/negotiate.py
@@ -1159,7 +1159,8 @@ class Negotiation(protocol.Protocol):
                              )
         b.factory = self.factory # not used for PB code
         b.setTub(self.tub)
-        self.transport.protocol = b
+        self.dataReceived = b.dataReceived
+        self.connectionLost = b.connectionLost
         b.makeConnection(self.transport)
         buf, self.buffer = self.buffer, "" # empty our buffer, just in case
         b.dataReceived(buf) # and hand it to the new protocol

[Brian Warner]

I'm trying to trigger the failure. Step one: you probably need pyOpenSSL-0.10 or newer installed, and OS-X 10.6 comes with 0.7 .

My hunch is that the root problem is that the new TLS code (which isn't always used, and it falls back to old code which works fine) is using a new protocol-wrapping mechanism. I'm guessing that the wrapper copies some attributes from .protocol early, such that it doesn't handle later changes correctly.

The patch above might be ok, but one feature I'd like to retain is that the Negotiation object should go away (be GCed) after the protocol is switched. I don't see how to do that with either this self.dataReceived = newprotocol.dataReceived approach, or the less-direct delegate-elsewhere approach used by AMP. It'd probably require a small intermediate Protocol on which intermediate.dataReceived = negotiator.dataReceived, then later gets set to =b.dataReceived. That would allow the (larger) Negotiate object to go away, retaining only the smaller intermediate object.

[Brian Warner]

Ok, fixed by [46e3a04]. I just applied exarkun's patch, plus a change to stop passing an argument into loseConnection (sometimes it works, but sometimes not). I'll defer the release-the-Negotiation-object change for a separate ticket (#183).