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.

Sign up for GitHub

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

Fix handling of sizehint=-1 in select.epoll() #76749

Closed
taleinat opened this issue Jan 16, 2018 · 9 comments
Closed

Fix handling of sizehint=-1 in select.epoll() #76749

taleinat opened this issue Jan 16, 2018 · 9 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@taleinat
Copy link
Contributor

BPO 32568
Nosy @vstinner, @taleinat, @giampaolo, @benjaminp, @serhiy-storchaka
PRs
  • bpo-32568: make select.epoll() and its docs consistent #7840
  • bpo-31938: Convert selectmodule.c to Argument Clinic #4265
  • [3.7] bpo-32568: make select.epoll() and its docs consistent (GH-7840) #8024
  • [3.6] bpo-32568: make select.epoll() and its docs consistent (GH-7840) #8025
    		•

    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 2018-06-30.13:16:38.061>
created_at = <Date 2018-01-16.10:48:25.552>
labels = ['extension-modules', '3.8', 'type-bug', '3.7']
title = 'Fix handling of sizehint=-1 in select.epoll()'
updated_at = <Date 2018-06-30.13:16:38.061>
user = 'https://github.com/taleinat'

    bugs.python.org fields:

    activity = <Date 2018-06-30.13:16:38.061>
actor = 'taleinat'
assignee = 'none'
closed = True
closed_date = <Date 2018-06-30.13:16:38.061>
closer = 'taleinat'
components = ['Extension Modules']
creation = <Date 2018-01-16.10:48:25.552>
creator = 'taleinat'
dependencies = []
files = []
hgrepos = []
issue_num = 32568
keywords = ['patch']
message_count = 9.0
messages = ['310072', '310073', '310075', '320236', '320238', '320252', '320774', '320776', '320778']
nosy_count = 5.0
nosy_names = ['vstinner', 'taleinat', 'giampaolo.rodola', 'benjamin.peterson', 'serhiy.storchaka']
pr_nums = ['7840', '4265', '8024', '8025']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32568'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']
    @taleinat
    Copy link
    Contributor Author

    In 2fb9ae9, select.epoll() was changed to reject sizehint=-1, despite that still being documented as the default value. Would it be possible to reverse this change, making -1 a special value signaling select.epoll() to use sizehint=FD_SETSIZE-1, as was before this change?

    Additionally, in the same commit the docs were changed to say that the sizehint parameter is deprecated and has no effect, but later changes have restored use of sizehint in certain cases without updating the documentation. This inconsistency should be fixed.

    For reference, this came up as part of bpo-31938.

    @taleinat
    Copy link
    Contributor Author

    Also see discussion in GitHub PR 4265 for bpo-31938.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error labels Jan 16, 2018
    @serhiy-storchaka
    Copy link
    Member

    Also sizehint=0 was rejected before 2fb9ae9. Now it is accepted.

    @vstinner
    Copy link
    Member

    *flags* is deprecated and completely ignored.

    What if tomorrow, Linux accepts a new flag, passing the flag would just be ignored? Maybe it's fine. Especially usually Python developers only use flags exposed in Python, and currently, only EPOLL_CLOEXEC is exposed and this flag is useless in the specific case of Python (Python always uses the flag).

    @vstinner
    Copy link
    Member

    Also sizehint=0 was rejected before 2fb9ae9. Now it is accepted.

    DESCRIPTION
    epoll_create() creates a new epoll(7) instance. Since Linux 2.6.8, the
    size argument is ignored, but must be greater than zero; see NOTES
    below.

    (...)

    NOTES
    In the initial epoll_create() implementation, the size argument
    informed the kernel of the number of file descriptors that the caller
    expected to add to the epoll instance. The kernel used this informa‐
    tion as a hint for the amount of space to initially allocate in inter‐
    nal data structures describing events. (If necessary, the kernel would
    allocate more space if the caller's usage exceeded the hint given in
    size.) Nowadays, this hint is no longer required (the kernel dynami‐
    cally sizes the required data structures without needing the hint), but
    size must still be greater than zero, in order to ensure backward com‐
    patibility when new epoll applications are run on older kernels.

    @taleinat
    Copy link
    Contributor Author

    > Also sizehint=0 was rejected before 2fb9ae9. Now it is accepted.

    Please note that by "Now it is accepted" I meant in the current codebase, before PR 7840. In this regard, that PR changes nothing.

    Given the documentation quoted by Victor, perhaps it would be wise to review that earlier change.

    @serhiy-storchaka serhiy-storchaka added the 3.8 only security fixes label Jun 25, 2018
    @taleinat
    Copy link
    Contributor Author

    New changeset 0cdf5f4 by Tal Einat in branch 'master':
    bpo-32568: make select.epoll() and its docs consistent (bpo-7840)
    0cdf5f4

    @taleinat
    Copy link
    Contributor Author

    New changeset fd1c092 by Tal Einat (Miss Islington (bot)) in branch '3.7':
    bpo-32568: make select.epoll() and its docs consistent (GH-7840) (GH-8024)
    fd1c092

    @taleinat
    Copy link
    Contributor Author

    New changeset db7ac30 by Tal Einat in branch '3.6':
    [3.6] bpo-32568: make select.epoll() and its docs consistent (GH-7840) (GH-8025)
    db7ac30

    @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
    3.7 (EOL) end of life 3.8 only security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @taleinat @serhiy-storchaka