You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
default_eventloop crashes when file descriptors > 1024 #222
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).
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.
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.
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.
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.
Activity
neilcook commentedon Nov 17, 2016
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.
wtoorop commentedon Nov 18, 2016
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.
neilcook commentedon Nov 18, 2016
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
wtoorop commentedon Apr 12, 2017
The default event loop is now poll() base, so this shouldn't be an issue anymore.
Thanks for starting this initiative!