[Twisted-Python] Circular references in TLSMemoryBIOProtocol

[Ilya Skriblovsky]

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 at twistedmatrix.com>:

>
>
> > On Jan 19, 2018, at 11:52 AM, Ilya Skriblovsky <
> ilyaskriblovsky at 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 at twistedmatrix.com
> https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20180120/c0bb261b/attachment-0002.html>