From 7583b8dffa104f66fb3789e8682f0dee5b1c7249 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 30 Apr 2026 15:35:50 +0200 Subject: [PATCH 1/2] fix: guard abi::__forced_unwind catch behind __GLIBCXX__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit abi::__forced_unwind is declared by libstdc++ (any libc) but not by libc++, so the catch only compiles where libstdc++ is the active C++ stdlib. Guard on __GLIBCXX__ — defined by libstdc++ regardless of libc, undefined under libc++ — so the file compiles on macOS clang+libc++ (and any libc++ build) while keeping the cancellation cleanup wherever libstdc++ is in use. The exception itself is raised by glibc's pthread_cancel; on platforms with a different cancellation mechanism (musl, macOS, AIX) the catch is harmless dead code but compiles cleanly under libstdc++. Co-Authored-By: Claude Opus 4.7 (1M context) --- ddprof-lib/src/main/cpp/j9/j9WallClock.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp b/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp index c965542f8..130f6a8ac 100644 --- a/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp +++ b/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp @@ -75,7 +75,16 @@ void J9WallClock::timerLoop() { // in the typical case. JNIEnv *jni = nullptr; ASGCT_CallFrame *frames = nullptr; + // abi::__forced_unwind is declared by libstdc++ (any libc) but not by libc++, + // so the catch only compiles where libstdc++ is the active C++ stdlib. Guard + // on __GLIBCXX__: defined by libstdc++ regardless of libc (glibc, musl, etc.), + // undefined under libc++ (macOS clang default, some Alpine/clang builds). + // The exception itself is raised by glibc's pthread_cancel — on platforms with + // a different cancellation mechanism (musl, macOS) the catch is harmless dead + // code but compiles cleanly under libstdc++. +#if defined(__GLIBCXX__) try { +#endif jni = VM::attachThread("java-profiler Sampler"); jvmtiEnv *jvmti = VM::jvmti(); @@ -138,11 +147,13 @@ void J9WallClock::timerLoop() { OS::sleep(_interval); } +#if defined(__GLIBCXX__) } catch (abi::__forced_unwind&) { free(frames); VM::detachThread(); // DetachCurrentThread releases any outstanding JNI local frames throw; } +#endif free(frames); From 92e99096b3ee2c5ef223ef00b644f3115a7221c6 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 30 Apr 2026 16:04:10 +0200 Subject: [PATCH 2/2] fix(j9): replace try/catch+#ifdef with RAII cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the libstdc++-only try/catch around timerLoop's body and replace it with an RAII Cleanup struct. Destructors run during forced unwinding under any C++ stdlib, so the cleanup path is no longer compiled out on libc++ (macOS) and is preserved on glibc + libc++ — addressing the review comment from chatgpt-codex-connector on PR #505. Also drops the unused include. Co-Authored-By: muse --- ddprof-lib/src/main/cpp/j9/j9WallClock.cpp | 160 ++++++++++----------- 1 file changed, 73 insertions(+), 87 deletions(-) diff --git a/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp b/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp index 130f6a8ac..9928ee838 100644 --- a/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp +++ b/ddprof-lib/src/main/cpp/j9/j9WallClock.cpp @@ -19,7 +19,6 @@ #include "j9/j9Support.h" #include "profiler.h" #include "threadState.h" -#include #include volatile bool J9WallClock::_enabled = false; @@ -58,14 +57,12 @@ void J9WallClock::stop() { } void J9WallClock::timerLoop() { - // IBM J9 may cancel this thread via abi::__forced_unwind during JVM shutdown. - // Catch it to ensure free(frames) and VM::detachThread() always run — including - // if the unwind fires during setup (VM::attachThread or malloc) before the - // sampling loop starts — then re-throw so the thread exits properly through - // the normal unwind path. jni and frames are declared before the try and - // initialized to null so the catch handler's cleanup is safe in either case: - // free(nullptr) is a no-op, and detachThread() is only called when attach - // succeeded. + // IBM J9 may cancel this thread via forced unwinding during JVM shutdown. + // glibc raises abi::__forced_unwind for pthread_cancel; libc++ does not declare + // that type, so we cannot name it in a catch. An RAII cleanup struct works + // under any C++ stdlib because destructors run during forced unwind regardless + // of whether the unwind exception type is C++-named. This also avoids the + // libc++ build break on macOS where abi::__forced_unwind is not available. // // PushLocalFrame / PopLocalFrame balance: if the forced unwind fires between // PushLocalFrame and PopLocalFrame, the outstanding JNI local frame is released @@ -73,89 +70,78 @@ void J9WallClock::timerLoop() { // destroy the current stack frame's local references. The common cancellation // point is OS::sleep() which runs after PopLocalFrame, so no imbalance occurs // in the typical case. - JNIEnv *jni = nullptr; - ASGCT_CallFrame *frames = nullptr; - // abi::__forced_unwind is declared by libstdc++ (any libc) but not by libc++, - // so the catch only compiles where libstdc++ is the active C++ stdlib. Guard - // on __GLIBCXX__: defined by libstdc++ regardless of libc (glibc, musl, etc.), - // undefined under libc++ (macOS clang default, some Alpine/clang builds). - // The exception itself is raised by glibc's pthread_cancel — on platforms with - // a different cancellation mechanism (musl, macOS) the catch is harmless dead - // code but compiles cleanly under libstdc++. -#if defined(__GLIBCXX__) - try { -#endif - jni = VM::attachThread("java-profiler Sampler"); - jvmtiEnv *jvmti = VM::jvmti(); - - int max_frames = _max_stack_depth + MAX_NATIVE_FRAMES + RESERVED_FRAMES; - frames = (ASGCT_CallFrame *)malloc(max_frames * sizeof(ASGCT_CallFrame)); - - while (_running) { - if (!_enabled) { - OS::sleep(_interval); - continue; - } + struct Cleanup { + ASGCT_CallFrame *frames = nullptr; + ~Cleanup() { + free(frames); // free(nullptr) is a no-op + VM::detachThread(); // DetachCurrentThread releases any outstanding JNI local frames + } + } cleanup; - jni->PushLocalFrame(64); - - jvmtiStackInfoExtended *stack_infos; - jint thread_count; - if (J9Support::GetAllStackTracesExtended( - _max_stack_depth, (void **)&stack_infos, &thread_count) == 0) { - for (int i = 0; i < thread_count; i++) { - jvmtiStackInfoExtended *si = &stack_infos[i]; - if (si->frame_count <= 0) { - // no frames recorded - continue; - } - OSThreadState ts = (si->state & JVMTI_THREAD_STATE_RUNNABLE) - ? OSThreadState::RUNNABLE - : OSThreadState::SLEEPING; - if (!_sample_idle_threads && ts != OSThreadState::RUNNABLE) { - // in execution profiler mode the non-running threads are skipped - continue; - } - for (int j = 0; j < si->frame_count; j++) { - jvmtiFrameInfoExtended *fi = &si->frame_buffer[j]; - frames[j].method_id = fi->method; - frames[j].bci = FrameType::encode(sanitizeJ9FrameType(fi->type), fi->location); - } - - int tid = J9Support::GetOSThreadID(si->thread); - if (tid == -1) { - // clearly an invalid TID; skip the thread - continue; - } - ExecutionEvent event; - event._thread_state = ts; - if (ts == OSThreadState::RUNNABLE) { - Profiler::instance()->recordExternalSample( - _interval, tid, si->frame_count, frames, /*truncated=*/false, - BCI_CPU, &event); - } - if (_sample_idle_threads) { - Profiler::instance()->recordExternalSample( - _interval, tid, si->frame_count, frames, /*truncated=*/false, - BCI_WALL, &event); - } - } - jvmti->Deallocate((unsigned char *)stack_infos); - } + JNIEnv *jni = VM::attachThread("java-profiler Sampler"); + jvmtiEnv *jvmti = VM::jvmti(); - jni->PopLocalFrame(NULL); + int max_frames = _max_stack_depth + MAX_NATIVE_FRAMES + RESERVED_FRAMES; + cleanup.frames = (ASGCT_CallFrame *)malloc(max_frames * sizeof(ASGCT_CallFrame)); + ASGCT_CallFrame *frames = cleanup.frames; + while (_running) { + if (!_enabled) { OS::sleep(_interval); + continue; + } + + jni->PushLocalFrame(64); + + jvmtiStackInfoExtended *stack_infos; + jint thread_count; + if (J9Support::GetAllStackTracesExtended( + _max_stack_depth, (void **)&stack_infos, &thread_count) == 0) { + for (int i = 0; i < thread_count; i++) { + jvmtiStackInfoExtended *si = &stack_infos[i]; + if (si->frame_count <= 0) { + // no frames recorded + continue; + } + OSThreadState ts = (si->state & JVMTI_THREAD_STATE_RUNNABLE) + ? OSThreadState::RUNNABLE + : OSThreadState::SLEEPING; + if (!_sample_idle_threads && ts != OSThreadState::RUNNABLE) { + // in execution profiler mode the non-running threads are skipped + continue; + } + for (int j = 0; j < si->frame_count; j++) { + jvmtiFrameInfoExtended *fi = &si->frame_buffer[j]; + frames[j].method_id = fi->method; + frames[j].bci = FrameType::encode(sanitizeJ9FrameType(fi->type), fi->location); + } + + int tid = J9Support::GetOSThreadID(si->thread); + if (tid == -1) { + // clearly an invalid TID; skip the thread + continue; + } + ExecutionEvent event; + event._thread_state = ts; + if (ts == OSThreadState::RUNNABLE) { + Profiler::instance()->recordExternalSample( + _interval, tid, si->frame_count, frames, /*truncated=*/false, + BCI_CPU, &event); + } + if (_sample_idle_threads) { + Profiler::instance()->recordExternalSample( + _interval, tid, si->frame_count, frames, /*truncated=*/false, + BCI_WALL, &event); + } + } + jvmti->Deallocate((unsigned char *)stack_infos); } -#if defined(__GLIBCXX__) - } catch (abi::__forced_unwind&) { - free(frames); - VM::detachThread(); // DetachCurrentThread releases any outstanding JNI local frames - throw; - } -#endif - free(frames); + jni->PopLocalFrame(NULL); - VM::detachThread(); + OS::sleep(_interval); + } + // Cleanup destructor runs here on normal exit; on a forced unwind it runs + // automatically as the stack unwinds out of timerLoop, regardless of the + // active C++ stdlib. }