refactor: address review findings on thread-context factory split

Restore ProfiledThread factory registration in gtest binaries, null-guard
nullable currentProfiled() in javaApi.cpp, re-read profiler-presence
lazily instead of latching it, fix support/profiler LOADED-state tracking
in LibraryLoader, resolve Dictionary encoding before the detach window,
and fail closed consistently on Dictionary/attrs_data overflow.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: jbachorik

.github/workflows/test_workflow.yml

@@ -148,6 +148,10 @@ jobs:
             exit 1
           fi
       - name: Verify libJavaSupport ABI
+        # NOTE: this ABI gate only runs on the glibc/musl Linux matrix legs
+        # (nm -D against libJavaSupport.so). There is no equivalent check for
+        # macOS builds — the otel_thread_ctx_v1 export guarantee documented
+        # below is verified on Linux only.
         if: success()
         run: |
           SUPPORT_LIB=$(find ddprof-lib/build/lib -name "libJavaSupport.so" | head -1)
@@ -158,7 +162,15 @@ jobs:
             echo "$PROFILER_UNDEFINED"
             exit 1
           fi
-          echo "ABI gate passed — no profiler symbols in libJavaSupport.so"
+          # ContextExtractionToSupportPlan Phase E.4: otel_thread_ctx_v1 must be exported
+          # from libJavaSupport.so so external profilers can discover the OTEP thread
+          # context record with only the support library loaded (no profiler).
+          if ! nm -D "$SUPPORT_LIB" | grep -q 'otel_thread_ctx_v1'; then
+            echo "ERROR: libJavaSupport.so does not export otel_thread_ctx_v1"
+            nm -D "$SUPPORT_LIB" | grep -i otel || true
+            exit 1
+          fi
+          echo "ABI gate passed — no profiler symbols in libJavaSupport.so, otel_thread_ctx_v1 exported"
       - name: Generate Unwinding Report
         if: success() && matrix.config == 'debug'
         run: |

ddprof-lib/src/main/cpp/javaApi.cpp

