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

release GIL around getaddrinfo() #37130

Closed
jhylton mannequin opened this issue Sep 3, 2002 · 7 comments
Closed

release GIL around getaddrinfo() #37130

jhylton mannequin opened this issue Sep 3, 2002 · 7 comments
Labels
extension-modules C modules in the Modules dir

Comments

@jhylton
Copy link
Mannequin

jhylton mannequin commented Sep 3, 2002

BPO 604210
Nosy @loewis
Files
  • getaddrinfo.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 2003-05-09.07:57:04.000>
    created_at = <Date 2002-09-03.21:48:41.000>
    labels = ['extension-modules']
    title = 'release GIL around getaddrinfo()'
    updated_at = <Date 2003-05-09.07:57:04.000>
    user = 'https://bugs.python.org/jhylton'

    bugs.python.org fields:

    activity = <Date 2003-05-09.07:57:04.000>
    actor = 'jvr'
    assignee = 'jhylton'
    closed = True
    closed_date = None
    closer = None
    components = ['Extension Modules']
    creation = <Date 2002-09-03.21:48:41.000>
    creator = 'jhylton'
    dependencies = []
    files = ['4554']
    hgrepos = []
    issue_num = 604210
    keywords = ['patch']
    message_count = 7.0
    messages = ['41096', '41097', '41098', '41099', '41100', '41101', '41102']
    nosy_count = 4.0
    nosy_names = ['loewis', 'jhylton', 'nnorwitz', 'jvr']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue604210'
    versions = []

    @jhylton
    Copy link
    Mannequin Author

    jhylton mannequin commented Sep 3, 2002

    If getaddrinfo() is thread-safe, then we should release
    the interpreter lock before calling it. There is every
    reason to believe that this operation will be slow.

    @jhylton jhylton mannequin closed this as completed Sep 3, 2002
    @jhylton jhylton mannequin self-assigned this Sep 3, 2002
    @jhylton jhylton mannequin added the extension-modules C modules in the Modules dir label Sep 3, 2002
    @jhylton jhylton mannequin closed this as completed Sep 3, 2002
    @jhylton jhylton mannequin self-assigned this Sep 3, 2002
    @jhylton jhylton mannequin added the extension-modules C modules in the Modules dir label Sep 3, 2002
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Sep 4, 2002

    Logged In: YES
    user_id=21627

    How do you know getaddrinfo is thread-safe? In particular,
    when using the getaddrinfo emulation, it is not.

    @jhylton
    Copy link
    Mannequin Author

    jhylton mannequin commented Sep 4, 2002

    Logged In: YES
    user_id=31392

    The getaddrinfo() on Linux says it thread-safe. It's a
    name-fangled call. Is it part of some standard? It would
    certainly be possible to make the fallback implementation
    thread-safe. Or, if we don't believe it is thread-safe, we
    could add a separate lock to protect it. The socket module
    of 2.1 vintage had a separate lock for this purpose. And
    the 2.1 vintage Python performs much better when a
    multi-threaded app does DNS lookups.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Sep 4, 2002

    Logged In: YES
    user_id=33168

    I looked on OpenBSD (I think, may have been NetBSD) and
    FreeBSD. The man pages were the same, both old (circa
    1995), but they said their implementation was not
    thread-safe. I did a man on Dec UNIX and it said it
    conformed to POSIX.1g Draft 6.6, but didn't indicate if it
    was thread safe. Solaris 8 doesn't say anything in the man
    page.

    Here's a NetBSD 1.6 man page dated 2002/05/14 and it's still
    not thread-safe:
    http://216.239.37.100/search?q=cache:4bTvQQqcwq4C:www.tac.eu.org/cgi-bin/man-cgi%3Fgetaddrinfo%2B3+getaddrinfo+thread+safe&hl=en&ie=UTF-8

    This:
    http://216.239.33.100/search?q=cache:q5egqJ5_mv4C:mail.gnu.org/pipermail/guile-devel/2001-October/004039.html+getaddrinfo+thread+safe&hl=en&ie=UTF-8
    says "getaddrinfo and getnameinfo are the recommended APIs.
    They avoid hidden static data; they're supposed to be
    thread-safe; they handle multiple address families."

    It seems safest to do what Jeremy proposes and add a lock.
    There can always be a define if getaddrinfo is thread safe
    or not. Maybe we can even determine with autoconf.

    Note: I get much better performance with this patch under
    linux.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Sep 5, 2002

    Logged In: YES
    user_id=21627

    RFC 2553 specifies that getaddrinfo, getipnodebyname, and
    all the other new netdb functions must be thread-safe -
    hence the BSD man pages document it as a bug that they are not.

    So adding an additional lock, in general, might be overkill.
    I'd suggest a strategy that assumes, in general, that
    getaddrinfo is thread-safe. If it isn't (by hard-coded
    knowledge), I'd propose just to continue using the
    interpreter lock - those systems deserve to lose.

    As for the fallback implementation - feel free to do
    anything about it that you can find the time for. Recording
    it as thread-unsafe might be fine, adding a lock is another
    option, using the thread-safe API inside the fallback (where
    available) is a third option.

    I would not care too much, except that this is used on
    Windows. Notice that it won't be used on Windows if Python
    is compiled with VC.NET, since that provides a "native"
    getaddrinfo (and Python knows that it does). So if Python
    2.3 is going to be compiled with VC.NET, I would not worry
    about the fallback implementation at all.

    @jvr
    Copy link
    Mannequin

    jvr mannequin commented May 3, 2003

    Logged In: YES
    user_id=92689

    FWIW, I opened a bug regarding this very same issue (I didn't know if this
    item): python.org/sf/731644. That item now also contains a new patch that
    takes care of locking.

    @jvr
    Copy link
    Mannequin

    jvr mannequin commented May 9, 2003

    Logged In: YES
    user_id=92689

    The last patch from python.org/sf/731644 was accepted by MvL, and
    consequently checked in as socketmodule.c rev. 1.265.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir
    Projects
    None yet
    Development

    No branches or pull requests

    0 participants