[Twisted-Python] Circular references in TLSMemoryBIOProtocol

[Ilya Skriblovsky]

I've created the pull request with breaking cycles in connectionLost,
please consider: https://github.com/twisted/twisted/pull/955

This change seems to fit well to the reasoning of Compatibility Exception
process. Should I create new thread in the mailing list with "INCOMPATIBLE
CHANGE" in a subject?

вс, 21 янв. 2018 г. в 12:51, Glyph <glyph at twistedmatrix.com>:

> On Jan 20, 2018, at 9:32 AM, Ilya Skriblovsky <ilyaskriblovsky at gmail.com>
> wrote:
>
>
> Yes, doing it only for TLSMemoryBIOProtocol fails test too :(
>
> SSL-related seem to be touching both ends of this reference cycle after
> connectionLost:
>
> 1. twisted/test/test_sslverify.py:2102
> self.assertEqual(sProto.wrappedProtocol.data, b'')
> This one touches `wrappedProtocol`
>
> 2. twisted/test/proto_helpers.py:924 (waitUntilAllDisconnected, used by
> twisted.web.test.test_webclient.WebClientSSLTests, for example)
> if not True in [x.transport is not None and x.transport.connected for x in
> protocols]:
> and this one touches `transport` field
>
> There are other examples as well.
>
> Sure, these test failures can probably be fixed by changing tests
> themselves, for example by making them to hold their own references to both
> wrapping and wrapped protocols. But I'm not sure this wouldn't break any
> user's code too... For my app it was easily fixed by breaking cycle in my
> protocol's connectionLost. But I'm not experienced enough in Twisted
> internals to be sure doing it inside TLSMemoryBIOProtocol wouldn't break
> any real-world usage scenarios.
>
>
> I think that this is worth trying, at least.  If you could write a PR that
> fixes the tests, you might want to try following the exception process
> documented in
> https://twistedmatrix.com/documents/current/core/development/policy/compatibility-policy.html#procedure-for-exceptions-to-this-policy and
> see if anyone has any code that might break.
>
> I'm pretty sure that the direction to break the cycle in is to break the
> reference to .wrappedProtocol, and not to mess with
> .wrappedProtocol.transport (which is not really something that should be
> touched from the outside of the wrapped protocol).
>
> -glyph
>
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20180127/d2e018d4/attachment-0002.html>