@@ -137,7 +137,9 @@ Java_com_datadoghq_profiler_JavaProfiler_getSamples(JNIEnv *env,
 extern "C" DLLEXPORT void JNICALL
 JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0() {
   ProfiledThread *current = ProfiledThread::currentProfiled();
-  assert(current != nullptr);
+  if (current == nullptr) {
+    return;
+  }
   int tid = current->tid();
   if (unlikely(tid < 0)) {
     return;
@@ -167,7 +169,9 @@ JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadAdd0() {
 extern "C" DLLEXPORT void JNICALL
 JavaCritical_com_datadoghq_profiler_JavaProfiler_filterThreadRemove0() {
   ProfiledThread *current = ProfiledThread::currentProfiled();
-  assert(current != nullptr);
+  if (current == nullptr) {
+    return;
+  }
   int tid = current->tid();
   if (unlikely(tid < 0)) {
     return;

ddprof-lib/src/main/cpp/support/otel_context.h

@@ -75,7 +75,7 @@ DLLEXPORT extern thread_local OtelThreadContextRecord* otel_thread_ctx_v1;
  *
  * Signal safety: signal handlers must never access
  * otel_thread_ctx_v1 directly (TLS lazy init can deadlock
- * in musl). Instead they read via ProfiledThread::getOtelContextRecord().
+ * in musl). Instead they read via ThreadContext::getOtelContextRecord().
  */
 
 #endif /* _OTEL_CONTEXT_H */

…of-lib/src/main/cpp/otel_process_ctx.cpp …rc/main/cpp/support/otel_process_ctx.cppddprof-lib/src/main/cpp/otel_process_ctx.cpp renamed to ddprof-lib/src/main/cpp/support/otel_process_ctx.cpp ddprof-lib/src/main/cpp/otel_process_ctx.cpp renamed to ddprof-lib/src/main/cpp/support/otel_process_ctx.cpp

@@ -13,6 +13,10 @@ bool ThreadContext::_tls_key_initialized = false;
 static ThreadContext* defaultFactory(int tid) { return new ThreadContext(tid); }
 std::atomic<ThreadContextFactory> g_thread_context_factory{defaultFactory};
 
+void resetThreadContextFactory() {
+  g_thread_context_factory.store(defaultFactory, std::memory_order_release);
+}
+
 void ThreadContext::initTLSKey() {
   static pthread_once_t tls_initialized = PTHREAD_ONCE_INIT;
   pthread_once(&tls_initialized, doInitTLSKey);

…prof-lib/src/main/cpp/otel_process_ctx.h …/src/main/cpp/support/otel_process_ctx.hddprof-lib/src/main/cpp/otel_process_ctx.h renamed to ddprof-lib/src/main/cpp/support/otel_process_ctx.h ddprof-lib/src/main/cpp/otel_process_ctx.h renamed to ddprof-lib/src/main/cpp/support/otel_process_ctx.h

@@ -76,6 +76,20 @@ class ThreadContext : public ThreadLocalData {
   }
   // Deletes a ThreadContext returned by clearCurrentThreadTLS().
   static void deleteForTest(ThreadContext *tls) { delete tls; }
+
+  // Writes span_id/root_span_id directly on the base object and marks the
+  // record valid, without requiring a ProfiledThread or the JNI put() path.
+  // Exercises the base-object write path for the defensive-mode test
+  // (ContextExtractionToSupportPlan Phase E.5): writing context via a plain
+  // ThreadContext before any profiler factory is registered.
+  inline void setContextForTest(u64 span_id, u64 root_span_id) {
+    OtelThreadContextRecord *record = getOtelContextRecord();
+    for (int i = 0; i < 8; i++) {
+      record->span_id[i] = (uint8_t)(span_id >> (8 * (7 - i)));
+    }
+    _otel_local_root_span_id = root_span_id;
+    __atomic_store_n(&record->valid, (uint8_t)1, __ATOMIC_RELEASE);
+  }
 #endif
 
   inline int tid() { return _tid; }
@@ -113,4 +127,9 @@ class ThreadContext : public ThreadLocalData {
 typedef ThreadContext* (*ThreadContextFactory)(int tid);
 extern std::atomic<ThreadContextFactory> g_thread_context_factory;
 
+// Restores g_thread_context_factory to the support-only default (plain
+// ThreadContext), undoing a profiler-installed factory. Called at profiler
+// teardown so a subsequent re-init starts from a clean slate.
+void resetThreadContextFactory();
+
 #endif // _SUPPORT_THREAD_CONTEXT_H

ddprof-lib/src/main/cpp/support/threadContext.cpp

@@ -24,6 +24,21 @@ Java_com_datadoghq_profiler_JVMAccess_findIntJVMFlag0
 Java_com_datadoghq_profiler_JVMAccess_findFloatJVMFlag0
 Java_com_datadoghq_profiler_JVMAccess_healthCheck0
 
+# OTEP #4947 TLS discovery pointer (support/otel_context.cpp) — external profilers
+# locate this via ELF dynsym; unmangled global-scope C++ variable (no extern "C" needed).
+# Trailing '*' (fnmatch-style, supported by both the GNU ld version script and the
+# macOS export-list generator) matches only this one symbol on Linux. On Mach-O the
+# raw TLS variable is always kept local by the ABI (only the compiler-generated
+# _ZTW... "TLV wrapper" accessor is ever external) — an exact-name entry would make
+# ld64 hard-fail with "symbol(s) not found", so the wildcard form degrades to a
+# harmless no-op match there instead. macOS is dev-degraded (§2.1); Linux is ship.
+otel_thread_ctx_v1*
+
+# Context JNI entry points (support/contextApi.cpp; support-only, no profiler dependency)
+Java_com_datadoghq_profiler_ContextStorage_initializeContextTLS0
+Java_com_datadoghq_profiler_OTelContext_setProcessCtx0
+Java_com_datadoghq_profiler_OTelContext_readProcessCtx0
+
 # Crash-protection probe (C linkage + helpers)
 g_crash_protection_probe
 _Z25crashProtectionProbeResetv

ddprof-lib/src/main/cpp/support/threadContext.h

@@ -11,23 +11,6 @@
 #include <cstring>
 #include <time.h>
 
-// The shared ThreadContext TLS slot defaults to producing plain ThreadContext
-// instances (support-only build). This translation unit is only linked into
-// the full profiler build, so it commits the factory to produce ProfiledThread
-// instead, at library-load time — before any thread (including the one that
-// calls Profiler::start()) can observe the TLS slot and lazily allocate a
-// plain ThreadContext.
-static ThreadContext *newProfiledThread(int tid) { return ProfiledThread::forTid(tid); }
-
-namespace {
-struct ProfiledThreadFactoryInstaller {
-  ProfiledThreadFactoryInstaller() {
-    g_thread_context_factory.store(newProfiledThread, std::memory_order_release);
-  }
-};
-static ProfiledThreadFactoryInstaller profiled_thread_factory_installer;
-} // namespace
-
 void ProfiledThread::initCurrentThread() {
   // JVMTI callback path - does NOT use buffer
   // Allocate dedicated ProfiledThread for Java threads (not from buffer)

ddprof-lib/src/main/cpp/support/vmstructs-abi.symbols

@@ -462,6 +462,12 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) {
       },
       std::memory_order_release);
 
+  // Profiler is present: per-thread OTEL context storage must be backed by
+  // the full ProfiledThread record, not the support-only ThreadContext base.
+  g_thread_context_factory.store(
+      [](int tid) -> ThreadContext* { return ProfiledThread::forTid(tid); },
+      std::memory_order_release);
+
   CodeCache *lib = openJvmLibrary();
   if (lib == nullptr) {
     return false;
@@ -754,4 +760,5 @@ extern "C" DLLEXPORT void JNICALL JNI_OnUnload(JavaVM *vm, void *reserved) {
   crashProtectionProbeReset();
   VMThread::resetIsJavaThreadProbe();
   resetIsInSignalProbe();
+  resetThreadContextFactory();
 }