From 145b76b894c6ed6406d7d3d47abedea5b29d942c Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sun, 7 Sep 2014 23:56:15 +0100 Subject: [PATCH] darwin: allocate enough space for select() hack `fd_set`s are way too small for `select()` hack when stream's fd is bigger than 1023. Make `fd_set`s a part of `uv__stream_select_t` structure. fix #1461 --- Makefile.am | 1 + src/unix/stream.c | 108 ++++++++++++++++++++++++++--------------- test/test-list.h | 2 + test/test-osx-select.c | 48 ++++++++++++++++++ uv.gyp | 6 ++- 5 files changed, 126 insertions(+), 39 deletions(-) diff --git a/Makefile.am b/Makefile.am index e3e33c80..c0ae6c4b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -259,6 +259,7 @@ endif if DARWIN include_HEADERS += include/uv-darwin.h libuv_la_CFLAGS += -D_DARWIN_USE_64_BIT_INODE=1 +libuv_la_CFLAGS += -D_DARWIN_UNLIMITED_SELECT=1 libuv_la_SOURCES += src/unix/darwin.c \ src/unix/darwin-proctitle.c \ src/unix/fsevents.c \ diff --git a/src/unix/stream.c b/src/unix/stream.c index 07c82a24..9c7d28cb 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -53,6 +53,10 @@ struct uv__stream_select_s { int fake_fd; int int_fd; int fd; + fd_set* sread; + size_t sread_sz; + fd_set* swrite; + size_t swrite_sz; }; #endif /* defined(__APPLE__) */ @@ -127,8 +131,6 @@ static void uv__stream_osx_select(void* arg) { uv_stream_t* stream; uv__stream_select_t* s; char buf[1024]; - fd_set sread; - fd_set swrite; int events; int fd; int r; @@ -149,17 +151,17 @@ static void uv__stream_osx_select(void* arg) { break; /* Watch fd using select(2) */ - FD_ZERO(&sread); - FD_ZERO(&swrite); + memset(s->sread, 0, s->sread_sz); + memset(s->swrite, 0, s->swrite_sz); if (uv__io_active(&stream->io_watcher, UV__POLLIN)) - FD_SET(fd, &sread); + FD_SET(fd, s->sread); if (uv__io_active(&stream->io_watcher, UV__POLLOUT)) - FD_SET(fd, &swrite); - FD_SET(s->int_fd, &sread); + FD_SET(fd, s->swrite); + FD_SET(s->int_fd, s->sread); /* Wait indefinitely for fd events */ - r = select(max_fd + 1, &sread, &swrite, NULL, NULL); + r = select(max_fd + 1, s->sread, s->swrite, NULL, NULL); if (r == -1) { if (errno == EINTR) continue; @@ -173,7 +175,7 @@ static void uv__stream_osx_select(void* arg) { continue; /* Empty socketpair's buffer in case of interruption */ - if (FD_ISSET(s->int_fd, &sread)) + if (FD_ISSET(s->int_fd, s->sread)) while (1) { r = read(s->int_fd, buf, sizeof(buf)); @@ -194,12 +196,12 @@ static void uv__stream_osx_select(void* arg) { /* Handle events */ events = 0; - if (FD_ISSET(fd, &sread)) + if (FD_ISSET(fd, s->sread)) events |= UV__POLLIN; - if (FD_ISSET(fd, &swrite)) + if (FD_ISSET(fd, s->swrite)) events |= UV__POLLOUT; - assert(events != 0 || FD_ISSET(s->int_fd, &sread)); + assert(events != 0 || FD_ISSET(s->int_fd, s->sread)); if (events != 0) { ACCESS_ONCE(int, s->events) = events; @@ -261,6 +263,9 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) { int ret; int kq; int old_fd; + int max_fd; + size_t sread_sz; + size_t swrite_sz; kq = kqueue(); if (kq == -1) { @@ -284,31 +289,48 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) { return 0; /* At this point we definitely know that this fd won't work with kqueue */ - s = malloc(sizeof(*s)); - if (s == NULL) - return -ENOMEM; + + /* + * Create fds for io watcher and to interrupt the select() loop. + * NOTE: do it ahead of malloc below to allocate enough space for fd_sets + */ + if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds)) + return -errno; + + max_fd = *fd; + if (fds[1] > max_fd) + max_fd = fds[1]; + + sread_sz = (max_fd + NBBY) / NBBY; + swrite_sz = sread_sz; + + s = malloc(sizeof(*s) + sread_sz + swrite_sz); + if (s == NULL) { + err = -ENOMEM; + goto failed_malloc; + } s->events = 0; s->fd = *fd; + s->sread = (fd_set*) ((char*) s + sizeof(*s)); + s->sread_sz = sread_sz; + s->swrite = (fd_set*) ((char*) s->sread + sread_sz); + s->swrite_sz = swrite_sz; err = uv_async_init(stream->loop, &s->async, uv__stream_osx_select_cb); - if (err) { - free(s); - return err; - } + if (err) + goto failed_async_init; s->async.flags |= UV__HANDLE_INTERNAL; uv__handle_unref(&s->async); - if (uv_sem_init(&s->close_sem, 0)) - goto fatal1; + err = uv_sem_init(&s->close_sem, 0); + if (err != 0) + goto failed_close_sem_init; - if (uv_sem_init(&s->async_sem, 0)) - goto fatal2; - - /* Create fds for io watcher and to interrupt the select() loop. */ - if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds)) - goto fatal3; + err = uv_sem_init(&s->async_sem, 0); + if (err != 0) + goto failed_async_sem_init; s->fake_fd = fds[0]; s->int_fd = fds[1]; @@ -318,26 +340,36 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) { stream->select = s; *fd = s->fake_fd; - if (uv_thread_create(&s->thread, uv__stream_osx_select, stream)) - goto fatal4; + err = uv_thread_create(&s->thread, uv__stream_osx_select, stream); + if (err != 0) + goto failed_thread_create; return 0; -fatal4: +failed_thread_create: s->stream = NULL; stream->select = NULL; *fd = old_fd; - uv__close(s->fake_fd); - uv__close(s->int_fd); - s->fake_fd = -1; - s->int_fd = -1; -fatal3: + uv_sem_destroy(&s->async_sem); -fatal2: + +failed_async_sem_init: uv_sem_destroy(&s->close_sem); -fatal1: + +failed_close_sem_init: + uv__close(fds[0]); + uv__close(fds[1]); uv_close((uv_handle_t*) &s->async, uv__stream_osx_cb_close); - return -errno; + return err; + +failed_async_init: + free(s); + +failed_malloc: + uv__close(fds[0]); + uv__close(fds[1]); + + return err; } #endif /* defined(__APPLE__) */ diff --git a/test/test-list.h b/test/test-list.h index 4d1d5db2..31888b9e 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -286,6 +286,7 @@ TEST_DECLARE (closed_fd_events) #endif #ifdef __APPLE__ TEST_DECLARE (osx_select) +TEST_DECLARE (osx_select_many_fds) #endif HELPER_DECLARE (tcp4_echo_server) HELPER_DECLARE (tcp6_echo_server) @@ -574,6 +575,7 @@ TASK_LIST_START #ifdef __APPLE__ TEST_ENTRY (osx_select) + TEST_ENTRY (osx_select_many_fds) #endif TEST_ENTRY (fs_file_noent) diff --git a/test/test-osx-select.c b/test/test-osx-select.c index e5e1bf8b..68e5a841 100644 --- a/test/test-osx-select.c +++ b/test/test-osx-select.c @@ -79,4 +79,52 @@ TEST_IMPL(osx_select) { return 0; } + +TEST_IMPL(osx_select_many_fds) { + int r; + int fd; + size_t i; + size_t len; + const char* str; + struct sockaddr_in addr; + uv_tty_t tty; + uv_tcp_t tcps[1500]; + + r = uv_ip4_addr("127.0.0.1", 0, &addr); + ASSERT(r == 0); + + for (i = 0; i < ARRAY_SIZE(tcps); i++) { + r = uv_tcp_init(uv_default_loop(), &tcps[i]); + ASSERT(r == 0); + r = uv_tcp_bind(&tcps[i], (const struct sockaddr *) &addr, 0); + ASSERT(r == 0); + uv_unref((uv_handle_t*) &tcps[i]); + } + + fd = open("/dev/tty", O_RDONLY); + ASSERT(fd >= 0); + + r = uv_tty_init(uv_default_loop(), &tty, fd, 1); + ASSERT(r == 0); + + r = uv_read_start((uv_stream_t*) &tty, alloc_cb, read_cb); + ASSERT(r == 0); + + /* Emulate user-input */ + str = "got some input\n" + "with a couple of lines\n" + "feel pretty happy\n"; + for (i = 0, len = strlen(str); i < len; i++) { + r = ioctl(fd, TIOCSTI, str + i); + ASSERT(r == 0); + } + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); + + ASSERT(read_count == 3); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + #endif /* __APPLE__ */ diff --git a/uv.gyp b/uv.gyp index 0a459be5..23295dcc 100644 --- a/uv.gyp +++ b/uv.gyp @@ -46,7 +46,10 @@ ], }], ['OS == "mac"', { - 'defines': [ '_DARWIN_USE_64_BIT_INODE=1' ], + 'defines': [ + '_DARWIN_USE_64_BIT_INODE=1', + '_DARWIN_UNLIMITED_SELECT=1', + ], }], ['OS == "linux"', { 'defines': [ '_POSIX_C_SOURCE=200112' ], @@ -192,6 +195,7 @@ ], 'defines': [ '_DARWIN_USE_64_BIT_INODE=1', + '_DARWIN_UNLIMITED_SELECT=1', ] }], [ 'OS!="mac"', {