Re: [Twisted-Python] ITransport.write after IProtocol.connectionLost -- no failure/exception?

[ex vito]

On 2016-12-17, at 23:18, Glyph Lefkowitz  wrote:

> On Dec 17, 2016, at 6:11 AM, exvito here  wrote:

[...]

>> 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.
> 
> They're not "destroyed", exactly.
> 
> The way to think about this is that the reactor, while running, is always on 
> the stack.  The stack is the root of what's currently 'active' in the garbage 
> collector.  The reactor then has references to various things - in this case, 
> TCP transports and timed DelayedCall objects.  The transport has a reference 
> to the protocol, which is what keeps both of them "usable".  The DelayedCall 
> has a reference to your function, and your function to anything it needs to 
> do its work.
> 
> Things can be removed from and added back to the reactor at any time - for 
> example, a transport may be removed without being closed by calling 
> .stopReading() and .stopWriting() on it.  If it gets garbage collected, its 
> socket will be closed, but because the reactor didn't close it, you won't get 
> a normal connectionLost notification.

Sure, I had that clear in my understanding, thanks for the additional details 
on the stopReading/stopWriting -- I'm not familiar with low lever reactor 
details and, maybe, who knows, one day I'll put myself to the interesting 
exercise/challenge of playing around writing a reactor. Just to improve my 
understanding of these details.


> So rather than thinking of a Transport as having a precisely-defined 
> lifecycle - although it does have certain states it goes through; 
> connecting/connected/disconnecting/disconnected - the best way to think about 
> it is as a graph of objects that the reactor is doing stuff to.

The bottom line (in my mind) is: is it correct or not to assume that, by 
design, once a Transport instance has been disconnected that same instance will 
never be connected again, regardless of when exactly that object is destroyed 
or garbage collected?


>> 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.
> 
> If you start logging a warning when write is called on a closed transport, 
> then that means any code that wants to be warnings-clean needs to check to 
> see if the transport is writable before writing to it.  This adds a lot of 
> complexity to that sort of message-formatting code, and potentially makes it 
> less efficient. After all, if

Re: [Twisted-Python] ITransport.write after IProtocol.connectionLost -- no failure/exception?

[Glyph Lefkowitz]

> On Dec 19, 2016, at 5:13 AM, ex vito  wrote:
> 
> On 2016-12-17, at 23:18, Glyph Lefkowitz  > wrote:
> 
>> On Dec 17, 2016, at 6:11 AM, exvito here > > wrote:
> 
> [...]
> 
>>> 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.
>> 
>> They're not "destroyed", exactly.
>> 
>> The way to think about this is that the reactor, while running, is always on 
>> the stack.  The stack is the root of what's currently 'active' in the 
>> garbage collector.  The reactor then has references to various things - in 
>> this case, TCP transports and timed DelayedCall objects.  The transport has 
>> a reference to the protocol, which is what keeps both of them "usable".  The 
>> DelayedCall has a reference to your function, and your function to anything 
>> it needs to do its work.
>> 
>> Things can be removed from and added back to the reactor at any time - for 
>> example, a transport may be removed without being closed by calling 
>> .stopReading() and .stopWriting() on it.  If it gets garbage collected, its 
>> socket will be closed, but because the reactor didn't close it, you won't 
>> get a normal connectionLost notification.
> 
> Sure, I had that clear in my understanding, thanks for the additional details 
> on the stopReading/stopWriting -- I'm not familiar with low lever reactor 
> details and, maybe, who knows, one day I'll put myself to the interesting 
> exercise/challenge of playing around writing a reactor. Just to improve my 
> understanding of these details.
> 
> 
>> So rather than thinking of a Transport as having a precisely-defined 
>> lifecycle - although it does have certain states it goes through; 
>> connecting/connected/disconnecting/disconnected - the best way to think 
>> about it is as a graph of objects that the reactor is doing stuff to.
> 
> The bottom line (in my mind) is: is it correct or not to assume that, by 
> design, once a Transport instance has been disconnected that same instance 
> will never be connected again, regardless of when exactly that object is 
> destroyed or garbage collected?

Yes.

>>> 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.
>> 
>> If you start logging a warning when write is called on a closed transport, 
>> then that means any code that wants