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.

- Ilya

сб, 20 янв. 2018 г. в 9:10, Glyph <glyph@twistedmatrix.com>:


> On Jan 19, 2018, at 11:52 AM, Ilya Skriblovsky <ilyaskriblovsky@gmail.com> wrote:
>
> > Protocols and transports have a fairly defined lifecycle, and as L. Daniel Burr already pointed out, it would probably be appropriate to explicitly break these reference cycles in connectionLost.
>
> Explicitly breaking cycle in ProtocolWrapper.connectionLost by any of:
> • self.wrappedProtocol = None
> • self.wrappedProtocol.transport = None
> • self.wrappedProtocol = weakref.proxy(self.wrappedProtocol)
> • self.wrappedProtocol.transport = weakref.proxy(self)
>
> ... breaks some existing tests :(
>
> Seems like these tests do some post-run checks against protocol instances and their transports. Not sure whether it is relevant to real-life usage.
> Will investigate more...
>
> - Ilya

Do these tests fail if you only do it in TLSMemoryBIOProtocol instead of WrapperProtocol?

If so, this may be worth a compatibility exception.

-g

_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python