Skip to content

Commit d58b820

Browse files
committed
Fix windows
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
1 parent 1b4b37f commit d58b820

File tree

3 files changed

+17
-18
lines changed

3 files changed

+17
-18
lines changed

ext/sidecar.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ struct ddog_InstanceId *ddtrace_sidecar_instance_id;
3131
static uint8_t dd_sidecar_formatted_session_id[36];
3232

3333
// Best-effort pointer for the signal handler (SIGTERM/SIGINT). Set to the first
34-
// per-thread connection; never cleared until MSHUTDOWN. Signal context cannot
35-
// safely call TSRMLS_FETCH(), so this is the only option.
36-
_Atomic(ddog_SidecarTransport *) ddtrace_sidecar_for_signal = NULL;
34+
// per-thread connection; never cleared until MSHUTDOWN. Not atomic: concurrent
35+
// shutdown is already a best-effort race for signal handlers, so atomicity of
36+
// the pointer load alone would not prevent the underlying use-after-free.
37+
ddog_SidecarTransport *ddtrace_sidecar_for_signal = NULL;
3738

3839
// Connection mode tracking
3940
dd_sidecar_active_mode_t ddtrace_sidecar_active_mode = DD_SIDECAR_CONNECTION_NONE;
@@ -406,9 +407,8 @@ void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) {
406407
}
407408

408409
// Record the first established connection for best-effort signal-handler use.
409-
if (DDTRACE_G(sidecar)) {
410-
ddog_SidecarTransport *expected = NULL;
411-
atomic_compare_exchange_strong(&ddtrace_sidecar_for_signal, &expected, DDTRACE_G(sidecar));
410+
if (DDTRACE_G(sidecar) && !ddtrace_sidecar_for_signal) {
411+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
412412
}
413413
}
414414

@@ -445,7 +445,7 @@ void ddtrace_sidecar_handle_fork(void) {
445445
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));
446446
DDTRACE_G(sidecar) = NULL;
447447
}
448-
atomic_store(&ddtrace_sidecar_for_signal, NULL);
448+
ddtrace_sidecar_for_signal = NULL;
449449

450450
if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD) {
451451
ddtrace_ffi_try("Failed clearing inherited listener state",
@@ -486,7 +486,7 @@ void ddtrace_sidecar_handle_fork(void) {
486486
}
487487

488488
if (DDTRACE_G(sidecar)) {
489-
atomic_store(&ddtrace_sidecar_for_signal, DDTRACE_G(sidecar));
489+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
490490
}
491491
#endif
492492
}
@@ -498,9 +498,8 @@ void ddtrace_sidecar_ensure_active(void) {
498498
// First RINIT on this thread: the process-level setup already ran (endpoint is
499499
// set), so establish this thread's own connection now.
500500
DDTRACE_G(sidecar) = ddtrace_sidecar_connect(false);
501-
if (DDTRACE_G(sidecar)) {
502-
ddog_SidecarTransport *expected = NULL;
503-
atomic_compare_exchange_strong(&ddtrace_sidecar_for_signal, &expected, DDTRACE_G(sidecar));
501+
if (DDTRACE_G(sidecar) && !ddtrace_sidecar_for_signal) {
502+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
504503
}
505504
}
506505
}
@@ -540,6 +539,8 @@ void ddtrace_sidecar_shutdown(void) {
540539
current_pid == ddtrace_sidecar_master_pid) {
541540

542541
if (DDTRACE_G(sidecar)) {
542+
ddtrace_sidecar_for_signal = NULL;
543+
543544
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));
544545
DDTRACE_G(sidecar) = NULL;
545546
}
@@ -554,8 +555,6 @@ void ddtrace_sidecar_shutdown(void) {
554555
ddtrace_sidecar_instance_id = NULL;
555556
}
556557

557-
atomic_store(&ddtrace_sidecar_for_signal, NULL);
558-
559558
if (ddtrace_endpoint) {
560559
dd_free_endpoints();
561560
}
@@ -873,6 +872,8 @@ void ddtrace_sidecar_rshutdown(void) {
873872

874873
void ddtrace_sidecar_gshutdown(void) {
875874
if (DDTRACE_G(sidecar)) {
875+
ddtrace_sidecar_for_signal = NULL;
876+
876877
// Drain any accumulated background-sender metrics before the transport goes away.
877878
ddtrace_telemetry_flush_bgs_metrics_final();
878879
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));

ext/sidecar.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ typedef enum {
1919
extern struct ddog_InstanceId *ddtrace_sidecar_instance_id;
2020
// Best-effort pointer used only by the signal handler (SIGTERM/SIGINT), which cannot call
2121
// TSRMLS_FETCH() safely. Set to the first thread's connection; never cleared until MSHUTDOWN.
22-
extern _Atomic(ddog_SidecarTransport *) ddtrace_sidecar_for_signal;
22+
// Not atomic: concurrent shutdown is a pre-existing best-effort race for signal handlers.
23+
extern ddog_SidecarTransport *ddtrace_sidecar_for_signal;
2324
extern ddog_Endpoint *ddtrace_endpoint;
2425
extern dd_sidecar_active_mode_t ddtrace_sidecar_active_mode;
2526
extern int32_t ddtrace_sidecar_master_pid;

ext/signals.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -363,10 +363,7 @@ static int dd_call_prev_handler(bool flush) {
363363
}
364364

365365
if (flush) {
366-
// Load into a plain local: the function expects ddog_SidecarTransport **,
367-
// but ddtrace_sidecar_for_signal is _Atomic so &it would be the wrong type.
368-
ddog_SidecarTransport *sidecar = atomic_load(&ddtrace_sidecar_for_signal);
369-
ddog_sidecar_flush_traces(&sidecar);
366+
ddog_sidecar_flush_traces(&ddtrace_sidecar_for_signal);
370367
}
371368

372369
if (prev_handler == SIG_DFL) {

0 commit comments

Comments
 (0)