> On Aug 20, 2016, at 2:13 AM, Adi Roiban <a...@roiban.ro> wrote: > > On 20 August 2016 at 08:37, Glyph Lefkowitz <gl...@twistedmatrix.com > <mailto:gl...@twistedmatrix.com>> wrote: >> >> On Aug 11, 2016, at 06:59, Adi Roiban <a...@roiban.ro> wrote: >> >> Hi, >> >> I have observed that when a SSL4ClientEndpoint was used, the >> protocol's transport `connected` attribute is not updated by the >> wrapper when the connection is lost. >> >> >> Hey Adi, >> >> I took a look through the code, and I think it's a lot simpler than you >> realize :-). >> >> My current finding is that the reactor will call connection lost on >> t.i.abstract.FileDescriptor.connectionLost and not on >> t.protocols.policies.ProtocolWrapper >> >> >> This is definitely not true, or dropping the connection just wouldn't work >> at all, and you wouldn't get connectionLost on your protocol. The call path >> is FileDescriptor.connectionLost -> endpoint wrapper protocol.connectionLost >> -> *TLS wrapper protocol.connectionLost* -> your protocol connectionLost. >> >> The bold-faced protocol is the interesting one, because that is the >> 'transport' object that your protocol sees. Here's that object's >> connectionLost method: >> >> https://github.com/twisted/twisted/blob/6022f65903eda2f8df74b2d284b8ef359df24904/src/twisted/protocols/tls.py#L488 >> >> As you can see, neither it, nor the superclass that it upcalls to >> <https://github.com/twisted/twisted/blob/6022f65903eda2f8df74b2d284b8ef359df24904/src/twisted/protocols/policies.py#L123> >> sets the "connected" attribute. So the method is totally called, it just >> doesn't do what you expect. >> >> so it seems that when SSL is used the wrapped protocol is registered >> instead of the wrapping protocol. >> >> >> I'm not sure what you mean here. >> >> I am doing something wrong here? >> >> >> Is this the expected behavior or this is a bug? >> >> >> Probably both, kinda? This attribute is undocumented; probably an old >> implementation accident. Technically it's public, but you probably >> shouldn't be depending on it. However, it's silly that this is inconsistent >> between TLS transports and raw TCP transports, so fixing up >> ProtocolWrapper.connectionLost to set 'connected' to False is probably not a >> bad change. > > Thanks for your comments. > > I asked the question about SSL4ClientEndpoint in parallel with the > question about twisted.internet.protocol.Protocol.connected ... so by > that time I was very confused about all this. > > I have created a ticket for this > https://twistedmatrix.com/trac/ticket/8774 > <https://twistedmatrix.com/trac/ticket/8774> and will try to work on it. > > I have searched to code for `self.connected =` and there are a few of > places where it is not set at connection lost: > > INotify, PTYProcess, udp.Port (not sure why we have this state for > UDP) , unix.Port (but is set to 0 for unix.DatagramPort) > iocpreactor.udp.Port (but is set to False for iocpreactor.tcp.Port) > > I will try to follow up with a ticket+branch. > > Thanks!
No problem! Glad we could clear that up - it was definitely a bit confusing at first for me as well, which is why I wanted to investigate to the bottom of it :). Another option here, though, is to simply deprecate the attribute; like I said, it's not documented and if you're properly implementing everything by reacting to events, it's not very useful. Why do you need it? -glyph
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python