Skip to content

Commit 34852e4

Browse files
authored
fix: handle Apache graceful restarts more accurately (#2483)
* fix(profiling): handle Apache graceful restarts more accurately * fix(tracing): reset pthread_once controls
1 parent c9757cc commit 34852e4

File tree

4 files changed

+37
-8
lines changed

4 files changed

+37
-8
lines changed

ext/ddtrace.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,9 @@ static PHP_MINIT_FUNCTION(ddtrace) {
981981
atexit(dd_clean_main_thread_locals);
982982
#endif
983983

984+
// Reset on every minit for `apachectl graceful`.
985+
dd_activate_once_control = PTHREAD_ONCE_INIT;
986+
984987
zai_hook_minit();
985988
zai_uhook_minit(module_number);
986989
#if PHP_VERSION_ID >= 80000
@@ -1149,7 +1152,7 @@ static void dd_initialize_request(void) {
11491152
zend_hash_init(&DDTRACE_G(propagated_root_span_tags), 8, unused, ZVAL_PTR_DTOR, 0);
11501153
zend_hash_init(&DDTRACE_G(tracestate_unknown_dd_keys), 8, unused, ZVAL_PTR_DTOR, 0);
11511154

1152-
// Things that should only run on the first RINIT
1155+
// Things that should only run on the first RINIT after each minit.
11531156
pthread_once(&dd_rinit_once_control, dd_rinit_once);
11541157

11551158
if (!DDTRACE_G(remote_config_reader)) {

ext/handlers_curl.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ static HashTable *ddtrace_curl_multi_get_gc(zend_object *object, zval **table, i
176176
}
177177

178178
static pthread_once_t dd_replace_curl_get_gc_once = PTHREAD_ONCE_INIT;
179-
static zend_object_handlers *dd_curl_object_handlers;
179+
ZEND_TLS zend_object_handlers *dd_curl_object_handlers = NULL;
180180
static void dd_replace_curl_get_gc(void) {
181181
dd_curl_multi_get_gc = dd_curl_object_handlers->get_gc;
182182
dd_curl_object_handlers->get_gc = ddtrace_curl_multi_get_gc;
@@ -406,6 +406,10 @@ void ddtrace_curl_handlers_startup(void) {
406406
for (size_t i = 0; i < handlers_len; ++i) {
407407
datadog_php_install_handler(handlers[i]);
408408
}
409+
410+
// Reset object state each MINIT/startup cycle for `apachectl graceful`.
411+
dd_curl_object_handlers = NULL;
412+
dd_replace_curl_get_gc_once = PTHREAD_ONCE_INIT;
409413
}
410414

411415
void ddtrace_curl_handlers_rinit(void) {

profiling/src/lib.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,17 @@ extern "C" fn minit(_type: c_int, module_number: c_int) -> ZendResult {
290290
#[cfg(feature = "exception_profiling")]
291291
exception::exception_profiling_minit();
292292

293+
// There are a few things which need to do something on the first rinit of
294+
// each minit/mshutdown cycle. In Apache, when doing `apachectl graceful`,
295+
// there can be more than one of these cycles per process.
296+
// Re-initializing these on each minit allows us to do it once per cycle.
297+
// This is unsafe generally, but all SAPIs are supposed to only have one
298+
// thread alive during minit, so it should be safe here specifically.
299+
unsafe {
300+
ZAI_CONFIG_ONCE = Once::new();
301+
RINIT_ONCE = Once::new();
302+
}
303+
293304
ZendResult::Success
294305
}
295306

@@ -374,15 +385,20 @@ extern "C" fn activate() {
374385
unsafe { profiling::activate_run_time_cache() };
375386
}
376387

388+
/// The mut here is *only* for resetting this back to uninitialized each minit.
389+
static mut ZAI_CONFIG_ONCE: Once = Once::new();
390+
/// The mut here is *only* for resetting this back to uninitialized each minit.
391+
static mut RINIT_ONCE: Once = Once::new();
392+
377393
/* If Failure is returned the VM will do a C exit; try hard to avoid that,
378394
* using it for catastrophic errors only.
379395
*/
380396
extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult {
381397
#[cfg(debug_assertions)]
382398
trace!("RINIT({_type}, {_module_number})");
383399

384-
static ONCE: Once = Once::new();
385-
ONCE.call_once(|| unsafe {
400+
// SAFETY: not being mutated during rinit.
401+
unsafe { &ZAI_CONFIG_ONCE }.call_once(|| unsafe {
386402
bindings::zai_config_first_time_rinit();
387403
config::first_rinit();
388404
});
@@ -420,8 +436,7 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult {
420436
// SAFETY: still safe to access in rinit after first_rinit.
421437
let system_settings = unsafe { system_settings.as_ref() };
422438

423-
static ONCE2: Once = Once::new();
424-
ONCE2.call_once(|| {
439+
unsafe { &RINIT_ONCE }.call_once(|| {
425440
if system_settings.profiling_enabled {
426441
/* Safety: sapi_module is initialized by rinit and shouldn't be
427442
* modified at this point (safe to read values).

profiling/src/logging.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,18 @@ pub fn log_init(level_filter: LevelFilter) {
1717
if fd != -1 {
1818
// Safety: the fd is a valid and open file descriptor, and the File has sole ownership.
1919
let target = Box::new(unsafe { File::from_raw_fd(fd) });
20-
env_logger::builder()
20+
let result = env_logger::builder()
2121
.filter_level(LevelFilter::Off)
2222
.filter_module("datadog_php_profiling", level_filter)
2323
.target(Target::Pipe(target))
2424
.format_timestamp_micros()
25-
.init();
25+
.try_init();
26+
27+
// Due to `apachectl graceful` doing multiple minit/mshutdowns in one
28+
// process, this may get called more than once. That's okay, set the
29+
// new log level.
30+
if result.is_err() {
31+
log::set_max_level(level_filter);
32+
}
2633
}
2734
}

0 commit comments

Comments
 (0)