Skip to content
This repository has been archived by the owner on Sep 13, 2024. It is now read-only.

transports: Close socket when closing the transport #52

Merged

Conversation

chrysn
Copy link
Contributor

@chrysn chrysn commented Dec 13, 2021

Merely detaching leads to the socket still being open, and thus behaves
differently from Python's asyncio where .close() on the transport also
closes the socket.


When calling .close() on a TCP socket, unlike in upstream asyncio, this does not cause the TCP connection to be closed, but merely leaks the file descriptor (which is what .detach() on a socket is described to do in the Python documentation).

This is precisly a revert of f900d41 -- @dob3001 might have ideas why this is a bad idea, or check against the original use case. I couldn't do that because #47 contains no further explanations. Possibly, the file descriptor was still used in earlier versions of gbulb by something that'd close it, and now that other component was changed.

The problem was observed when running aiocoap with the gbulb loop; while all its test cases run through on several Python versions with asyncio, the TCP tests hang on gbulb. To observe what gbulb is doing it is easier to run, in a checked out aiocoap master branch, the command AIOCOAP_TESTS_LOOP=gbulb python3 -m tests.test_server and on a different terminal socat - tcp6:localhost:5683. Entering 0000<Enter> leads to the termination of the connection on asyncio, and just the error message (but no TCP connetion termination) on gbulb. It's probably possible to make this into a more minimal proof of different behavior, but so far I hope that this relatively laborious process won't be necessary here.

PR Checklist:

  • All new features have been tested
    (No new features were added; a tox run skipped a few interpreters but completed the tests on 3.9. towncrier, package and docs failed on dependencies but should not be relevant to this commit)
  • All new features have been documented
    (There are no new features)
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Merely detaching leads to the socket still being open, and thus behaves
differently from Python's asyncio where .close() on the transport also
closes the socket.
@freakboy3742 freakboy3742 merged commit 0eb502e into beeware:master Feb 20, 2022
@freakboy3742
Copy link
Member

Thanks for the fix. On the basis that this PR has a complete test regimen, I'll accept it as a reversion.

freakboy3742 added a commit that referenced this pull request Feb 20, 2022
transports: Close socket when closing the transport
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants