Repository navigation
gh-159049: Fix reference cycles after a failed TLS handshake in asyncio - #159140
Conversation
| await waiter | ||
| except: | ||
| transport.close() | ||
| # gh-159049 |
There was a problem hiding this comment.
the comment is not useful here
| super()._call_connection_lost(exc) | ||
| finally: | ||
| self._write_ready = None | ||
| # gh-159049: clear callback too |
| def server(sock): | ||
| try: | ||
| sock.start_tls(sslctx, server_side=True) | ||
| except OSError: |
There was a problem hiding this comment.
is this try-except really needed?
There was a problem hiding this comment.
yes, handshake always fails here, so without except server will crash the test
| support.gc_collect() | ||
| with self.tcp_server(server) as srv, support.disable_gc(): | ||
| self.loop.run_until_complete(conn(srv.addr)) | ||
| self.assertFalse([o for o in gc.get_objects() |
There was a problem hiding this comment.
using gc. get_objects is fragile, a better way to write this test is to have weakrefs for the transport and check that it is none at this point.
There was a problem hiding this comment.
I've addressed all the comments, could you please take another look?
kumaraditya303
added
topic-asyncio
needs backport to 3.14
|
I think there is a similar ref cycle present in |
I'll check it in a couple of hours. If yes, is that okay if I push the fix into this PR without new issue+pr? |
|
Yes, push directly to this PR. |
Yes, the cycle has been confirmed. I’ve pushed the changes |
|
Thanks @deadlovelll for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15. |
|
Sorry, @deadlovelll and @kumaraditya303, I could not cleanly backport this to Please backport manually with cherry_picker, see the devguide for more information. |
|
Sorry, @deadlovelll and @kumaraditya303, I could not cleanly backport this to Please backport manually with cherry_picker, see the devguide for more information. |
|
i'll work on the bp |
2 participants
Fix reference cycles after a failed TLS handshake in asyncio
For details see gh-159049