-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document how to close the TCPServer listening socket #67443
Comments
Running the example from the Asynchronous Mixins section of the “socketserver” documentation generates a ResourceWarning: $ ./python -btWall ThreadedTCPServer.py
Server loop running in thread: Thread-1
Received: Thread-2: Hello World 1
Received: Thread-3: Hello World 2
Received: Thread-4: Hello World 3
sys:1: ResourceWarning: unclosed <socket.socket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('127.0.0.1', 43804)> There is a server_close() method mentioned in the doc string of the BaseServer class, so I assume it is meant to be part of the API. But there is no mention of it in the reference documentation. I think server.server_close() should be documented, and called after server.shutdown() in the example. A further enhancement might be to turn BaseServer into a context manager, but I would be happy with using the existing server_close() method. |
Here is a simple patch to add server_close() to the documentation, and a simple test to ensure it closes the socket. |
Left a couple small nitpicks in Rietveld. Otherwise LGTM. |
Posting server_close.v2.patch, which tests fileno(), and uses a single space between the new sentences. Also added a bit to the how-to at the top (using doubly-spaced sentences to match the rest of the paragraph). |
LGTM. I'm not sure whether or not it's eligible for 3.4 though as it's a documentation and not a functional fix. |
In general documentation changes go in all maintained versions (ie: right now that would be 2.7, 3.4, and default/3.5). The only exception, really, would be if the change didn't apply to one or more of the versions because of code differences. (Note: I haven't reviewed the patch itself yet ;) |
New changeset 8afd995802a6 by Robert Collins in branch '2.7': New changeset 1123de53195e by Robert Collins in branch '3.4': New changeset 5ee8a4efc06f by Robert Collins in branch '3.5': New changeset 256d5c7146cb by Robert Collins in branch 'default': |
Applied to 2.7/3.4/3.5/3.6. Thanks! |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: