Skip to content

Commit

Permalink
src: fix race condition in ~NodeTraceBuffer
Browse files Browse the repository at this point in the history
Libuv does not guarantee that handles have their close
callbacks called in the order in which they were added
(and in fact, currently calls them in reverse order).

This patch ensures that the `flush_signal_` handle
is no longer in use (i.e. its close callback has already
been run) when we signal to the main thread that
`~NodeTraceBuffer` may be destroyed.

Credit for debugging goes to Gireesh Punathil.

Fixes: nodejs#25512
  • Loading branch information
addaleax committed Feb 2, 2019
1 parent 28c0f84 commit 60311fe
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions src/tracing/node_trace_buffer.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "tracing/node_trace_buffer.h"
#include "util-inl.h"

namespace node {
namespace tracing {
Expand Down Expand Up @@ -170,15 +171,25 @@ void NodeTraceBuffer::NonBlockingFlushSignalCb(uv_async_t* signal) {

// static
void NodeTraceBuffer::ExitSignalCb(uv_async_t* signal) {
NodeTraceBuffer* buffer = reinterpret_cast<NodeTraceBuffer*>(signal->data);
uv_close(reinterpret_cast<uv_handle_t*>(&buffer->flush_signal_), nullptr);
uv_close(reinterpret_cast<uv_handle_t*>(&buffer->exit_signal_),
NodeTraceBuffer* buffer =
ContainerOf(&NodeTraceBuffer::exit_signal_, signal);

// Close both flush_signal_ and exit_signal_.
uv_close(reinterpret_cast<uv_handle_t*>(&buffer->flush_signal_),
[](uv_handle_t* signal) {
NodeTraceBuffer* buffer =
ContainerOf(&NodeTraceBuffer::flush_signal_,
reinterpret_cast<uv_async_t*>(signal));

uv_close(reinterpret_cast<uv_handle_t*>(&buffer->exit_signal_),
[](uv_handle_t* signal) {
NodeTraceBuffer* buffer =
reinterpret_cast<NodeTraceBuffer*>(signal->data);
Mutex::ScopedLock scoped_lock(buffer->exit_mutex_);
buffer->exited_ = true;
buffer->exit_cond_.Signal(scoped_lock);
ContainerOf(&NodeTraceBuffer::exit_signal_,
reinterpret_cast<uv_async_t*>(signal));
Mutex::ScopedLock scoped_lock(buffer->exit_mutex_);
buffer->exited_ = true;
buffer->exit_cond_.Signal(scoped_lock);
});
});
}

Expand Down

0 comments on commit 60311fe

Please sign in to comment.