Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 60311fe

Browse files
committedFeb 2, 2019
src: fix race condition in ~NodeTraceBuffer
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: #25512
1 parent 28c0f84 commit 60311fe

File tree

1 file changed

+18
-7
lines changed

1 file changed

+18
-7
lines changed
 

‎src/tracing/node_trace_buffer.cc

+18-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "tracing/node_trace_buffer.h"
2+
#include "util-inl.h"
23

34
namespace node {
45
namespace tracing {
@@ -170,15 +171,25 @@ void NodeTraceBuffer::NonBlockingFlushSignalCb(uv_async_t* signal) {
170171

171172
// static
172173
void NodeTraceBuffer::ExitSignalCb(uv_async_t* signal) {
173-
NodeTraceBuffer* buffer = reinterpret_cast<NodeTraceBuffer*>(signal->data);
174-
uv_close(reinterpret_cast<uv_handle_t*>(&buffer->flush_signal_), nullptr);
175-
uv_close(reinterpret_cast<uv_handle_t*>(&buffer->exit_signal_),
174+
NodeTraceBuffer* buffer =
175+
ContainerOf(&NodeTraceBuffer::exit_signal_, signal);
176+
177+
// Close both flush_signal_ and exit_signal_.
178+
uv_close(reinterpret_cast<uv_handle_t*>(&buffer->flush_signal_),
176179
[](uv_handle_t* signal) {
180+
NodeTraceBuffer* buffer =
181+
ContainerOf(&NodeTraceBuffer::flush_signal_,
182+
reinterpret_cast<uv_async_t*>(signal));
183+
184+
uv_close(reinterpret_cast<uv_handle_t*>(&buffer->exit_signal_),
185+
[](uv_handle_t* signal) {
177186
NodeTraceBuffer* buffer =
178-
reinterpret_cast<NodeTraceBuffer*>(signal->data);
179-
Mutex::ScopedLock scoped_lock(buffer->exit_mutex_);
180-
buffer->exited_ = true;
181-
buffer->exit_cond_.Signal(scoped_lock);
187+
ContainerOf(&NodeTraceBuffer::exit_signal_,
188+
reinterpret_cast<uv_async_t*>(signal));
189+
Mutex::ScopedLock scoped_lock(buffer->exit_mutex_);
190+
buffer->exited_ = true;
191+
buffer->exit_cond_.Signal(scoped_lock);
192+
});
182193
});
183194
}
184195

0 commit comments

Comments
 (0)
Please sign in to comment.