Skip to content

Commit 7b5db07

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

File tree

3 files changed

+17
-19
lines changed

3 files changed

+17
-19
lines changed

ext/sidecar.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
#include <php.h>
22
#include <main/SAPI.h>
3-
#include <stdatomic.h>
43
#include "ddtrace.h"
54
#include "auto_flush.h"
65
#include "compat_string.h"
@@ -31,9 +30,10 @@ struct ddog_InstanceId *ddtrace_sidecar_instance_id;
3130
static uint8_t dd_sidecar_formatted_session_id[36];
3231

3332
// 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;
33+
// per-thread connection; never cleared until MSHUTDOWN. Not atomic: concurrent
34+
// shutdown is already a best-effort race for signal handlers, so atomicity of
35+
// the pointer load alone would not prevent the underlying use-after-free.
36+
ddog_SidecarTransport *ddtrace_sidecar_for_signal = NULL;
3737

3838
// Connection mode tracking
3939
dd_sidecar_active_mode_t ddtrace_sidecar_active_mode = DD_SIDECAR_CONNECTION_NONE;
@@ -406,9 +406,8 @@ void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) {
406406
}
407407

408408
// 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));
409+
if (DDTRACE_G(sidecar) && !ddtrace_sidecar_for_signal) {
410+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
412411
}
413412
}
414413

@@ -445,7 +444,7 @@ void ddtrace_sidecar_handle_fork(void) {
445444
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));
446445
DDTRACE_G(sidecar) = NULL;
447446
}
448-
atomic_store(&ddtrace_sidecar_for_signal, NULL);
447+
ddtrace_sidecar_for_signal = NULL;
449448

450449
if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD) {
451450
ddtrace_ffi_try("Failed clearing inherited listener state",
@@ -486,7 +485,7 @@ void ddtrace_sidecar_handle_fork(void) {
486485
}
487486

488487
if (DDTRACE_G(sidecar)) {
489-
atomic_store(&ddtrace_sidecar_for_signal, DDTRACE_G(sidecar));
488+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
490489
}
491490
#endif
492491
}
@@ -498,9 +497,8 @@ void ddtrace_sidecar_ensure_active(void) {
498497
// First RINIT on this thread: the process-level setup already ran (endpoint is
499498
// set), so establish this thread's own connection now.
500499
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));
500+
if (DDTRACE_G(sidecar) && !ddtrace_sidecar_for_signal) {
501+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
504502
}
505503
}
506504
}
@@ -540,6 +538,8 @@ void ddtrace_sidecar_shutdown(void) {
540538
current_pid == ddtrace_sidecar_master_pid) {
541539

542540
if (DDTRACE_G(sidecar)) {
541+
ddtrace_sidecar_for_signal = NULL;
542+
543543
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));
544544
DDTRACE_G(sidecar) = NULL;
545545
}
@@ -554,8 +554,6 @@ void ddtrace_sidecar_shutdown(void) {
554554
ddtrace_sidecar_instance_id = NULL;
555555
}
556556

557-
atomic_store(&ddtrace_sidecar_for_signal, NULL);
558-
559557
if (ddtrace_endpoint) {
560558
dd_free_endpoints();
561559
}
@@ -873,6 +871,8 @@ void ddtrace_sidecar_rshutdown(void) {
873871

874872
void ddtrace_sidecar_gshutdown(void) {
875873
if (DDTRACE_G(sidecar)) {
874+
ddtrace_sidecar_for_signal = NULL;
875+
876876
// Drain any accumulated background-sender metrics before the transport goes away.
877877
ddtrace_telemetry_flush_bgs_metrics_final();
878878
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)