Skip to content

default_eventloop crashes when file descriptors > 1024 #222

Closed

Description

@neilcook

The whole design of the default_eventloop code assumes that fds will never be >1024. This is because it uses select, which doesn't handle more than 1024 simultaneous fds, nor does it handle fds whose value is >1024. However it's worse than that because the code uses the fd as an index into several arrays whose size is statically allocated to be FD_SETSIZE (fd_events, fd_timeout_times etc.)

This code needs to be rewritten to use poll() instead of select, and some suitable data structure that doesn't use the fd as the index (because that is not scalable). In practice, the number of outstanding fds associated with a context is likely to be quite small, even if the value of individual fds could be very high (and certainly higher than 1024).

Activity

  1. neilcook commented on Nov 17, 2016

    @neilcook
    ContributorAuthor

    Although I should note that the crash I reliably get is actually in:

    #5 0x00007fc857ca2acd in _getdns_call_user_callback (dns_req=0x7fc7fc2644a0,
    response=0x7fc8243c73f0) at ./general.c:74
    74 _getdns_dns_req_free(dns_req);

    Stacktrace is:
    #0 0x00007fc856cae5f7 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
    #1 0x00007fc856cafce8 in __GI_abort () at abort.c:90
    #2 0x00007fc856cee327 in __libc_message (do_abort=do_abort@entry=2,
    fmt=fmt@entry=0x7fc856df8488 "*** Error in `%s': %s: 0x%s ***\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:196
    #3 0x00007fc856cf6053 in malloc_printerr (ar_ptr=0x7fc857034760 <main_arena>,
    ptr=, str=0x7fc856df8568 "double free or corruption (out)",
    action=3) at malloc.c:5022
    #4 _int_free (av=0x7fc857034760 <main_arena>, p=, have_lock=0)
    at malloc.c:3842
    #5 0x00007fc857ca2acd in _getdns_call_user_callback (dns_req=0x7fc7fc2644a0,
    response=0x7fc8243c73f0) at ./general.c:74
    #6 0x00007fc857cb85d3 in default_read_cb (fd=1021, event=,
    event=) at extension/default_eventloop.c:160
    #7 default_eventloop_run_once (loop=loop@entry=0x7fc824057678,
    blocking=blocking@entry=1) at extension/default_eventloop.c:235
    #8 0x00007fc857cb867d in default_eventloop_run (loop=0x7fc824057678)
    at extension/default_eventloop.c:267
    ... my code ...

    FYI, the trigger for this is simply to make over 1000 connections to my test program, thus ensuring that fds will be allocated which have a value of higher than 1024. I'm not entirely sure how this relates to the double free however.

  2. wtoorop commented on Nov 18, 2016

    @wtoorop
    Contributor

    Acknowledged!

    Note that there is a --with-fd-setsize=size option with configure that can increase this number a bit. Also note that not all systems have poll (windows), so we'll have to make a test in configure.ac for it. Ideally we'd have a default "poll" eventloop when poll is available, and the select eventloop when it is not I suppose.

  3. neilcook commented on Nov 18, 2016

    @neilcook
    ContributorAuthor

    Yeah, the problem with build-time options is that this is entirely dependent on who builds the package, so I wanted something that worked based on runtime information. Now that it’s dynamic, we could also add a new API call to explicitly configure the max fds, to make it explicit.

    Also, Windows does have poll (WSAPoll) - https://msdn.microsoft.com/en-us/library/windows/desktop/ms741669(v=vs.85).aspx https://msdn.microsoft.com/en-us/library/windows/desktop/ms741669(v=vs.85).aspx

    It’s been there since Vista apparently,

    Neil

    On 18 Nov 2016, at 00:48, wtoorop notifications@github.com wrote:

    Acknowledged!

    Note that there is a --with-fd-setsize=size option with configure that can increase this number a bit. Also note that not all systems have poll (windows), so we'll have to make a test in configure.ac for it. Ideally we'd have a default "poll" eventloop when poll is available, and the select eventloop when it is not I suppose.

    —
    You are receiving this because you authored the thread.
    Reply to this email directly, view it on GitHub #222 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AOWv2MWtaEQoRZaSxuN9KE7fplWy-4bcks5q_PX6gaJpZM4K1Z_6.

  4. wtoorop commented on Apr 12, 2017

    @wtoorop
    Contributor

    The default event loop is now poll() base, so this shouldn't be an issue anymore.
    Thanks for starting this initiative!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions