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

Cannot use high-numbered sockets in 2.4.3 #43402

Closed
mlrsmith mannequin opened this issue May 24, 2006 · 21 comments
Closed

Cannot use high-numbered sockets in 2.4.3 #43402

mlrsmith mannequin opened this issue May 24, 2006 · 21 comments
Labels
extension-modules C modules in the Modules dir

Comments

@mlrsmith
Copy link
Mannequin

mlrsmith mannequin commented May 24, 2006

BPO 1494314
Nosy @tim-one, @loewis, @akuchling, @gpshead
Files
  • socket.patch: Proposed simple patch
  • poll-in-socketmodule.diff: Patch to use poll() in preference to select()
  • high-socket.diff: use poll in socket and ssl modules
  • high-socket.diff: v2 of using poll
  • 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 2006-10-11.02:40:04.000>
    created_at = <Date 2006-05-24.13:51:05.000>
    labels = ['extension-modules']
    title = 'Cannot use high-numbered sockets in 2.4.3'
    updated_at = <Date 2006-10-11.02:40:04.000>
    user = 'https://bugs.python.org/mlrsmith'

    bugs.python.org fields:

    activity = <Date 2006-10-11.02:40:04.000>
    actor = 'nnorwitz'
    assignee = 'anthonybaxter'
    closed = True
    closed_date = None
    closer = None
    components = ['Extension Modules']
    creation = <Date 2006-05-24.13:51:05.000>
    creator = 'mlrsmith'
    dependencies = []
    files = ['2011', '2012', '2013', '2014']
    hgrepos = []
    issue_num = 1494314
    keywords = []
    message_count = 21.0
    messages = ['28607', '28608', '28609', '28610', '28611', '28612', '28613', '28614', '28615', '28616', '28617', '28618', '28619', '28620', '28621', '28622', '28623', '28624', '28625', '28626', '28627']
    nosy_count = 8.0
    nosy_names = ['tim.peters', 'loewis', 'akuchling', 'nnorwitz', 'anthonybaxter', 'gregory.p.smith', 'mlrsmith', 'gabrielwicke']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue1494314'
    versions = ['Python 2.4']

    @mlrsmith
    Copy link
    Mannequin Author

    mlrsmith mannequin commented May 24, 2006

    Python 2.4.3 introduced (in Modules/socketmodule.c) the
    IS_SELECTABLE() macro, to check whether calling
    select() on a given file descriptor is permissible.

    However, it executes this check even when it won't
    actually call select(). Specifically, select() is
    called ONLY when s->sock_timeout > 0 (in
    internal_select mostly, but also internal_connect).

    I have a twisted application which uses many FDs
    (several thousand), and upgrading python to 2.4.3 makes
    it hit this limit (at 1024 file descriptors),
    regardless of ulimit settings. Twisted itself always
    uses non-blocking I/O on sockets, so with older
    versions of python this worked fine.

    A simple solution relies on the fact that select is
    only ever actually called, and changes the
    IS_SELECTABLE macro as in the attached fairly trivial
    patch. This is sufficient to restore my application to
    its previous state (where it works fine).

    This doesn't solve the more general problem in
    socketmodule - that we simply can't do all operations
    properly with the current reliance on select(), though,
    and it seems like a bit of a hack... If I wrote up a
    patch to use poll() on systems where it's available,
    throughout socketmodule.c, in preference to select(),
    would this be accepted?

    Mike

    @mlrsmith mlrsmith mannequin closed this as completed May 24, 2006
    @mlrsmith mlrsmith mannequin assigned anthonybaxter May 24, 2006
    @mlrsmith mlrsmith mannequin added the extension-modules C modules in the Modules dir label May 24, 2006
    @mlrsmith mlrsmith mannequin closed this as completed May 24, 2006
    @mlrsmith mlrsmith mannequin assigned anthonybaxter May 24, 2006
    @mlrsmith mlrsmith mannequin added the extension-modules C modules in the Modules dir label May 24, 2006
    @gabrielwicke
    Copy link
    Mannequin

    gabrielwicke mannequin commented May 29, 2006

    Logged In: YES
    user_id=956757

    Try "ulimit -n 8096" (only permitted in a root shell) to set
    your max socket usage to something larger. Opening more than
    1024 sockets works for me in python 2.4.3 after changing the
    limit for the terminal in question and restarting the
    interpreter. Just ulimit -n will show you your current
    limit. Ulimits are enforced by the Linux kernel and can
    likely be configured system-wide in /etc/sysctl.cfg.

    @gabrielwicke
    Copy link
    Mannequin

    gabrielwicke mannequin commented May 29, 2006

    Logged In: YES
    user_id=956757

    Never mind- you already tried ulimit. It still works for me
    though, 2.4.3 from Ubuntu Dapper.

    @gabrielwicke
    Copy link
    Mannequin

    gabrielwicke mannequin commented May 29, 2006

    Logged In: YES
    user_id=956757

    Please disregard my comments completely- just opening more
    than 1024 files does not trigger this bug, but doing a
    socket.send() on the 1025th socket does.

    @mlrsmith
    Copy link
    Mannequin Author

    mlrsmith mannequin commented May 29, 2006

    Logged In: YES
    user_id=1488997

    Yes, I had my ulimit set appropriately.

    There's no problem with _opening_ many sockets (at least, I
    don't think there is) - the problem is with trying to do
    something (like call socket.recv()) with them.

    The code in socketmodule.c is pretty clear - and having
    upgraded to 2.4.3 with ubuntu dapper, I _am_ running into this.

    For now, we're having to keep all our production machines on
    2.4.2.

    @akuchling
    Copy link
    Member

    Logged In: YES
    user_id=11375

    I expect such a patch would be acceptable. The largest
    issue is probably whether poll() is available everywhere, or
    if we'd be stuck maintaining both select() and poll() based
    versions of internal_select(). Does Windows support poll()
    at all?

    @mlrsmith
    Copy link
    Mannequin Author

    mlrsmith mannequin commented May 31, 2006

    Logged In: YES
    user_id=1488997

    That, of course, is the problem - select() is available more
    or less universally, but poll() is not. However, it's not
    terribly difficult to write a poll() wrapper using select(),
    though doing so in general would have serious performance
    issues (the specific uses in socketmodule.c do not hit this
    problem), and retains the limitations of select.

    It's not clear that the complexity of doing this is
    worthwhile compared to just implementing the simple API
    needed internally to socketmodule.c using both APIs (i.e.
    just adding a poll() option).

    Regardless, it'd be nice if at least the basic fix I wrote
    up went in - it solves the immediate problem, at least.

    @tim-one
    Copy link
    Member

    tim-one commented May 31, 2006

    Logged In: YES
    user_id=31435

    akuchling: No, poll() is not part of the Windows socket API.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jun 4, 2006

    Logged In: YES
    user_id=21627

    A patch to replace internal_select with poll(2) where
    available is acceptable. The current version should be
    conditionally kept. That Windows doesn't have poll(2) is no
    problem: its select implementation supports arbitrarily
    large FDs (just like poll).

    Relaxing the IS_SELECTABLE usage to cases where select is
    actually going to be called is optional: there shouldn't be
    too many systems left without select where people still want
    to open many file descriptors.

    @gpshead
    Copy link
    Member

    gpshead commented Jun 8, 2006

    Logged In: YES
    user_id=413

    eew yuck. yes use poll at the very least.

    we should also consider using epoll (linux) and kevent (bsd)
    in the future as both of those scale better than O(n) unlike
    select and poll.

    @mlrsmith
    Copy link
    Mannequin Author

    mlrsmith mannequin commented Jun 9, 2006

    Logged In: YES
    user_id=1488997

    Ok, I'll attach a patch that uses poll when available
    (HAVE_POLL is already being set by the configure stuff
    appropriately).

    It replaces one of the two uses of select() (specifically,
    the internal_select() function) in socketmodule.c. The other
    is win32-specific, so replacing it with poll() wouldn't make
    sense.

    greg: epoll/kevent don't make sense for replacing the use of
    select/poll in this particular case - socketmodule.c always
    selects/polls precisely one file descriptor.

    I've tested this locally, and it fixes the problem
    (linux/x86). I don't have a windows system to test it on,
    but it shouldn't change behaviour in any way for windows.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jul 7, 2006

    Logged In: YES
    user_id=33168

    I've added a more complete patch (against 2.5, hopefully
    applies to 2.4 too). It cleans up some things and adds
    support for SSL sockets too. Can people please review/test
    this? I manually tested this with regular sockets and it
    seemed to work. All the tests pass, but this is somewhat
    tricky. I hate the duplicated code between socket and ssl
    modules, but added to it. It would be great to clean this
    up for 2.6.

    If you are forced to use select with a high socket, the
    exception on connect is operation in progress rather than
    can't select on socket. I guess that's ok, it doesn't
    actually change the existing behaviour. That would have
    been more involved and I'm not sure it's worth it.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jul 7, 2006

    Logged In: YES
    user_id=21627

    The __APPLE__ stuff looks wrong in file 184131. You would
    have to use selectmodule.c:select_have_broken_poll at
    run-time to be sure you can use poll(2) on OS X (you can use
    it on 10.4, but can't on 10.3, IIRC).

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jul 9, 2006

    Logged In: YES
    user_id=33168

    I think you're right Martin. Looking at what it means to
    have a broken poll, I don't think we can in this instance.
    So I removed all refs to HAVE_BROKEN_POLL. What do you
    think of the new version?

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jul 9, 2006

    Logged In: YES
    user_id=33168

    I meant I don't think we *care* in this case (not can).

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jul 10, 2006

    Logged In: YES
    user_id=21627

    The patch is fine, please apply.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Jul 11, 2006

    Logged In: YES
    user_id=33168

    Anthony checked this in to 2.5 as 50567. I will backport at
    least part of it later.

    @anthonybaxter
    Copy link
    Mannequin

    anthonybaxter mannequin commented Jul 11, 2006

    Logged In: YES
    user_id=29957

    Applied. Patch will be in 2.5b2 (to be released shortly).

    @anthonybaxter
    Copy link
    Mannequin

    anthonybaxter mannequin commented Jul 11, 2006

    Logged In: YES
    user_id=29957

    Re-opening to remind myself to apply this to release24-maint
    for the eventual 2.4.4 release.

    @anthonybaxter
    Copy link
    Mannequin

    anthonybaxter mannequin commented Oct 11, 2006

    Logged In: YES
    user_id=29957

    This is applied and will be in 2.3.3c1

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Oct 11, 2006

    Logged In: YES
    user_id=33168

    I assume 2.3.3c1 means 2.4.4c1.

    @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
    Labels
    extension-modules C modules in the Modules dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants