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 <gl...@twistedmatrix.com>: > > > > On Jan 19, 2018, at 11:52 AM, Ilya Skriblovsky < > ilyaskriblov...@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 >
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python