From e515d71592afe66ddecd6bf2b1409848811cf7ff Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 17 May 2013 14:42:14 +0200 Subject: [PATCH 01/23] darwin: make two uv__cf_*() functions static --- src/unix/darwin.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/unix/darwin.c b/src/unix/darwin.c index 54351395..d8e0c62f 100644 --- a/src/unix/darwin.c +++ b/src/unix/darwin.c @@ -38,8 +38,8 @@ #include /* sysconf */ /* Forward declarations */ -void uv__cf_loop_runner(void* arg); -void uv__cf_loop_cb(void* arg); +static void uv__cf_loop_runner(void* arg); +static void uv__cf_loop_cb(void* arg); typedef struct uv__cf_loop_signal_s uv__cf_loop_signal_t; struct uv__cf_loop_signal_s { @@ -102,7 +102,7 @@ void uv__platform_loop_delete(uv_loop_t* loop) { } -void uv__cf_loop_runner(void* arg) { +static void uv__cf_loop_runner(void* arg) { uv_loop_t* loop; loop = arg; @@ -124,7 +124,7 @@ void uv__cf_loop_runner(void* arg) { } -void uv__cf_loop_cb(void* arg) { +static void uv__cf_loop_cb(void* arg) { uv_loop_t* loop; ngx_queue_t* item; ngx_queue_t split_head; From a1cb52a3ebe13f8e26a48e194e595e95c677de30 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 20 May 2013 14:35:10 +0200 Subject: [PATCH 02/23] darwin: task_info() cannot fail And if it does: assert, don't return errno. It's a mach function, it doesn't set errno. --- src/unix/darwin.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/unix/darwin.c b/src/unix/darwin.c index d8e0c62f..77e662f4 100644 --- a/src/unix/darwin.c +++ b/src/unix/darwin.c @@ -257,19 +257,21 @@ void uv_loadavg(double avg[3]) { uv_err_t uv_resident_set_memory(size_t* rss) { - struct task_basic_info t_info; - mach_msg_type_number_t t_info_count = TASK_BASIC_INFO_COUNT; + mach_msg_type_number_t count; + task_basic_info_data_t info; + kern_return_t err; - int r = task_info(mach_task_self(), - TASK_BASIC_INFO, - (task_info_t)&t_info, - &t_info_count); - - if (r != KERN_SUCCESS) { - return uv__new_sys_error(errno); - } - - *rss = t_info.resident_size; + count = TASK_BASIC_INFO_COUNT; + err = task_info(mach_task_self(), + TASK_BASIC_INFO, + (task_info_t) &info, + &count); + (void) &err; + /* task_info(TASK_BASIC_INFO) cannot really fail. Anything other than + * KERN_SUCCESS implies a libuv bug. + */ + assert(err == KERN_SUCCESS); + *rss = info.resident_size; return uv_ok_; } From 739a5b25b5704d526a46a953da8b9b8db31770d4 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 20 May 2013 20:04:45 +0200 Subject: [PATCH 03/23] unix: add mapping for ENETDOWN --- src/unix/error.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/unix/error.c b/src/unix/error.c index 9e3e84ad..05ab4820 100644 --- a/src/unix/error.c +++ b/src/unix/error.c @@ -79,6 +79,7 @@ uv_err_code uv_translate_sys_error(int sys_errno) { case EMSGSIZE: return UV_EMSGSIZE; case ENAMETOOLONG: return UV_ENAMETOOLONG; case EINVAL: return UV_EINVAL; + case ENETDOWN: return UV_ENETDOWN; case ENETUNREACH: return UV_ENETUNREACH; case ECONNABORTED: return UV_ECONNABORTED; case ELOOP: return UV_ELOOP; From c53fe815442559fe58f362279bdc63f5483d6fdb Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 22 May 2013 16:41:52 +0200 Subject: [PATCH 04/23] unix: implicitly signal write errors to libuv user Fix an infinite loop in the example below when the stream encounters an EPIPE/ECONNRESET/etc. error: // keep writing until we start buffering while (stream->write_queue_size == 0) { uv_write_t* req = make_write_req(); uv_buf_t buf = uv_buf_init("PING", 4); uv_write(req, stream, &buf, 1, write_cb); } uv_write() does not return an error code on write errors, the error is only reported to the callback. Before this commit, uv_write() left stream->write_queue_size untouched on error, meaning the caller had no way to find out about that error until the next tick of the event loop - which in the example above leads to an infinite loop because that next tick is indefinitely postponed. This commit works around that at the cost of some added internal complexity. Fixes joyent/node#5516. --- src/unix/stream.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/unix/stream.c b/src/unix/stream.c index fda2f02f..824de9f4 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -60,6 +60,7 @@ static void uv__stream_connect(uv_stream_t*); static void uv__write(uv_stream_t* stream); static void uv__read(uv_stream_t* stream); static void uv__stream_io(uv_loop_t* loop, uv__io_t* w, unsigned int events); +static size_t uv__write_req_size(uv_write_t* req); /* Used by the accept() EMFILE party trick. */ @@ -399,6 +400,7 @@ void uv__stream_destroy(uv_stream_t* stream) { if (req->bufs != req->bufsml) free(req->bufs); + req->bufs = NULL; if (req->cb) { uv__set_artificial_error(req->handle->loop, UV_ECANCELED); @@ -413,6 +415,13 @@ void uv__stream_destroy(uv_stream_t* stream) { req = ngx_queue_data(q, uv_write_t, queue); uv__req_unregister(stream->loop, req); + if (req->bufs != NULL) { + stream->write_queue_size -= uv__write_req_size(req); + if (req->bufs != req->bufsml) + free(req->bufs); + req->bufs = NULL; + } + if (req->cb) { uv__set_sys_error(stream->loop, req->error); req->cb(req, req->error ? -1 : 0); @@ -652,6 +661,7 @@ static void uv__drain(uv_stream_t* stream) { static size_t uv__write_req_size(uv_write_t* req) { size_t size; + assert(req->bufs != NULL); size = uv__buf_count(req->bufs + req->write_index, req->bufcnt - req->write_index); assert(req->handle->write_queue_size >= size); @@ -665,10 +675,18 @@ static void uv__write_req_finish(uv_write_t* req) { /* Pop the req off tcp->write_queue. */ ngx_queue_remove(&req->queue); - if (req->bufs != req->bufsml) { - free(req->bufs); + + /* Only free when there was no error. On error, we touch up write_queue_size + * right before making the callback. The reason we don't do that right away + * is that a write_queue_size > 0 is our only way to signal to the user that + * he should stop writing - which he should if we got an error. Something to + * revisit in future revisions of the libuv API. + */ + if (req->error == 0) { + if (req->bufs != req->bufsml) + free(req->bufs); + req->bufs = NULL; } - req->bufs = NULL; /* Add it to the write_completed_queue where it will have its * callback called in the near future. @@ -778,7 +796,6 @@ start: if (errno != EAGAIN && errno != EWOULDBLOCK) { /* Error */ req->error = errno; - stream->write_queue_size -= uv__write_req_size(req); uv__write_req_finish(req); return; } else if (stream->flags & UV_STREAM_BLOCKING) { @@ -855,6 +872,13 @@ static void uv__write_callbacks(uv_stream_t* stream) { ngx_queue_remove(q); uv__req_unregister(stream->loop, req); + if (req->bufs != NULL) { + stream->write_queue_size -= uv__write_req_size(req); + if (req->bufs != req->bufsml) + free(req->bufs); + req->bufs = NULL; + } + /* NOTE: call callback AFTER freeing the request data. */ if (req->cb) { uv__set_sys_error(stream->loop, req->error); From c5d570ddba7b3e95fdade96758df0eb2d24cf42f Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 23 May 2013 14:44:45 +0200 Subject: [PATCH 05/23] unix: fix assert on signal pipe overflow An incorrect assert() statement was causing libuv to crash when writing to an internal signal pipe would result in EAGAIN/EWOULDBLOCK. This commit doesn't solve the underlying issue that the signal pipe can overflow. This should fix joyent/node#5538 --- src/unix/signal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unix/signal.c b/src/unix/signal.c index f7fd2e5e..22c7783b 100644 --- a/src/unix/signal.c +++ b/src/unix/signal.c @@ -160,7 +160,7 @@ static void uv__signal_handler(int signum) { } while (r == -1 && errno == EINTR); assert(r == sizeof msg || - (r == -1 && errno != EAGAIN && errno != EWOULDBLOCK)); + (r == -1 && (errno == EAGAIN || errno == EWOULDBLOCK))); if (r != -1) handle->caught_signals++; From 80f2f826bf90b84e659321c0b7fd8af419acb85e Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 23 May 2013 07:16:00 +0200 Subject: [PATCH 06/23] unix: fix stream refcounting buglet Fix a buglet where uv_read_stop() would mark the handle as stopped even when there are in-progress write requests. This bug is unlikely to have affected anyone, the only case where it has a user-visible effect is when: a) the handle has been stopped for reading but not writing, and b) it's the last active handle in the event loop's pollset If both conditions are met, it's possible for the event loop to terminate prematurely. --- src/unix/stream.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/unix/stream.c b/src/unix/stream.c index 824de9f4..378f21c6 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1307,9 +1307,10 @@ int uv_read2_start(uv_stream_t* stream, uv_alloc_cb alloc_cb, int uv_read_stop(uv_stream_t* stream) { - uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLIN); - uv__handle_stop(stream); stream->flags &= ~UV_STREAM_READING; + uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLIN); + if (!uv__io_active(&stream->io_watcher, UV__POLLOUT)) + uv__handle_stop(stream); #if defined(__APPLE__) /* Notify select() thread about state change */ From 41468050745bc135247f587eae1c38e958fd8377 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 23 May 2013 07:37:36 +0200 Subject: [PATCH 07/23] unix: turn off POLLOUT after stream connect Clear the POLLOUT flag after we're done connecting. Not doing so isn't really harmful but it may cause the event loop to wake up more often than it has to. --- src/unix/stream.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/unix/stream.c b/src/unix/stream.c index 378f21c6..ac74537e 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1160,6 +1160,7 @@ static void uv__stream_connect(uv_stream_t* stream) { stream->connect_req = NULL; uv__req_unregister(stream->loop, req); + uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLOUT); if (req->cb) { uv__set_sys_error(stream->loop, error); @@ -1307,6 +1308,16 @@ int uv_read2_start(uv_stream_t* stream, uv_alloc_cb alloc_cb, int uv_read_stop(uv_stream_t* stream) { + /* Sanity check. We're going to stop the handle unless it's primed for + * writing but that means there should be some kind of write action in + * progress. + */ + assert(!uv__io_active(&stream->io_watcher, UV__POLLOUT) || + !ngx_queue_empty(&stream->write_completed_queue) || + !ngx_queue_empty(&stream->write_queue) || + stream->shutdown_req != NULL || + stream->connect_req != NULL); + stream->flags &= ~UV_STREAM_READING; uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLIN); if (!uv__io_active(&stream->io_watcher, UV__POLLOUT)) From fe7b154476145ebc69ab70d3ca1d195116a00065 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 24 May 2013 21:23:09 +0200 Subject: [PATCH 08/23] Revert "unix: fix stream refcounting buglet" This change is making 45 out of 527 node.js tests fail on OS X with the following assertion: Assertion failed: (!uv__is_active(handle)), function uv__finish_close, file ../../deps/uv/src/unix/core.c, line 165. It's likely a manifestation of a bug elsewhere but, because there's a new node.js release going out tonight, I'm reverting it for now. This reverts commit 80f2f826bf90b84e659321c0b7fd8af419acb85e. Conflicts: src/unix/stream.c --- src/unix/stream.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/unix/stream.c b/src/unix/stream.c index ac74537e..aeefa2c4 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1318,10 +1318,9 @@ int uv_read_stop(uv_stream_t* stream) { stream->shutdown_req != NULL || stream->connect_req != NULL); - stream->flags &= ~UV_STREAM_READING; uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLIN); - if (!uv__io_active(&stream->io_watcher, UV__POLLOUT)) - uv__handle_stop(stream); + uv__handle_stop(stream); + stream->flags &= ~UV_STREAM_READING; #if defined(__APPLE__) /* Notify select() thread about state change */ From 0f39be12926fe2d8766a9f025797a473003e6504 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 24 May 2013 14:37:53 -0700 Subject: [PATCH 09/23] 2013.05.25, Version 0.10.8 (Stable) Changes since version 0.10.7: * windows: make uv_spawn not fail under job control (Bert Belder) * darwin: assume CFRunLoopStop() isn't thread-safe (Fedor Indutny) * win: fix UV_EALREADY incorrectly set (Bert Belder) * darwin: make two uv__cf_*() functions static (Ben Noordhuis) * darwin: task_info() cannot fail (Ben Noordhuis) * unix: add mapping for ENETDOWN (Ben Noordhuis) * unix: implicitly signal write errors to libuv user (Ben Noordhuis) * unix: fix assert on signal pipe overflow (Bert Belder) * unix: turn off POLLOUT after stream connect (Ben Noordhuis) --- ChangeLog | 23 +++++++++++++++++++++++ src/version.c | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index aa1f5d9a..356a5b10 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2013.05.25, Version 0.10.8 (Stable) + +Changes since version 0.10.7: + +* windows: make uv_spawn not fail under job control (Bert Belder) + +* darwin: assume CFRunLoopStop() isn't thread-safe (Fedor Indutny) + +* win: fix UV_EALREADY incorrectly set (Bert Belder) + +* darwin: make two uv__cf_*() functions static (Ben Noordhuis) + +* darwin: task_info() cannot fail (Ben Noordhuis) + +* unix: add mapping for ENETDOWN (Ben Noordhuis) + +* unix: implicitly signal write errors to libuv user (Ben Noordhuis) + +* unix: fix assert on signal pipe overflow (Bert Belder) + +* unix: turn off POLLOUT after stream connect (Ben Noordhuis) + + 2013.05.15, Version 0.10.7 (Stable), 028baaf0846b686a81e992cb2f2f5a9b8e841fcf Changes since version 0.10.6: diff --git a/src/version.c b/src/version.c index 9c523311..98765533 100644 --- a/src/version.c +++ b/src/version.c @@ -35,7 +35,7 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 #define UV_VERSION_PATCH 8 -#define UV_VERSION_IS_RELEASE 0 +#define UV_VERSION_IS_RELEASE 1 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From 7d5024e7e6564c36b99af39db075b0c9d75797f9 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 24 May 2013 14:37:56 -0700 Subject: [PATCH 10/23] Now working on v0.10.9 --- ChangeLog | 2 +- src/version.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 356a5b10..eb28efcb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,4 @@ -2013.05.25, Version 0.10.8 (Stable) +2013.05.25, Version 0.10.8 (Stable), 0f39be12926fe2d8766a9f025797a473003e6504 Changes since version 0.10.7: diff --git a/src/version.c b/src/version.c index 98765533..2d0100f3 100644 --- a/src/version.c +++ b/src/version.c @@ -34,8 +34,8 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 -#define UV_VERSION_PATCH 8 -#define UV_VERSION_IS_RELEASE 1 +#define UV_VERSION_PATCH 9 +#define UV_VERSION_IS_RELEASE 0 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From 636a13b8c46c52413e1da1795a952bfc738f3c55 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 23 May 2013 07:16:00 +0200 Subject: [PATCH 11/23] unix: fix stream refcounting buglet Fix a buglet where uv_read_stop() would mark the handle as stopped even when there are in-progress write requests. This bug is unlikely to have affected anyone, the only case where it has a user-visible effect is when: a) the handle has been stopped for reading but not writing, and b) it's the last active handle in the event loop's pollset If both conditions are met, it's possible for the event loop to terminate prematurely. This reapplies commit 80f2f82 which was temporarily reverted in fe7b154 because it was making a lot of node.js tests fail on OS X with the following assertion: Assertion failed: (!uv__is_active(handle)), function uv__finish_close, file ../../deps/uv/src/unix/core.c, line 165. Expecting that the handle is inactive when the state is UV_CLOSING turns out to be a bad assumption: it's possible that the handle is executing (for example) a shutdown request when uv__finish_close() is called. That's okay, uv__stream_destroy() takes care of that. The issue wasn't specific to OS X, it was just more visible on that platform. (Slow) debug builds on Linux exhibited the same behavior. --- src/unix/core.c | 7 ++++++- src/unix/stream.c | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/unix/core.c b/src/unix/core.c index 7ab9bd6c..b7f0da38 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -162,7 +162,12 @@ void uv__make_close_pending(uv_handle_t* handle) { static void uv__finish_close(uv_handle_t* handle) { - assert(!uv__is_active(handle)); + /* Note: while the handle is in the UV_CLOSING state now, it's still possible + * for it to be active in the sense that uv__is_active() returns true. + * A good example is when the user calls uv_shutdown(), immediately followed + * by uv_close(). The handle is considered active at this point because the + * completion of the shutdown req is still pending. + */ assert(handle->flags & UV_CLOSING); assert(!(handle->flags & UV_CLOSED)); handle->flags |= UV_CLOSED; diff --git a/src/unix/stream.c b/src/unix/stream.c index aeefa2c4..fb1be005 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -429,6 +429,11 @@ void uv__stream_destroy(uv_stream_t* stream) { } if (stream->shutdown_req) { + /* The UV_ECANCELED error code is a lie, the shutdown(2) syscall is a + * fait accompli at this point. Maybe we should revisit this in v0.11. + * A possible reason for leaving it unchanged is that it informs the + * callee that the handle has been destroyed. + */ uv__req_unregister(stream->loop, stream->shutdown_req); uv__set_artificial_error(stream->loop, UV_ECANCELED); stream->shutdown_req->cb(stream->shutdown_req, -1); @@ -1318,9 +1323,10 @@ int uv_read_stop(uv_stream_t* stream) { stream->shutdown_req != NULL || stream->connect_req != NULL); - uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLIN); - uv__handle_stop(stream); stream->flags &= ~UV_STREAM_READING; + uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLIN); + if (!uv__io_active(&stream->io_watcher, UV__POLLOUT)) + uv__handle_stop(stream); #if defined(__APPLE__) /* Notify select() thread about state change */ From b38c9c1004993ca4f642629f5af1b7b09bbc6887 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 25 May 2013 02:36:45 +0200 Subject: [PATCH 12/23] unix: remove erroneous asserts As of commit c53fe81, it's legal for write_queue_size > 0 when the write_queue itself is empty. Sounds illogical but it means there are error-state write requests in the write_completed_queue that will touch up the write_queue_size on the next tick of the event loop. Remove a few stray asserts that still checked for the old situation. --- src/unix/stream.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/unix/stream.c b/src/unix/stream.c index fb1be005..892838cc 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -632,8 +632,6 @@ static void uv__drain(uv_stream_t* stream) { uv_shutdown_t* req; assert(ngx_queue_empty(&stream->write_queue)); - assert(stream->write_queue_size == 0); - uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLOUT); /* Shutdown? */ @@ -727,10 +725,8 @@ start: assert(uv__stream_fd(stream) >= 0); - if (ngx_queue_empty(&stream->write_queue)) { - assert(stream->write_queue_size == 0); + if (ngx_queue_empty(&stream->write_queue)) return; - } q = ngx_queue_head(&stream->write_queue); req = ngx_queue_data(q, uv_write_t, queue); @@ -1205,6 +1201,12 @@ int uv_write2(uv_write_t* req, return uv__set_artificial_error(stream->loop, UV_EBADF); } + /* It's legal for write_queue_size > 0 even when the write_queue is empty; + * it means there are error-state requests in the write_completed_queue that + * will touch up write_queue_size later, see also uv__write_req_finish(). + * We chould check that write_queue is empty instead but that implies making + * a write() syscall when we know that the handle is in error mode. + */ empty_queue = (stream->write_queue_size == 0); /* Initialize the req */ From 8e16f8e0564a7b853c2cb0f92572e7959c6cadae Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 26 May 2013 23:02:17 +0200 Subject: [PATCH 13/23] unix: add uv__is_closing() macro --- src/unix/core.c | 2 +- src/uv-common.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/unix/core.c b/src/unix/core.c index b7f0da38..61d72493 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -225,7 +225,7 @@ static void uv__run_closing_handles(uv_loop_t* loop) { int uv_is_closing(const uv_handle_t* handle) { - return handle->flags & (UV_CLOSING | UV_CLOSED); + return uv__is_closing(handle); } diff --git a/src/uv-common.h b/src/uv-common.h index 80c9c719..bbf24859 100644 --- a/src/uv-common.h +++ b/src/uv-common.h @@ -149,6 +149,9 @@ void uv__fs_poll_close(uv_fs_poll_t* handle); #define uv__is_active(h) \ (((h)->flags & UV__HANDLE_ACTIVE) != 0) +#define uv__is_closing(h) \ + (((h)->flags & (UV_CLOSING | UV_CLOSED)) != 0) + #define uv__handle_start(h) \ do { \ assert(((h)->flags & UV__HANDLE_CLOSING) == 0); \ From b329d51ef4ce32f34c21a016a7c311ddeb077878 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 26 May 2013 23:44:55 +0200 Subject: [PATCH 14/23] unix: stop stream POLLOUT watcher on write error The node.js test suite sometimes hits the assert that was added in commit 4146805 that checks if there are connect, write or shutdown requests pending when the user calls uv_read_stop() while the stream is primed for writing. The libuv user (i.e. node.js) is supposed to close the stream on error. Because uv__stream_close() calls uv_read_stop(), it's possible that the POLLOUT watcher is still active. --- src/unix/stream.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/unix/stream.c b/src/unix/stream.c index 892838cc..52972d9c 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -798,6 +798,9 @@ start: /* Error */ req->error = errno; uv__write_req_finish(req); + uv__io_stop(stream->loop, &stream->io_watcher, UV__POLLOUT); + if (!uv__io_active(&stream->io_watcher, UV__POLLIN)) + uv__handle_stop(stream); return; } else if (stream->flags & UV_STREAM_BLOCKING) { /* If this is a blocking stream, try again. */ From a195f9ace23d92345baf57582678bfc3017e6632 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 28 May 2013 12:08:46 -0700 Subject: [PATCH 15/23] 2013.05.29, Version 0.10.9 (Stable) Changes since version 0.10.8: * unix: fix stream refcounting buglet (Ben Noordhuis) * unix: remove erroneous asserts (Ben Noordhuis) * unix: add uv__is_closing() macro (Ben Noordhuis) * unix: stop stream POLLOUT watcher on write error (Ben Noordhuis) --- ChangeLog | 13 +++++++++++++ src/version.c | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index eb28efcb..1451180c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2013.05.29, Version 0.10.9 (Stable) + +Changes since version 0.10.8: + +* unix: fix stream refcounting buglet (Ben Noordhuis) + +* unix: remove erroneous asserts (Ben Noordhuis) + +* unix: add uv__is_closing() macro (Ben Noordhuis) + +* unix: stop stream POLLOUT watcher on write error (Ben Noordhuis) + + 2013.05.25, Version 0.10.8 (Stable), 0f39be12926fe2d8766a9f025797a473003e6504 Changes since version 0.10.7: diff --git a/src/version.c b/src/version.c index 2d0100f3..9bf92fdd 100644 --- a/src/version.c +++ b/src/version.c @@ -35,7 +35,7 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 #define UV_VERSION_PATCH 9 -#define UV_VERSION_IS_RELEASE 0 +#define UV_VERSION_IS_RELEASE 1 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From 21c12b824a07be22a24547904b50ff022db11dd7 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 28 May 2013 12:08:49 -0700 Subject: [PATCH 16/23] Now working on v0.10.10 --- ChangeLog | 2 +- src/version.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1451180c..3954586c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,4 @@ -2013.05.29, Version 0.10.9 (Stable) +2013.05.29, Version 0.10.9 (Stable), a195f9ace23d92345baf57582678bfc3017e6632 Changes since version 0.10.8: diff --git a/src/version.c b/src/version.c index 9bf92fdd..a9f96eeb 100644 --- a/src/version.c +++ b/src/version.c @@ -34,8 +34,8 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 -#define UV_VERSION_PATCH 9 -#define UV_VERSION_IS_RELEASE 1 +#define UV_VERSION_PATCH 10 +#define UV_VERSION_IS_RELEASE 0 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From dfff2e9e2336ac7b89234c3f7744a73fc6560bb1 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 28 May 2013 23:20:35 +0200 Subject: [PATCH 17/23] include: document uv_update_time() and uv_now() --- include/uv.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/include/uv.h b/include/uv.h index dd836131..19075f99 100644 --- a/include/uv.h +++ b/include/uv.h @@ -287,7 +287,28 @@ UV_EXTERN void uv_stop(uv_loop_t*); UV_EXTERN void uv_ref(uv_handle_t*); UV_EXTERN void uv_unref(uv_handle_t*); +/* + * Update the event loop's concept of "now". Libuv caches the current time + * at the start of the event loop tick in order to reduce the number of + * time-related system calls. + * + * You won't normally need to call this function unless you have callbacks + * that block the event loop for longer periods of time, where "longer" is + * somewhat subjective but probably on the order of a millisecond or more. + */ UV_EXTERN void uv_update_time(uv_loop_t*); + +/* + * Return the current timestamp in milliseconds. The timestamp is cached at + * the start of the event loop tick, see |uv_update_time()| for details and + * rationale. + * + * The timestamp increases monotonically from some arbitrary point in time. + * Don't make assumptions about the starting point, you will only get + * disappointed. + * + * Use uv_hrtime() if you need sub-milliseond granularity. + */ UV_EXTERN uint64_t uv_now(uv_loop_t*); /* From 92c72f58bf59ee51a1680dd52b0e91a0ccae485d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 29 May 2013 00:24:02 +0200 Subject: [PATCH 18/23] linux: fix cpu model parsing on newer arm kernels The format of /proc/cpuinfo on ARM kernels >= 3.8 has changed. Scan for the string "model name" (like x86) first, "Processor" second. Fixes #812. --- src/unix/linux-core.c | 97 +++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c index 35374b17..cb920447 100644 --- a/src/unix/linux-core.c +++ b/src/unix/linux-core.c @@ -416,75 +416,74 @@ static void read_speeds(unsigned int numcpus, uv_cpu_info_t* ci) { * a BogoMIPS field, which may not be very accurate. */ static int read_models(unsigned int numcpus, uv_cpu_info_t* ci) { -#if defined(__i386__) || defined(__x86_64__) static const char model_marker[] = "model name\t: "; static const char speed_marker[] = "cpu MHz\t\t: "; -#elif defined(__arm__) - static const char model_marker[] = "Processor\t: "; - static const char speed_marker[] = ""; -#elif defined(__mips__) - static const char model_marker[] = "cpu model\t\t: "; - static const char speed_marker[] = ""; -#else -# warning uv_cpu_info() is not supported on this architecture. - static const char model_marker[] = ""; - static const char speed_marker[] = ""; -#endif - static const char bogus_model[] = "unknown"; + const char* inferred_model; unsigned int model_idx; unsigned int speed_idx; char buf[1024]; char* model; FILE* fp; - char* inferred_model; - fp = fopen("/proc/cpuinfo", "r"); - if (fp == NULL) - return -1; + /* Most are unused on non-ARM and non-x86 architectures. */ + (void) &model_marker; + (void) &speed_marker; + (void) &speed_idx; + (void) &model; + (void) &buf; + (void) &fp; model_idx = 0; speed_idx = 0; +#if defined(__arm__) || defined(__i386__) || defined(__x86_64__) + fp = fopen("/proc/cpuinfo", "r"); + if (fp == NULL) + return -1; + while (fgets(buf, sizeof(buf), fp)) { - if (model_marker[0] != '\0' && - model_idx < numcpus && - strncmp(buf, model_marker, sizeof(model_marker) - 1) == 0) - { - model = buf + sizeof(model_marker) - 1; - model = strndup(model, strlen(model) - 1); /* strip newline */ - ci[model_idx++].model = model; - continue; + if (model_idx < numcpus) { + if (strncmp(buf, model_marker, sizeof(model_marker) - 1) == 0) { + model = buf + sizeof(model_marker) - 1; + model = strndup(model, strlen(model) - 1); /* Strip newline. */ + ci[model_idx++].model = model; + continue; + } } - - if (speed_marker[0] != '\0' && - speed_idx < numcpus && - strncmp(buf, speed_marker, sizeof(speed_marker) - 1) == 0) - { - ci[speed_idx++].speed = atoi(buf + sizeof(speed_marker) - 1); - continue; +#if defined(__arm__) + /* Fallback for pre-3.8 kernels. */ + if (model_idx < numcpus) { + static const char model_marker[] = "Processor\t: "; + if (strncmp(buf, model_marker, sizeof(model_marker) - 1) == 0) { + model = buf + sizeof(model_marker) - 1; + model = strndup(model, strlen(model) - 1); /* Strip newline. */ + ci[model_idx++].model = model; + continue; + } } +#else /* !__arm____ */ + if (speed_idx < numcpus) { + if (strncmp(buf, speed_marker, sizeof(speed_marker) - 1) == 0) { + ci[speed_idx++].speed = atoi(buf + sizeof(speed_marker) - 1); + continue; + } + } +#endif /* __arm__ */ } + fclose(fp); +#endif /* __arm__ || __i386__ || __x86_64__ */ - /* Now we want to make sure that all the models contain *something*: - * it's not safe to leave them as null. + /* Now we want to make sure that all the models contain *something* because + * it's not safe to leave them as null. Copy the last entry unless there + * isn't one, in that case we simply put "unknown" into everything. */ - if (model_idx == 0) { - /* No models at all: fake up the first one. */ - ci[0].model = strndup(bogus_model, sizeof(bogus_model) - 1); - model_idx = 1; - } + inferred_model = "unknown"; + if (model_idx > 0) + inferred_model = ci[model_idx - 1].model; - /* Not enough models, but we do have at least one. So we'll just - * copy the rest down: it might be better to indicate somehow that - * the remaining ones have been guessed. - */ - inferred_model = ci[model_idx - 1].model; - - while (model_idx < numcpus) { - ci[model_idx].model = strndup(inferred_model, strlen(inferred_model)); - model_idx++; - } + while (model_idx < numcpus) + ci[model_idx++].model = strndup(inferred_model, strlen(inferred_model)); return 0; } From 31282a97e70b24df7ebe4692967fee2a48aa2096 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 29 May 2013 01:25:37 +0200 Subject: [PATCH 19/23] linux: fix memory leak in uv_cpu_info() error path Any memory allocated to hold CPU model strings wasn't freed on error. --- src/unix/linux-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c index cb920447..62730f5c 100644 --- a/src/unix/linux-core.c +++ b/src/unix/linux-core.c @@ -382,12 +382,12 @@ uv_err_t uv_cpu_info(uv_cpu_info_t** cpu_infos, int* count) { return uv__new_sys_error(ENOMEM); if (read_models(numcpus, ci)) { - SAVE_ERRNO(free(ci)); + SAVE_ERRNO(uv_free_cpu_info(ci, numcpus)); return uv__new_sys_error(errno); } if (read_times(numcpus, ci)) { - SAVE_ERRNO(free(ci)); + SAVE_ERRNO(uv_free_cpu_info(ci, numcpus)); return uv__new_sys_error(errno); } From b93cf8b594b5eaf4617174e674961fd3db3fb0c6 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 29 May 2013 01:37:36 +0200 Subject: [PATCH 20/23] linux: don't ignore OOM errors in uv_cpu_info() --- src/unix/linux-core.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c index 62730f5c..13765ae3 100644 --- a/src/unix/linux-core.c +++ b/src/unix/linux-core.c @@ -414,6 +414,8 @@ static void read_speeds(unsigned int numcpus, uv_cpu_info_t* ci) { /* Also reads the CPU frequency on x86. The other architectures only have * a BogoMIPS field, which may not be very accurate. + * + * Note: Simply returns on error, uv_cpu_info() takes care of the cleanup. */ static int read_models(unsigned int numcpus, uv_cpu_info_t* ci) { static const char model_marker[] = "model name\t: "; @@ -446,6 +448,10 @@ static int read_models(unsigned int numcpus, uv_cpu_info_t* ci) { if (strncmp(buf, model_marker, sizeof(model_marker) - 1) == 0) { model = buf + sizeof(model_marker) - 1; model = strndup(model, strlen(model) - 1); /* Strip newline. */ + if (model == NULL) { + fclose(fp); + return -1; + } ci[model_idx++].model = model; continue; } @@ -457,6 +463,10 @@ static int read_models(unsigned int numcpus, uv_cpu_info_t* ci) { if (strncmp(buf, model_marker, sizeof(model_marker) - 1) == 0) { model = buf + sizeof(model_marker) - 1; model = strndup(model, strlen(model) - 1); /* Strip newline. */ + if (model == NULL) { + fclose(fp); + return -1; + } ci[model_idx++].model = model; continue; } @@ -482,8 +492,12 @@ static int read_models(unsigned int numcpus, uv_cpu_info_t* ci) { if (model_idx > 0) inferred_model = ci[model_idx - 1].model; - while (model_idx < numcpus) - ci[model_idx++].model = strndup(inferred_model, strlen(inferred_model)); + while (model_idx < numcpus) { + model = strndup(inferred_model, strlen(inferred_model)); + if (model == NULL) + return -1; + ci[model_idx++].model = model; + } return 0; } From e0bdb3dbc916d8311538de2b783c53e9739bf652 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 29 May 2013 16:13:34 +0200 Subject: [PATCH 21/23] unix, windows: move uv_now() to uv-common.c --- src/unix/core.c | 5 ----- src/uv-common.c | 5 +++++ src/win/timer.c | 5 ----- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/unix/core.c b/src/unix/core.c index 61d72493..e8e5f9e6 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -340,11 +340,6 @@ void uv_update_time(uv_loop_t* loop) { } -uint64_t uv_now(uv_loop_t* loop) { - return loop->time; -} - - int uv_is_active(const uv_handle_t* handle) { return uv__is_active(handle); } diff --git a/src/uv-common.c b/src/uv-common.c index 8c9d323a..f0792f11 100644 --- a/src/uv-common.c +++ b/src/uv-common.c @@ -429,3 +429,8 @@ void uv_unref(uv_handle_t* handle) { void uv_stop(uv_loop_t* loop) { loop->stop_flag = 1; } + + +uint64_t uv_now(uv_loop_t* loop) { + return loop->time; +} diff --git a/src/win/timer.c b/src/win/timer.c index 7fd10571..8dae7414 100644 --- a/src/win/timer.c +++ b/src/win/timer.c @@ -45,11 +45,6 @@ void uv_update_time(uv_loop_t* loop) { } -uint64_t uv_now(uv_loop_t* loop) { - return loop->time; -} - - static int uv_timer_compare(uv_timer_t* a, uv_timer_t* b) { if (a->due < b->due) return -1; From 081f7018ecc1c66a76f76c4b5cacb327820674b9 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Wed, 29 May 2013 18:32:25 +0300 Subject: [PATCH 22/23] test: use c-style comments Fixes a compilation problem on OS X caused by the use of c++-style comments in test-osx-select.c. --- test/test-osx-select.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-osx-select.c b/test/test-osx-select.c index 5c2cbd4d..bf4c3952 100644 --- a/test/test-osx-select.c +++ b/test/test-osx-select.c @@ -62,7 +62,7 @@ TEST_IMPL(osx_select) { uv_read_start((uv_stream_t*) &tty, alloc_cb, read_cb); - // Emulate user-input + /* Emulate user-input */ str = "got some input\n" "with a couple of lines\n" "feel pretty happy\n"; From b4c658c3c0e650590cc0496833fead4f29deea75 Mon Sep 17 00:00:00 2001 From: Wynn Wilkes Date: Wed, 29 May 2013 12:13:34 -0600 Subject: [PATCH 23/23] darwin: make uv_fs_sendfile() respect length param The darwin sendfile implementation uses the &len parameter as input and output. The code was sending 0 (not using the value of req->len) so the behavior wasn't what the caller was expecting. This makes sure to initialize len with req->len to ensure that the caller can send portions of a file (not always everything to the end of the file). --- src/unix/fs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/unix/fs.c b/src/unix/fs.c index 53f46ce7..2f58a563 100644 --- a/src/unix/fs.c +++ b/src/unix/fs.c @@ -436,11 +436,14 @@ static ssize_t uv__fs_sendfile(uv_fs_t* req) { * non-blocking mode and not all data could be written. If a non-zero * number of bytes have been sent, we don't consider it an error. */ - len = 0; #if defined(__FreeBSD__) + len = 0; r = sendfile(in_fd, out_fd, req->off, req->len, NULL, &len, 0); #else + /* The darwin sendfile takes len as an input for the length to send, + * so make sure to initialize it with the caller's value. */ + len = req->len; r = sendfile(in_fd, out_fd, req->off, &len, NULL, 0); #endif