Fix concurrent force-close on asyncio socket streams - #1277
Conversation
|
you say you reproduced this on windows, does UDPSocket and ConnectedUDPSocket have the same problem there? |
|
I also checked the Windows UDP paths: UDPSocket and ConnectedUDPSocket do not have the same race because both aclose() implementations already wait for DatagramProtocol.closed_event, which connection_lost() sets after the transport is closed. Their focused IPv4 FD-release regressions pass across the available backends (10 tests). The Windows IPv6 variants are unavailable on this machine (WinError 5/10013), so CI remains the cross-platform authority. |
|
|
|
Thanks — agreed. I updated both asyncio |
|
we also need to make sure aclose() raises a CancelledError: stream = await connect_tcp(*server_addr)
exc = None
with CancelScope() as scope:
scope.cancel()
try:
await stream.aclose()
except anyio.get_cancelled_exc_class() as e:
exc = e
raise
assert exc is not Nonethe checkpoint doesn't need to be shielded to wait for the close event, there just needs to be a checkpoint - the sock.close() gets called from loop.call_soon which is called before the task is woken by cancellation |
…se-forcefully-1273 # Conflicts: # docs/versionhistory.rst
|
Implemented the cancellation test and shutdown simplification in b84eb22, then merged the latest master in b3e2de6 to resolve the CI merge conflict. The new asyncio-specific regression verifies that SocketStream.aclose() raises the backend cancellation exception from an already-cancelled scope, while the existing concurrent aclose_forcefully() regression still verifies both callers return only after the raw socket is closed. Local validation: 110 IPv4 TestTCPStream cases, 81 CapacityLimiter cases, and Ruff check/format passed. This Windows host cannot open IPv6 loopback sockets (WinError 5/10013), so CI remains authoritative for IPv6. |
NOTE Erasing or replacing the contents of this template will result in your pull
request being summarily closed without consideration!
Changes
Fixes #1273.
Ensure
SocketStream.aclose()always aborts the asyncio transport and takes acancellable checkpoint before returning. This keeps concurrent
aclose_forcefully()calls from reporting completion while the raw socket is still open without suppressing
cancellation of normal
aclose().Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/) which would fail without your patchdocs/), in case of behavior changes or newfeatures
docs/versionhistory.rst).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**as the version.If, say, your patch fixes issue #123, the entry should look like this:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.
Testing
TestTCPStreammatrix on Windows/Python 3.13: 110 passedCapacityLimiterregression matrix after merging latest master: 81 passedauthoritative