Skip to content

Commit a355f60

Browse files
authored
gh-114914: Avoid keeping dead StreamWriter alive (#115661)
In some cases we might cause a StreamWriter to stay alive even when the application has dropped all references to it. This prevents us from doing automatical cleanup, and complaining that the StreamWriter wasn't properly closed. Fortunately, the extra reference was never actually used for anything so we can just drop it.
1 parent 686ec17 commit a355f60

File tree

3 files changed

+31
-10
lines changed

3 files changed

+31
-10
lines changed

Lib/asyncio/streams.py

+4-10
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ def __init__(self, stream_reader, client_connected_cb=None, loop=None):
201201
# is established.
202202
self._strong_reader = stream_reader
203203
self._reject_connection = False
204-
self._stream_writer = None
205204
self._task = None
206205
self._transport = None
207206
self._client_connected_cb = client_connected_cb
@@ -214,10 +213,8 @@ def _stream_reader(self):
214213
return None
215214
return self._stream_reader_wr()
216215

217-
def _replace_writer(self, writer):
216+
def _replace_transport(self, transport):
218217
loop = self._loop
219-
transport = writer.transport
220-
self._stream_writer = writer
221218
self._transport = transport
222219
self._over_ssl = transport.get_extra_info('sslcontext') is not None
223220

@@ -239,11 +236,8 @@ def connection_made(self, transport):
239236
reader.set_transport(transport)
240237
self._over_ssl = transport.get_extra_info('sslcontext') is not None
241238
if self._client_connected_cb is not None:
242-
self._stream_writer = StreamWriter(transport, self,
243-
reader,
244-
self._loop)
245-
res = self._client_connected_cb(reader,
246-
self._stream_writer)
239+
writer = StreamWriter(transport, self, reader, self._loop)
240+
res = self._client_connected_cb(reader, writer)
247241
if coroutines.iscoroutine(res):
248242
def callback(task):
249243
if task.cancelled():
@@ -405,7 +399,7 @@ async def start_tls(self, sslcontext, *,
405399
ssl_handshake_timeout=ssl_handshake_timeout,
406400
ssl_shutdown_timeout=ssl_shutdown_timeout)
407401
self._transport = new_transport
408-
protocol._replace_writer(self)
402+
protocol._replace_transport(new_transport)
409403

410404
def __del__(self, warnings=warnings):
411405
if not self._transport.is_closing():

Lib/test/test_asyncio/test_streams.py

+25
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,31 @@ async def inner(httpd):
11301130

11311131
self.assertEqual(messages, [])
11321132

1133+
def test_unclosed_server_resource_warnings(self):
1134+
async def inner(rd, wr):
1135+
fut.set_result(True)
1136+
with self.assertWarns(ResourceWarning) as cm:
1137+
del wr
1138+
gc.collect()
1139+
self.assertEqual(len(cm.warnings), 1)
1140+
self.assertTrue(str(cm.warnings[0].message).startswith("unclosed <StreamWriter"))
1141+
1142+
async def outer():
1143+
srv = await asyncio.start_server(inner, socket_helper.HOSTv4, 0)
1144+
async with srv:
1145+
addr = srv.sockets[0].getsockname()
1146+
with socket.create_connection(addr):
1147+
# Give the loop some time to notice the connection
1148+
await fut
1149+
1150+
messages = []
1151+
self.loop.set_exception_handler(lambda loop, ctx: messages.append(ctx))
1152+
1153+
fut = self.loop.create_future()
1154+
self.loop.run_until_complete(outer())
1155+
1156+
self.assertEqual(messages, [])
1157+
11331158
def _basetest_unhandled_exceptions(self, handle_echo):
11341159
port = socket_helper.find_unused_port()
11351160

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix an issue where an abandoned :class:`StreamWriter` would not be garbage
2+
collected.

0 commit comments

Comments
 (0)