Skip to content
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

Jump to bottom

Document how to close the TCPServer listening socket #67443

Closed
vadmium opened this issue Jan 17, 2015 · 8 comments
Closed

Document how to close the TCPServer listening socket #67443

vadmium opened this issue Jan 17, 2015 · 8 comments
Labels
docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@vadmium
Copy link
Member

vadmium commented Jan 17, 2015

BPO 23254
Nosy @vstinner, @rbtcollins, @bitdancer, @berkerpeksag, @vadmium, @demianbrecht
Files
  • server_close.patch
  • server_close.v2.patch
    		•

    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:

    assignee = None
closed_at = <Date 2015-07-29.00:57:29.442>
created_at = <Date 2015-01-17.04:32:54.957>
labels = ['type-feature', 'docs']
title = 'Document how to close the TCPServer listening socket'
updated_at = <Date 2015-07-29.00:57:29.430>
user = 'https://github.com/vadmium'

    bugs.python.org fields:

    activity = <Date 2015-07-29.00:57:29.430>
actor = 'rbcollins'
assignee = 'docs@python'
closed = True
closed_date = <Date 2015-07-29.00:57:29.442>
closer = 'rbcollins'
components = ['Documentation']
creation = <Date 2015-01-17.04:32:54.957>
creator = 'martin.panter'
dependencies = []
files = ['37940', '38271']
hgrepos = []
issue_num = 23254
keywords = ['patch']
message_count = 8.0
messages = ['234161', '235105', '236639', '236859', '236860', '236866', '247550', '247551']
nosy_count = 8.0
nosy_names = ['vstinner', 'rbcollins', 'r.david.murray', 'docs@python', 'python-dev', 'berker.peksag', 'martin.panter', 'demian.brecht']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue23254'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']
    @vadmium
    Copy link
    Member Author

    vadmium commented Jan 17, 2015

    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.

    @vadmium vadmium added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Jan 17, 2015
    @vadmium
    Copy link
    Member Author

    vadmium commented Jan 31, 2015

    Here is a simple patch to add server_close() to the documentation, and a simple test to ensure it closes the socket.

    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 26, 2015

    Left a couple small nitpicks in Rietveld. Otherwise LGTM.

    @vadmium
    Copy link
    Member Author

    vadmium commented Feb 27, 2015

    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).

    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 28, 2015

    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.

    @bitdancer
    Copy link
    Member

    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 ;)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 29, 2015

    New changeset 8afd995802a6 by Robert Collins in branch '2.7':
    Issue bpo-23254: Document how to close the TCPServer listening socket.
    https://hg.python.org/cpython/rev/8afd995802a6

    New changeset 1123de53195e by Robert Collins in branch '3.4':
    Issue bpo-23254: Document how to close the TCPServer listening socket.
    https://hg.python.org/cpython/rev/1123de53195e

    New changeset 5ee8a4efc06f by Robert Collins in branch '3.5':
    Issue bpo-23254: Document how to close the TCPServer listening socket.
    https://hg.python.org/cpython/rev/5ee8a4efc06f

    New changeset 256d5c7146cb by Robert Collins in branch 'default':
    Issue bpo-23254: Document how to close the TCPServer listening socket.
    https://hg.python.org/cpython/rev/256d5c7146cb

    @rbtcollins
    Copy link
    Member

    Applied to 2.7/3.4/3.5/3.6. Thanks!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @bitdancer @rbtcollins @vadmium