Skip to content

Commit fd34daf

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

6 files changed

Lines changed: 29 additions & 28 deletions

File tree

ext/coms.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -771,10 +771,10 @@ static struct curl_slist *dd_agent_headers_alloc(void) {
771771
dd_append_header(&list, "Datadog-Meta-Lang-Interpreter", sapi_module.name, strlen(sapi_module.name));
772772
dd_append_header(&list, "Datadog-Meta-Lang-Version", php_version_rt.ptr, php_version_rt.len);
773773
dd_append_header(&list, "Datadog-Meta-Tracer-Version", ZEND_STRL(PHP_DDTRACE_VERSION));
774-
if (!get_global_DD_APM_TRACING_ENABLED() || (ddtrace_sidecar && get_global_DD_TRACE_STATS_COMPUTATION_ENABLED())) {
774+
if (!get_global_DD_APM_TRACING_ENABLED() || (ddtrace_sidecar_for_signal && get_global_DD_TRACE_STATS_COMPUTATION_ENABLED())) {
775775
dd_append_header(&list, "Datadog-Client-Computed-Stats", ZEND_STRL("true"));
776776
}
777-
if (ddtrace_sidecar && get_global_DD_TRACE_STATS_COMPUTATION_ENABLED()) {
777+
if (ddtrace_sidecar_for_signal && get_global_DD_TRACE_STATS_COMPUTATION_ENABLED()) {
778778
dd_append_header(&list, "Datadog-Client-Computed-Top-Level", ZEND_STRL("true"));
779779
}
780780

ext/serializer.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,7 +1362,7 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo
13621362

13631363
// Trace-level filter: when stats computation is enabled, drop the span from the
13641364
// entire pipeline (trace sending + stats) if its trace is filtered.
1365-
if (ddtrace_sidecar && get_DD_TRACE_STATS_COMPUTATION_ENABLED()) {
1365+
if (DDTRACE_G(sidecar) && get_DD_TRACE_STATS_COMPUTATION_ENABLED()) {
13661366
if (DDTRACE_G(agent_info_reader)) {
13671367
ddog_apply_agent_info_concentrator_config(DDTRACE_G(agent_info_reader));
13681368
}
@@ -1534,7 +1534,7 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo
15341534
ZEND_HASH_FOREACH_END();
15351535

15361536

1537-
if (!span_sampling_applied && ddtrace_sidecar && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && ddog_is_agent_info_ready()) {
1537+
if (!span_sampling_applied && DDTRACE_G(sidecar) && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && ddog_is_agent_info_ready()) {
15381538
if (inferred_span) {
15391539
// Inferred span won't be serialized, so feed it to the concentrator here.
15401540
ddtrace_span_precomputed inferred_pre;
@@ -1779,7 +1779,7 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo
17791779
zend_string_release(Z_STR(prop_root_service_as_string));
17801780
zend_string_release(Z_STR(prop_service_as_string));
17811781

1782-
if (ddtrace_sidecar && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && ddog_is_agent_info_ready()) {
1782+
if (DDTRACE_G(sidecar) && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && ddog_is_agent_info_ready()) {
17831783
ddtrace_feed_span_to_concentrator(span, &pre);
17841784
}
17851785

@@ -1804,7 +1804,7 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo
18041804
}
18051805
}
18061806

1807-
if (ddtrace_sidecar && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && !is_inferred_span) {
1807+
if (DDTRACE_G(sidecar) && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && !is_inferred_span) {
18081808
bool is_top_level_span = !span->parent;
18091809
if (span->parent) {
18101810
zval *parent_service = &SPANDATA(span->parent)->property_service;

ext/sidecar.c

Lines changed: 18 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;
@@ -408,9 +408,8 @@ void ddtrace_sidecar_setup(bool appsec_activation, bool appsec_config) {
408408
}
409409

410410
// Record the first established connection for best-effort signal-handler use.
411-
if (DDTRACE_G(sidecar)) {
412-
ddog_SidecarTransport *expected = NULL;
413-
atomic_compare_exchange_strong(&ddtrace_sidecar_for_signal, &expected, DDTRACE_G(sidecar));
411+
if (DDTRACE_G(sidecar) && !ddtrace_sidecar_for_signal) {
412+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
414413
}
415414
}
416415

@@ -447,7 +446,7 @@ void ddtrace_sidecar_handle_fork(void) {
447446
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));
448447
DDTRACE_G(sidecar) = NULL;
449448
}
450-
atomic_store(&ddtrace_sidecar_for_signal, NULL);
449+
ddtrace_sidecar_for_signal = NULL;
451450

452451
if (ddtrace_sidecar_active_mode == DD_SIDECAR_CONNECTION_THREAD) {
453452
ddtrace_ffi_try("Failed clearing inherited listener state",
@@ -488,7 +487,7 @@ void ddtrace_sidecar_handle_fork(void) {
488487
}
489488

490489
if (DDTRACE_G(sidecar)) {
491-
atomic_store(&ddtrace_sidecar_for_signal, DDTRACE_G(sidecar));
490+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
492491
}
493492
#endif
494493
}
@@ -500,9 +499,8 @@ void ddtrace_sidecar_ensure_active(void) {
500499
// First RINIT on this thread: the process-level setup already ran (endpoint is
501500
// set), so establish this thread's own connection now.
502501
DDTRACE_G(sidecar) = ddtrace_sidecar_connect(false);
503-
if (DDTRACE_G(sidecar)) {
504-
ddog_SidecarTransport *expected = NULL;
505-
atomic_compare_exchange_strong(&ddtrace_sidecar_for_signal, &expected, DDTRACE_G(sidecar));
502+
if (DDTRACE_G(sidecar) && !ddtrace_sidecar_for_signal) {
503+
ddtrace_sidecar_for_signal = DDTRACE_G(sidecar);
506504
}
507505
}
508506
}
@@ -528,6 +526,8 @@ void ddtrace_sidecar_finalize(bool clear_id) {
528526
}
529527

530528
void ddtrace_sidecar_shutdown(void) {
529+
ddtrace_sidecar_for_signal = NULL;
530+
531531
// In thread mode, drop the main thread's connection before shutting down the
532532
// listener to avoid deadlock. GSHUTDOWN owns transport cleanup for all other
533533
// threads; the main thread's GSHUTDOWN runs after MSHUTDOWN on some SAPIs,
@@ -542,6 +542,8 @@ void ddtrace_sidecar_shutdown(void) {
542542
current_pid == ddtrace_sidecar_master_pid) {
543543

544544
if (DDTRACE_G(sidecar)) {
545+
ddtrace_sidecar_for_signal = NULL;
546+
545547
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));
546548
DDTRACE_G(sidecar) = NULL;
547549
}
@@ -556,8 +558,6 @@ void ddtrace_sidecar_shutdown(void) {
556558
ddtrace_sidecar_instance_id = NULL;
557559
}
558560

559-
atomic_store(&ddtrace_sidecar_for_signal, NULL);
560-
561561
if (ddtrace_endpoint) {
562562
dd_free_endpoints();
563563
}
@@ -875,6 +875,10 @@ void ddtrace_sidecar_rshutdown(void) {
875875

876876
void ddtrace_sidecar_gshutdown(void) {
877877
if (DDTRACE_G(sidecar)) {
878+
if (DDTRACE_G(sidecar) == ddtrace_sidecar_for_signal) {
879+
ddtrace_sidecar_for_signal = NULL;
880+
}
881+
878882
// Drain any accumulated background-sender metrics before the transport goes away.
879883
ddtrace_telemetry_flush_bgs_metrics_final();
880884
ddog_sidecar_transport_drop(DDTRACE_G(sidecar));

ext/sidecar.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ typedef enum {
1414
DD_SIDECAR_CONNECTION_THREAD = 2
1515
} dd_sidecar_active_mode_t;
1616

17-
// ddtrace_sidecar lives in DDTRACE_G(sidecar) — one transport per thread.
1817
// ddtrace_sidecar_instance_id is a process global — one identity per PHP process.
1918
extern struct ddog_InstanceId *ddtrace_sidecar_instance_id;
2019
// Best-effort pointer used only by the signal handler (SIGTERM/SIGINT), which cannot call
2120
// TSRMLS_FETCH() safely. Set to the first thread's connection; never cleared until MSHUTDOWN.
22-
extern _Atomic(ddog_SidecarTransport *) ddtrace_sidecar_for_signal;
21+
// Not atomic: concurrent shutdown is a pre-existing best-effort race for signal handlers.
22+
extern ddog_SidecarTransport *ddtrace_sidecar_for_signal;
2323
extern ddog_Endpoint *ddtrace_endpoint;
2424
extern dd_sidecar_active_mode_t ddtrace_sidecar_active_mode;
2525
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) {

ext/span_stats.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ void ddtrace_feed_span_to_concentrator(ddtrace_span_data *span, const ddtrace_sp
397397
ddtrace_concentrator_cb_data data = { .span = span, .pre = pre, .needs_ipc = false };
398398
ddog_span_concentrator_with(env_slice, version_slice, service_slice, ddtrace_span_concentrator_feed_cb, &data);
399399

400-
if (data.needs_ipc && ddtrace_sidecar) {
401-
ddog_sidecar_add_php_span_to_concentrator(&ddtrace_sidecar, env_slice, version_slice, &data.ipc_stats);
400+
if (data.needs_ipc && DDTRACE_G(sidecar)) {
401+
ddog_sidecar_add_php_span_to_concentrator(&DDTRACE_G(sidecar), env_slice, version_slice, &data.ipc_stats);
402402
}
403403
}

0 commit comments

Comments
 (0)