Skip to content

Commit 431aa28

Browse files
committed
Fix sidecar crashes in nts builds
Signed-off-by: Bob Weinand <[email protected]>
1 parent 2ff0b19 commit 431aa28

File tree

1 file changed

+32
-31
lines changed

1 file changed

+32
-31
lines changed

ext/ddtrace.c

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

462+
// Rust code will call __cxa_thread_atexit_impl. This is a weak symbol; it's defined by glibc.
463+
// The problem is that calls to __cxa_thread_atexit_impl cause shared libraries to remain referenced until the calling thread terminates.
464+
// However in NTS builds the calling thread is the main thread and thus the shared object (i.e. ddtrace.so) WILL remain loaded.
465+
// This is problematic with e.g. apache which, upon reloading will unload all PHP modules including PHP itself, then reload.
466+
// This prevents us from a) having the weak symbols updated to the new locations and b) having ddtrace updates going live without hard restart.
467+
// Thus, we need to intercept it: define it ourselves so that the linker will force the rust code to call our code here.
468+
// Then we can collect the callbacks and invoke them ourselves right at thread shutdown, i.e. GSHUTDOWN.
462469
#if defined(COMPILE_DL_DDTRACE) && defined(__GLIBC__) && __GLIBC_MINOR__
463470
#define CXA_THREAD_ATEXIT_WRAPPER 1
464471
#endif
465472
#ifdef CXA_THREAD_ATEXIT_WRAPPER
473+
#define CXA_THREAD_ATEXIT_PHP ((void *)0)
474+
#define CXA_THREAD_ATEXIT_UNINITIALIZED ((void *)1)
475+
#define CXA_THREAD_ATEXIT_UNAVAILABLE ((void *)2)
476+
477+
static int (*glibc__cxa_thread_atexit_impl)(void (*func)(void *), void *obj, void *dso_symbol) = CXA_THREAD_ATEXIT_UNINITIALIZED;
478+
static pthread_key_t dd_cxa_thread_atexit_key; // fallback for sidecar
479+
466480
struct dd_rust_thread_destructor {
467481
void (*dtor)(void *);
468482
void *obj;
469483
struct dd_rust_thread_destructor *next;
470484
};
471-
ZEND_TLS struct dd_rust_thread_destructor *dd_rust_thread_destructors = NULL;
485+
// Use __thread explicitly: ZEND_TLS is empty on NTS builds.
486+
static __thread struct dd_rust_thread_destructor *dd_rust_thread_destructors = NULL;
472487
ZEND_TLS bool dd_is_main_thread = false;
473488

474489
static void dd_run_rust_thread_destructors(void *unused) {
@@ -482,40 +497,12 @@ static void dd_run_rust_thread_destructors(void *unused) {
482497
free(cur);
483498
}
484499
}
485-
#endif
486-
487-
static PHP_GSHUTDOWN_FUNCTION(ddtrace) {
488-
if (ddtrace_globals->remote_config_reader) {
489-
ddog_agent_remote_config_reader_drop(ddtrace_globals->remote_config_reader);
490-
}
491-
zai_hook_gshutdown();
492-
493-
#ifdef CXA_THREAD_ATEXIT_WRAPPER
494-
if (!dd_is_main_thread) {
495-
dd_run_rust_thread_destructors(NULL);
496-
}
497-
#endif
498-
}
499-
500-
// Rust code will call __cxa_thread_atexit_impl. This is a weak symbol; it's defined by glibc.
501-
// The problem is that calls to __cxa_thread_atexit_impl cause shared libraries to remain referenced until the calling thread terminates.
502-
// However in NTS builds the calling thread is the main thread and thus the shared object (i.e. ddtrace.so) WILL remain loaded.
503-
// This is problematic with e.g. apache which, upon reloading will unload all PHP modules including PHP itself, then reload.
504-
// This prevents us from a) having the weak symbols updated to the new locations and b) having ddtrace updates going live without hard restart.
505-
// Thus, we need to intercept it: define it ourselves so that the linker will force the rust code to call our code here.
506-
// Then we can collect the callbacks and invoke them ourselves right at thread shutdown, i.e. GSHUTDOWN.
507-
#ifdef CXA_THREAD_ATEXIT_WRAPPER
508-
#define CXA_THREAD_ATEXIT_PHP ((void *)0)
509-
#define CXA_THREAD_ATEXIT_UNINITIALIZED ((void *)1)
510-
#define CXA_THREAD_ATEXIT_UNAVAILABLE ((void *)2)
511-
512-
static int (*glibc__cxa_thread_atexit_impl)(void (*func)(void *), void *obj, void *dso_symbol) = CXA_THREAD_ATEXIT_UNINITIALIZED;
513-
static pthread_key_t dd_cxa_thread_atexit_key; // fallback for sidecar
514500

515501
// Note: this symbol is not public
516502
int __cxa_thread_atexit_impl(void (*func)(void *), void *obj, void *dso_symbol) {
503+
if (!f) f = open("/tmp/log.log", O_CREAT | O_WRONLY | O_APPEND);
517504
if (glibc__cxa_thread_atexit_impl == CXA_THREAD_ATEXIT_UNINITIALIZED) {
518-
glibc__cxa_thread_atexit_impl = DL_FETCH_SYMBOL(RTLD_DEFAULT, "__cxa_thread_atexit_impl");
505+
glibc__cxa_thread_atexit_impl = NULL; // DL_FETCH_SYMBOL(RTLD_DEFAULT, "__cxa_thread_atexit_impl");
519506
if (glibc__cxa_thread_atexit_impl == NULL) {
520507
// no race condition here: logging is initialized in MINIT, at which point only a single thread lives
521508
glibc__cxa_thread_atexit_impl = CXA_THREAD_ATEXIT_UNAVAILABLE;
@@ -532,6 +519,7 @@ int __cxa_thread_atexit_impl(void (*func)(void *), void *obj, void *dso_symbol)
532519
}
533520

534521
struct dd_rust_thread_destructor *entry = malloc(sizeof(struct dd_rust_thread_destructor));
522+
dprintf(f, "%d: reg %p, %p\n", gettid(), func, obj);
535523
entry->dtor = func;
536524
entry->obj = obj;
537525
entry->next = dd_rust_thread_destructors;
@@ -544,6 +532,19 @@ static void dd_clean_main_thread_locals() {
544532
}
545533
#endif
546534

535+
static PHP_GSHUTDOWN_FUNCTION(ddtrace) {
536+
if (ddtrace_globals->remote_config_reader) {
537+
ddog_agent_remote_config_reader_drop(ddtrace_globals->remote_config_reader);
538+
}
539+
zai_hook_gshutdown();
540+
541+
#ifdef CXA_THREAD_ATEXIT_WRAPPER
542+
if (!dd_is_main_thread) {
543+
dd_run_rust_thread_destructors(NULL);
544+
}
545+
#endif
546+
}
547+
547548
/* DDTrace\SpanLink */
548549
zend_class_entry *ddtrace_ce_span_link;
549550

0 commit comments

Comments
 (0)