Re: [Twisted-Python] SMTPConnectError as a side effect on connection lost

On Nov 26, 2017, at 5:10 AM, Adi Roiban wrote:

On 26 November 2017 at 05:36, Glyph wrote:

...

...

On Nov 25, 2017, at 8:52 PM, Adi Roiban wrote:

Do you see anything suspicious with this code?

Just that it looks like it leaves a Deferred in an error state. You said the tests weren’t “written using” trial, but are you running them with trial, or with any runner that will consider a logged failure to be a failure? If so, the failure won’t be logged until the garbage collector runs, which might be in a later test.

Many thanks for looking into that.

I dag deeper and I found the error.

Oops, looks like I was totally off base :-).

The root cause is twisted.internet.posixbase._DisconnectSelectableMixin._disconnectSelectable

It starts as:

def _disconnectSelectable(self, selectable, why, isRead, faildict={ error.ConnectionDone: failure.Failure(error.ConnectionDone()), error.ConnectionLost: failure.Failure(error.ConnectionLost()) }): """ Utility function for disconnecting a selectable.

Supports half-close notification, isRead should be boolean indicating whether error resulted from doRead(). """ self.removeReader(selectable) f = faildict.get(why.__class__)

Then `f` is returned but in STMP we have:

f.value = smtp.SMTPConnectError( -1, "Unable to connect to server.")

This replaces the value for `faildict` via f. Any future call will return that replaced error.

Yeah, this is definitely wrong. I don't think it occurred to anyone that someone might set the `value` attribute of an existing Failure. More on the ticket.

I have created a ticket for this

https://twistedmatrix.com/trac/ticket/9339

I guess that we can continue the discussion on how to fix it as part of that ticket.

----------

Here is my answer to Glyph's question regarding unhandled failed deferred.

The tests are not inheriting from the Trial test case.

I can run them with trial, including with --force-gc and no leftover deferred is detected. The SMTP test is [OK].

If I reverse the order, so that the STMP tests is executed last, trial will execute all tests without any error.

Yeah, your diagnosis is correct, mine was wrong. -glyph