Skip to content

Commit 9a595b2

Browse files
committed
Ensure rust code doesn't prevent unloading of ddtrace
Signed-off-by: Bob Weinand <[email protected]>
1 parent e2ddbee commit 9a595b2

File tree

2 files changed

+85
-1
lines changed

2 files changed

+85
-1
lines changed

ext/ddtrace.c

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,13 +459,89 @@ static PHP_GINIT_FUNCTION(ddtrace) {
459459
zai_hook_ginit();
460460
}
461461

462+
#if defined(COMPILE_DL_DDTRACE) && defined(__GLIBC__) && __GLIBC_MINOR__
463+
#define CXA_THREAD_ATEXIT_WRAPPER 1
464+
#endif
465+
#ifdef CXA_THREAD_ATEXIT_WRAPPER
466+
struct dd_rust_thread_destructor {
467+
void (*dtor)(void *);
468+
void *obj;
469+
struct dd_rust_thread_destructor *next;
470+
};
471+
ZEND_TLS struct dd_rust_thread_destructor *dd_rust_thread_destructors = NULL;
472+
ZEND_TLS bool dd_is_main_thread = false;
473+
474+
void dd_run_rust_thread_destructors(void *unused) {
475+
UNUSED(unused);
476+
struct dd_rust_thread_destructor *entry = dd_rust_thread_destructors;
477+
while (entry) {
478+
struct dd_rust_thread_destructor *cur = entry;
479+
cur->dtor(cur->obj);
480+
entry = entry->next;
481+
free(cur);
482+
}
483+
}
484+
#endif
485+
462486
static PHP_GSHUTDOWN_FUNCTION(ddtrace) {
463487
if (ddtrace_globals->remote_config_reader) {
464488
ddog_agent_remote_config_reader_drop(ddtrace_globals->remote_config_reader);
465489
}
466490
zai_hook_gshutdown();
491+
492+
#ifdef CXA_THREAD_ATEXIT_WRAPPER
493+
if (!dd_is_main_thread) {
494+
dd_run_rust_thread_destructors(NULL);
495+
}
496+
#endif
467497
}
468498

499+
// Rust code will call __cxa_thread_atexit_impl. This is a weak symbol; it's defined by glibc.
500+
// The problem is that calls to __cxa_thread_atexit_impl cause shared libraries to remain referenced until the calling thread terminates.
501+
// However in NTS builds the calling thread is the main thread and thus the shared object (i.e. ddtrace.so) WILL remain loaded.
502+
// This is problematic with e.g. apache which, upon reloading will unload all PHP modules including PHP itself, then reload.
503+
// This prevents us from a) having the weak symbols updated to the new locations and b) having ddtrace updates going live without hard restart.
504+
// Thus, we need to intercept it: define it ourselves so that the linker will force the rust code to call our code here.
505+
// Then we can collect the callbacks and invoke them ourselves right at thread shutdown, i.e. GSHUTDOWN.
506+
#ifdef CXA_THREAD_ATEXIT_WRAPPER
507+
#define CXA_THREAD_ATEXIT_PHP ((void *)0)
508+
#define CXA_THREAD_ATEXIT_UNINITIALIZED ((void *)1)
509+
#define CXA_THREAD_ATEXIT_UNAVAILABLE ((void *)2)
510+
511+
static int (*glibc__cxa_thread_atexit_impl)(void (*func)(void *), void *obj, void *dso_symbol) = CXA_THREAD_ATEXIT_UNINITIALIZED;
512+
static pthread_key_t dd_cxa_thread_atexit_key; // fallback for sidecar
513+
514+
// Note: this symbol is not public
515+
int __cxa_thread_atexit_impl(void (*func)(void *), void *obj, void *dso_symbol) {
516+
while (glibc__cxa_thread_atexit_impl != CXA_THREAD_ATEXIT_PHP && glibc__cxa_thread_atexit_impl != CXA_THREAD_ATEXIT_UNAVAILABLE) {
517+
if (glibc__cxa_thread_atexit_impl == CXA_THREAD_ATEXIT_UNINITIALIZED) {
518+
glibc__cxa_thread_atexit_impl = DL_FETCH_SYMBOL(NULL, "__cxa_thread_atexit_impl");
519+
if (glibc__cxa_thread_atexit_impl == NULL) {
520+
glibc__cxa_thread_atexit_impl = CXA_THREAD_ATEXIT_UNAVAILABLE;
521+
pthread_key_create(&dd_cxa_thread_atexit_key, dd_run_rust_thread_destructors);
522+
break;
523+
}
524+
}
525+
return glibc__cxa_thread_atexit_impl(func, obj, dso_symbol);
526+
}
527+
528+
if (glibc__cxa_thread_atexit_impl == CXA_THREAD_ATEXIT_UNAVAILABLE) {
529+
pthread_setspecific(dd_cxa_thread_atexit_key, (void *)0x1); // needs to be non-NULL
530+
}
531+
532+
struct dd_rust_thread_destructor *entry = malloc(sizeof(struct dd_rust_thread_destructor));
533+
entry->dtor = func;
534+
entry->obj = obj;
535+
entry->next = dd_rust_thread_destructors;
536+
dd_rust_thread_destructors = entry;
537+
return 0;
538+
}
539+
540+
static void dd_clean_main_thread_locals() {
541+
dd_run_rust_thread_destructors(NULL);
542+
}
543+
#endif
544+
469545
/* DDTrace\SpanLink */
470546
zend_class_entry *ddtrace_ce_span_link;
471547

@@ -898,6 +974,11 @@ static void dd_disable_if_incompatible_sapi_detected(void) {
898974
static PHP_MINIT_FUNCTION(ddtrace) {
899975
UNUSED(type);
900976

977+
#ifdef CXA_THREAD_ATEXIT_WRAPPER
978+
dd_is_main_thread = true;
979+
glibc__cxa_thread_atexit_impl = CXA_THREAD_ATEXIT_PHP;
980+
atexit(dd_clean_main_thread_locals);
981+
#endif
901982

902983
zai_hook_minit();
903984
zai_uhook_minit(module_number);

tests/ext/dd_trace_log_file.phpt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ datadog.trace.generate_root_span=0
1111
--FILE--
1212
<?php
1313

14-
readfile(__DIR__ . "/dd_trace_log_file.log");
14+
// Prevent side-effects from other tests (sidecar), hence filter it
15+
$log = file_get_contents(__DIR__ . "/dd_trace_log_file.log");
16+
preg_match("(.*\[span\].*)", $log, $m);
17+
echo $m[0];
1518

1619
?>
1720
--EXPECTF--

0 commit comments

Comments
 (0)