Skip to content

Commit

Permalink
Ensure rust code doesn't prevent unloading of ddtrace
Browse files Browse the repository at this point in the history
Signed-off-by: Bob Weinand <[email protected]>
  • Loading branch information
bwoebi committed Feb 5, 2024
1 parent e2ddbee commit 6ae58e7
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
82 changes: 82 additions & 0 deletions ext/ddtrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -459,13 +459,90 @@ static PHP_GINIT_FUNCTION(ddtrace) {
zai_hook_ginit();
}

#if defined(COMPILE_DL_DDTRACE) && defined(__GLIBC__) && __GLIBC_MINOR__
#define CXA_THREAD_ATEXIT_WRAPPER 1
#endif
#ifdef CXA_THREAD_ATEXIT_WRAPPER
struct dd_rust_thread_destructor {
void (*dtor)(void *);
void *obj;
struct dd_rust_thread_destructor *next;
};
ZEND_TLS struct dd_rust_thread_destructor *dd_rust_thread_destructors = NULL;
ZEND_TLS bool dd_is_main_thread = false;

static void dd_run_rust_thread_destructors(void *unused) {
UNUSED(unused);
struct dd_rust_thread_destructor *entry = dd_rust_thread_destructors;
while (entry) {
struct dd_rust_thread_destructor *cur = entry;
cur->dtor(cur->obj);
entry = entry->next;
free(cur);
}
}
#endif

static PHP_GSHUTDOWN_FUNCTION(ddtrace) {
if (ddtrace_globals->remote_config_reader) {
ddog_agent_remote_config_reader_drop(ddtrace_globals->remote_config_reader);
}
zai_hook_gshutdown();

#ifdef CXA_THREAD_ATEXIT_WRAPPER
if (!dd_is_main_thread) {
dd_run_rust_thread_destructors(NULL);
}
#endif
}

// Rust code will call __cxa_thread_atexit_impl. This is a weak symbol; it's defined by glibc.
// The problem is that calls to __cxa_thread_atexit_impl cause shared libraries to remain referenced until the calling thread terminates.
// However in NTS builds the calling thread is the main thread and thus the shared object (i.e. ddtrace.so) WILL remain loaded.
// This is problematic with e.g. apache which, upon reloading will unload all PHP modules including PHP itself, then reload.
// This prevents us from a) having the weak symbols updated to the new locations and b) having ddtrace updates going live without hard restart.
// Thus, we need to intercept it: define it ourselves so that the linker will force the rust code to call our code here.
// Then we can collect the callbacks and invoke them ourselves right at thread shutdown, i.e. GSHUTDOWN.
#ifdef CXA_THREAD_ATEXIT_WRAPPER
#define CXA_THREAD_ATEXIT_PHP ((void *)0)
#define CXA_THREAD_ATEXIT_UNINITIALIZED ((void *)1)
#define CXA_THREAD_ATEXIT_UNAVAILABLE ((void *)2)

static int (*glibc__cxa_thread_atexit_impl)(void (*func)(void *), void *obj, void *dso_symbol) = CXA_THREAD_ATEXIT_UNINITIALIZED;
static pthread_key_t dd_cxa_thread_atexit_key; // fallback for sidecar

// Note: this symbol is not public
int __cxa_thread_atexit_impl(void (*func)(void *), void *obj, void *dso_symbol) {
if (glibc__cxa_thread_atexit_impl == CXA_THREAD_ATEXIT_UNINITIALIZED) {
glibc__cxa_thread_atexit_impl = DL_FETCH_SYMBOL(NULL, "__cxa_thread_atexit_impl");
if (glibc__cxa_thread_atexit_impl == NULL) {
// no race condition here: logging is initialized in MINIT, at which point only a single thread lives
glibc__cxa_thread_atexit_impl = CXA_THREAD_ATEXIT_UNAVAILABLE;
pthread_key_create(&dd_cxa_thread_atexit_key, dd_run_rust_thread_destructors);
}
}

if (glibc__cxa_thread_atexit_impl != CXA_THREAD_ATEXIT_PHP && glibc__cxa_thread_atexit_impl != CXA_THREAD_ATEXIT_UNAVAILABLE) {
return glibc__cxa_thread_atexit_impl(func, obj, dso_symbol);
}

if (glibc__cxa_thread_atexit_impl == CXA_THREAD_ATEXIT_UNAVAILABLE) {
pthread_setspecific(dd_cxa_thread_atexit_key, (void *)0x1); // needs to be non-NULL
}

struct dd_rust_thread_destructor *entry = malloc(sizeof(struct dd_rust_thread_destructor));
entry->dtor = func;
entry->obj = obj;
entry->next = dd_rust_thread_destructors;
dd_rust_thread_destructors = entry;
return 0;
}

static void dd_clean_main_thread_locals() {
dd_run_rust_thread_destructors(NULL);
}
#endif

/* DDTrace\SpanLink */
zend_class_entry *ddtrace_ce_span_link;

Expand Down Expand Up @@ -898,6 +975,11 @@ static void dd_disable_if_incompatible_sapi_detected(void) {
static PHP_MINIT_FUNCTION(ddtrace) {
UNUSED(type);

#ifdef CXA_THREAD_ATEXIT_WRAPPER
dd_is_main_thread = true;
glibc__cxa_thread_atexit_impl = CXA_THREAD_ATEXIT_PHP;
atexit(dd_clean_main_thread_locals);
#endif

zai_hook_minit();
zai_uhook_minit(module_number);
Expand Down
5 changes: 4 additions & 1 deletion tests/ext/dd_trace_log_file.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ datadog.trace.generate_root_span=0
--FILE--
<?php

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

?>
--EXPECTF--
Expand Down

0 comments on commit 6ae58e7

Please sign in to comment.