Skip to content

Commit b2c1bb3

Browse files
santigimenotrevnorris
authored andcommitted
agents: fix OTLPAgent race conditions on cleanup
Move the `exit_lock_` out of the class and make it static so we can use it even if the agent has been deleted. Also, make sure we use the `exit_lock_` in `OTLPAgent::config_msg_cb_`. Fixes: #29 PR-URL: #30 Reviewed-by: Trevor Norris <[email protected]>
1 parent 7362f32 commit b2c1bb3

File tree

2 files changed

+15
-11
lines changed

2 files changed

+15
-11
lines changed

agents/otlp/src/otlp_agent.cc

+15-9
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ static const size_t kTraceIdSize = 32;
2222
static const size_t kSpanIdSize = 16;
2323

2424
static std::atomic<bool> is_running_ = { false };
25+
nsuv::ns_rwlock exit_lock_;
2526
static resource::ResourceAttributes resource_attributes_;
2627

2728
namespace node {
@@ -203,7 +204,7 @@ int OTLPAgent::config(const nlohmann::json& config) {
203204
/*static*/ void OTLPAgent::config_agent_cb_(std::string config,
204205
OTLPAgent* agent) {
205206
// Check if the agent is already delete or if it's closing
206-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
207+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
207208
if (!is_running_) {
208209
return;
209210
}
@@ -217,6 +218,11 @@ int OTLPAgent::config(const nlohmann::json& config) {
217218

218219

219220
/*static*/ void OTLPAgent::config_msg_cb_(nsuv::ns_async*, OTLPAgent* agent) {
221+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
222+
if (!is_running_) {
223+
return;
224+
}
225+
220226
json config_msg;
221227
while (agent->config_msg_q_.dequeue(config_msg)) {
222228
agent->config(config_msg);
@@ -225,7 +231,7 @@ int OTLPAgent::config(const nlohmann::json& config) {
225231

226232

227233
/*static*/ void OTLPAgent::env_msg_cb_(nsuv::ns_async*, OTLPAgent* agent) {
228-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
234+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
229235
if (!is_running_) {
230236
return;
231237
}
@@ -249,7 +255,7 @@ int OTLPAgent::config(const nlohmann::json& config) {
249255

250256
/*static*/ void OTLPAgent::on_thread_add_(SharedEnvInst envinst,
251257
OTLPAgent* agent) {
252-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
258+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
253259
if (!is_running_) {
254260
return;
255261
}
@@ -261,7 +267,7 @@ int OTLPAgent::config(const nlohmann::json& config) {
261267

262268
/*static*/ void OTLPAgent::on_thread_remove_(SharedEnvInst envinst,
263269
OTLPAgent* agent) {
264-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
270+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
265271
if (!is_running_) {
266272
return;
267273
}
@@ -309,7 +315,7 @@ void OTLPAgent::do_stop() {
309315
void OTLPAgent::trace_hook_(Tracer* tracer,
310316
const Tracer::SpanStor& stor,
311317
OTLPAgent* agent) {
312-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
318+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
313319
if (!is_running_) {
314320
return;
315321
}
@@ -321,7 +327,7 @@ void OTLPAgent::trace_hook_(Tracer* tracer,
321327

322328
void OTLPAgent::span_msg_cb_(nsuv::ns_async*, OTLPAgent* agent) {
323329
// Don't exit until all the pending spans are sent
324-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
330+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
325331
if (!is_running_) {
326332
return;
327333
}
@@ -396,7 +402,7 @@ void OTLPAgent::span_msg_cb_(nsuv::ns_async*, OTLPAgent* agent) {
396402

397403

398404
/*static*/void OTLPAgent::metrics_timer_cb_(nsuv::ns_timer*, OTLPAgent* agent) {
399-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
405+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
400406
if (!is_running_) {
401407
return;
402408
}
@@ -411,7 +417,7 @@ void OTLPAgent::span_msg_cb_(nsuv::ns_async*, OTLPAgent* agent) {
411417

412418

413419
/*static*/void OTLPAgent::metrics_msg_cb_(nsuv::ns_async*, OTLPAgent* agent) {
414-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
420+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
415421
if (!is_running_) {
416422
return;
417423
}
@@ -437,7 +443,7 @@ void OTLPAgent::span_msg_cb_(nsuv::ns_async*, OTLPAgent* agent) {
437443

438444
/*static*/void OTLPAgent::thr_metrics_cb_(ThreadMetrics* metrics,
439445
OTLPAgent* agent) {
440-
nsuv::ns_rwlock::scoped_rdlock lock(agent->exit_lock_);
446+
nsuv::ns_rwlock::scoped_rdlock lock(exit_lock_);
441447
if (!is_running_) {
442448
return;
443449
}

agents/otlp/src/otlp_agent.h

-2
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,6 @@ class OTLPAgent {
112112
uv_cond_t start_cond_;
113113
uv_mutex_t start_lock_;
114114

115-
nsuv::ns_rwlock exit_lock_;
116-
117115
bool hooks_init_;
118116

119117
nsuv::ns_async env_msg_;

0 commit comments

Comments
 (0)