On Fri, Dec 16, 2016 at 8:27 PM, Glyph Lefkowitz <gl...@twistedmatrix.com> wrote: > >> On Dec 16, 2016, at 5:22 AM, Cory Benfield <c...@lukasa.co.uk> wrote: >> >>> On 15 Dec 2016, at 12:07, exvito here <ex.vitor...@gmail.com> wrote: >>> >>> Dear all, >>> >>> The subject says most of it. While diagnosing a behavior on a somewhat >>> large codebase, I found out something somewhat surprising -- see >>> subject -- which is replicated with the minimal code example below. >>> >>> It is a LineReceiver client that: >>> 1. Connects to 127.0.0.1:10000. >>> 2. Sends a line of text. >>> 3. Disconnects one second later. >>> 4. Tries to send another line of text, one second after disconnection. >>> >>> I expected step 4 to fail somehow, given that Twisted promptly >>> detected that the connection closed and IProtocol.connectionLost was >>> called, as documented. Nevertheless, it does not fail. >> >> I can’t speak for the design of Twisted, but this is definitely an intended >> implementation behaviour: because twisted’s write method is non-blocking, it >> is generally nicer to avoid write exploding in your face when disconnected. > > I can! The main design feature of this approach is that if you have a loop > like this: > > for x in messages: > self.transport.write(self.encode(x)) > > you should not have to write it to be: > > for x in messages: > if self._flagISetInConnectionLost: > break > self.transport.write(self.encode(x)) > > just to avoid seeing tracebacks in your logs. > > If you care about the disconnection event, implement connectionLost to do the > thing that needs to be done (in this case, stop your LoopingCall). That's > what that callback is there for!
Thanks for your input Cory and Glyph. I do agree that a well written protocol should not self.transport.write after connectionLost -- it does not make any kind of sense to do that. It's just that the one I was debugging was doing it in its app-level keep alive messages triggered by IReactorTime.callLater which wasn't properly cancelled on connectionLost. This, in turn, lead to unpexpected app-level keep alive timeouts after disconnection which, while acceptable (depending on app/protocol design), were having a negative impact on the overall connection/protocol teardown and cleanup. Having said this, the fact that self.transport.write does not complain after connectionLost just made my analysis and debugging more difficult (yes, I was aware that it is non-blocking -- thanks Cory -- and that when talking to the network the only confirmation of delivery must be obtained from a received message stating that fact). All in all, I was just looking for confirmation from a conceptual / design standpoint. That was my purpose and it is now clear. Thanks. Going a little bit further, in this context, I wonder if my understanding of the Protocol / Transport objects lifecycle is 100% clear. I think there is a one to one relationship in the sense that, simply put, on an incoming connection the underlying machinery instantiates a Protocol via the Factory's buildProtocol method, instantiates a Transport object and then calls Protocol.makeConnection to "attach" the transport to the protocol; when the connection is closed, the Transport calls connectionLost on the Protocol and, at some point, those two objects are no longer "usable" -- they've done their job and will be destroyed if no app level references are kept to them. If this is correct, in particular the fact that a given Transport instance is not reusable after disconnection (if it is, could you please point out such a case?), wouldn't it be valuable to log a warning when write after disconnect is called? (agreed, raising an Exception would be too much and break compatiblity). I find two reasons for that: on one hand, given that it would have made my debugging easier, it may help other developers pin point such cases; on the other, the fact that Twisted itself logs messages on several occasions, such as, for example, the default noisy Factory on doStart / doStop. Adding to that, the fact that, if I'm reading the code correctly, ITransport.write just throws away the data for TCP transports since it falls back to FileDescriptor.write that simply returns if "not self.connected or self._writeDisconnected" in src/twisted/internet/abstract.py. Thoughts? (or corrections, of course?) Again, thanks for your time and input. Regards, PS: I don't mean to start a huge discussion on the topic or question the current design. It is very nice as it is, thank you! I'm just looking for improving my understanding and, humbly, contributing back. -- exvito _______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
