luo980/libuv
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

unix: fix stream refcounting buglet

Browse Source 
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.
This commit is contained in:
Ben Noordhuis 2013-05-23 07:16:00 +02:00
parent 7d5024e7e6
commit 636a13b8c4
2 changed files with 14 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/unix/core.c
View File

@ -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;

10
src/unix/stream.c
View File

@ -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 */