Skip to content

Commit

Permalink
🐛 fix async connection shutdown in HTTP/1.1 and HTTP/2 leaving a `asy…
Browse files Browse the repository at this point in the history
…ncio.TransportSocket` and `_SelectorSocketTransport` partially closed
  • Loading branch information
Ousret committed Oct 31, 2024
1 parent faaa80f commit 44b99d6
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
2.11.908 (2024-10-31)
=====================

- Fixed async connection shutdown in HTTP/1.1 and HTTP/2 leaving a ``asyncio.TransportSocket`` and ``_SelectorSocketTransport`` partially closed.

2.11.907 (2024-10-30)
=====================

Expand Down
3 changes: 3 additions & 0 deletions src/urllib3/backend/_async/hface.py
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,9 @@ async def close(self) -> None: # type: ignore[override]

try:
self.sock.close()
# this avoids having SelectorSocketTransport in "closing" state
# pending. Thus avoid a ResourceWarning.
await self.sock.wait_for_close()
except OSError:
pass

Expand Down
46 changes: 41 additions & 5 deletions src/urllib3/contrib/ssa/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,23 @@ def __init__(
def fileno(self) -> int:
return self._fileno if self._fileno is not None else self._sock.fileno()

async def wait_for_close(self) -> None:
if self._connect_called:
return

if self._writer is None:
return

is_ssl = self._writer.get_extra_info("ssl_object") is not None

if is_ssl:
# Give the connection a chance to write any data in the buffer,
# and then forcibly tear down the SSL connection.
await asyncio.sleep(0)
self._writer.transport.abort()

await self._writer.wait_closed()

def close(self) -> None:
if self._writer is not None:
self._writer.close()
Expand All @@ -83,15 +100,21 @@ def close(self) -> None:
# probably not just uvloop.
uvloop_edge_case_bug = False

# keep track of our clean exit procedure
shutdown_called = False
close_called = False

if hasattr(self._sock, "shutdown"):
try:
self._sock.shutdown(SHUT_RD)
shutdown_called = True
except TypeError:
uvloop_edge_case_bug = True
# uvloop don't support shutdown! and sometime does not support close()...
# see https://github.com/jawah/niquests/issues/166 for ctx.
try:
self._sock.close()
close_called = True
except TypeError:
# last chance of releasing properly the underlying fd!
try:
Expand All @@ -101,6 +124,7 @@ def close(self) -> None:
else:
try:
direct_sock.shutdown(SHUT_RD)
shutdown_called = True
except OSError:
warnings.warn(
(
Expand All @@ -113,13 +137,25 @@ def close(self) -> None:
)
finally:
direct_sock.detach()
elif hasattr(self._sock, "close"):
self._sock.close()
# we have to force call close() on our sock object in UDP ctx. (even after shutdown)

# we have to force call close() on our sock object (even after shutdown).
# or we'll get a resource warning for sure!
if self.type == socket.SOCK_DGRAM and hasattr(self._sock, "close"):
if isinstance(self._sock, socket.socket) and hasattr(self._sock, "close"):
if not uvloop_edge_case_bug:
self._sock.close()
try:
self._sock.close()
close_called = True
except OSError:
pass

if not close_called or not shutdown_called:
# this branch detect whether we have an asyncio.TransportSocket instead of socket.socket.
if hasattr(self._sock, "_sock"):
try:
self._sock._sock.detach()
except (AttributeError, OSError):
pass

except OSError:
pass

Expand Down

0 comments on commit 44b99d6

Please sign in to comment.