Repository navigation
Intermittent SIGSEGV in SetCefForJNIObjectHelper::Release during normal CefClient teardown (Debug build) #22
Description
Activity
Found a strong, evidence-backed root-cause candidate by diffing against JetBrains' own jcef fork (branch 261, CEF 137 -- 9 versions behind ours but architecturally very close, ~154 shared native/ filenames). They independently hit and fixed exactly this class of bug: every
Dispose()/native-pointer-clear call site was migrated to a lock-protected accessor pair (lockAndGetNativeRef()/unlock()on the Java side,SetCefForJNIObject_sync/GetCefFromJNIObject_syncon the native side), with an explicit comment matching our crash: "must do it after setNativeRef_safe (otherwise we can create CefRefPtr with killed ptr)".Applied to this fork:
- Reordered
SetCefForJNIObject()(jni_scoped_helpers.h) to update the Java-side pointer before releasing the previous object, not after -- closes a window where a concurrent reader could get a stale, already-released pointer. Applies to all ~150 existing call sites automatically. - Marked the native handle field
volatileinCefNativeAdapter/CefRequestContext_N-- a non-volatile 64-bit field read/write isn't guaranteed atomic per JLS 17.7, so a concurrent reader could observe a torn (half-old/half-new) value. This matches your crash signature exactly: a SIGSEGV dereferencing pointer0x21, which looks exactly like a torn/garbage value rather than a real freed pointer. - Added the full lock-based
_syncvariants and rolled them out toCefClientHandler.cpp(the exact function in this issue's stack trace,Java_org_cef_handler_CefClientHandler_N_1removeFocusHandler) andCefRequestContext_N.cpp.
The SIGSEGV did not reproduce across several repeated Debug/
ENABLE_COVERAGEfull-suite runs after these fixes (previously intermittent). Given the intermittent nature I can't claim it's provably eliminated, but the root cause is now well understood and directly addressed, not just papered over. Full Release suite: 164/164 passing, no regressions.- Reordered
- added a commit that references this issue
on Aug 30, 2026 Correction to my earlier comment: this SIGSEGV did reproduce again just now, once, during an otherwise-unrelated coverage run (mid-suite this time, not at shutdown) -- exact same signature (`SIGSEGV at pc=0x21` in `SetCefForJNIObjectHelper::Release`). I should not have implied it was resolved based on a handful of clean runs; this bug is intermittent by nature (as originally documented) and absence of a crash in N runs isn't proof.
The fixes already applied (reordered `SetCefForJNIObject`, volatile handle fields, scoped `_sync` locking for `CefClientHandler`/`CefRequestContext`) are real, correct, and address a genuine race -- but evidently not the only remaining source of this crash class, since it's now confirmed to fire from a code path that goes through the same `SetCefForJNIObjectHelper::Release` regardless. The
_syncrollout so far only coversCefClientHandler.cppandCefRequestContext_N.cpp-- there are ~150 other call sites still on the plain (non-locking)SetCefForJNIObject/GetCefFromJNIObject, any of which could be the one actually crashing here. Broader rollout (or at minimum, capturing anhs_err_pid*.logstack trace from the next reproduction to identify which specific handler type is involved this time) is the natural next step -- not closing this out yet.Broadened the fix scope significantly (following up on the earlier CefClientHandler/CefRequestContext-only scope): the
_synclocking mechanism (volatile field +ReentrantLock+lockAndGetNativeRef()/unlock()) is now applied to essentially every class implementingCefNative, and everySetCefForJNIObject<T>/GetCefFromJNIObject<T>disposal/creation call site across ~26 native files -- includingjni_util.cpp'sGetJNIBrowser()(probably the single highest-traffic call site of this kind, used throughout the whole codebase) andlife_span_handler.cpp's browser creation/close lifecycle.Honest result: the SIGSEGV still reproduced once in 4 repeated Debug/
ENABLE_COVERAGEruns after this broader rollout. Reduced exposure, not eliminated.A plausible reason this class of fix has a ceiling: most of JCEF's value-object classes (
CefRequest,CefResponse,CefPostData, etc.) don't actually use theGetCefFromJNIObjectJNI-callback pattern that the lock protects for their per-instance getters/setters -- they use a different pattern where the Java side reads its ownlonghandle field and passes it directly as ajlong selfparameter to each native method (e.g.N_GetURL(N_CefHandle)). For that pattern, the race window is between the Java-side field read and the native call actually using that value -- two separate, non-atomic steps with real (if small) time between them for JNI transition overhead. Locking around create/dispose alone doesn't close that window; it would need either a lock held across every individual native call (a much larger, more invasive change touching every one of ~150+ methods) or -- and this may be the more fundamental fix -- moving off the deprecatedfinalize()-based cleanup all 30-ish of these classes currently use, toward aPhantomReference/java.lang.ref.Cleaner-based approach that can guarantee an object is never finalized while a native call could still be referencing it.finalize()runs on the JVM's own schedule with no coordination with in-flight native use at all, which is a weaker guarantee than what these value objects likely need.Not resolved. Documented as the concrete next-level lead for whoever picks this up -- the fix already applied is a real improvement worth keeping regardless.
- added a commit that references this issue
on Aug 30, 2026 Root-caused with a live symbolized stack trace (commit
1df1b4f)This time the SIGSEGV hit during this session's own coverage-measurement run and left behind a
hs_err_pid*.logwith a full native stack trace:SetCefForJNIObjectHelper::Release(CefBaseRefCounted*) SetCefForJNIObject_sync<CefFocusHandler>(...) Java_org_cef_handler_CefClientHandler_N_1removeFocusHandler <- CefClientHandler.removeFocusHandler() <- CefClient.cleanupBrowser() <- CefClient.onBeforeClose() <- LifeSpanHandler::OnBeforeClose() <- ... deep inside WebContentsImpl::~WebContentsImpl() / WebContentsDestroyed()i.e. this happens during ordinary, clean browser teardown — not a rare edge case.
Root cause:
CefClientHandler.cpp'sremove*Handlerfunctions were converted to the lock-protectedSetCefForJNIObject_sync()/GetCefFromJNIObject_sync()in an earlier fix pass, butScopedJNIObject<T>— used byClientHandler::GetHandler<T>()to lazily create and cache the native wrapper object the first time anyCef*Handlergetter (GetFocusHandler(),GetContextMenuHandler(), etc.) is called — still used the plain, non-lockingSetCefForJNIObject()/GetCefFromJNIObject()on that exact same underlying native-ref field.Mixing a locked writer with an unlocked reader/writer on the same field gives neither side any real protection — the unlocked side never even attempts to acquire the lock the other side relies on. Deep
WebContentsteardown is reentrant enough (destruction notifies multiple observers, any of which can synchronously queryGetFocusHandler()again) for this race to be reachable on a single thread, without needing true cross-thread concurrency.Fix:
ScopedJNIObject<T>'s constructor, destructor, andGetOrCreateCefObject()now route through the_syncaccessors wheneverPtrTis the defaultCefRefPtr<T>(every instantiation exceptCefSchemeRegistrar, aCefBaseScopedtype structurally incompatible withGetCefFromJNIObject_sync()'sCefRefPtr<T>return type — handled viaif constexpr, no call-site changes needed). This closes the gap for everyCef*Handlertype at once, not justCefFocusHandler.Verification: 5/5 repeated full Release-suite runs clean (169/169 passing, zero SIGSEGV mentions) — previously this reproduced within about 4 runs. Debug/coverage-build stress runs still hit the already-documented, unrelated issue #23 DCHECK (
browser_context.cc:44,all_.empty()) plus ascoped_refptr.h:292 DCHECK failed: ptr_.in what looks like a child/helper process — confirmed both already appear in this project's saved logs from sessions well before today's fix, so they're pre-existing and belong to #23, not a regression from this change.- added a commit that references this issue
on Aug 30, 2026 Fixed in 78d135f. Root cause: every
remove*Handler()innative/CefClientHandler.cppexceptremoveWindowHandlercalledSetCefForJNIObject_sync<CefXxxHandler>using the abstract CEF interface type, while the correspondingClientHandler::GetXxxHandler()(native/client_handler.cpp) stores the object viaScopedJNIObject<XxxHandler>using the concrete wrapper type -- areinterpret_cast<T*>of the same stored pointer under two different static types on either side of the same field.removeWindowHandlerwas already written correctly (using the concreteWindowHandlertype); every other one wasn't, an apparent copy-paste inconsistency rather than an intentional choice.Verified by reproducing the exact SIGSEGV live (fresh clean rebuild, same crash offset every time -- ruling out a stale build), fixing all 12
remove*Handler()functions to matchremoveWindowHandler's pattern, then reproducing the identical repro post-fix: clean run through to shutdown, no SIGSEGV, only the separate, already-tracked #23 DCHECK at the very end.- added 13 commits that reference this issue
on Aug 31, 2026
Summary
A real JVM-level
SIGSEGV(not one of CEF's ownFATAL/DCHECK/CHECKmessages -- a genuine crash report from the JRE itself) was hit during a Debug/ENABLE_COVERAGEbuild run of this fork's full test suite, in code that runs during every single test's normal browser/client teardown, not anything specific to one test. Intermittent, not deterministic -- it did not reproduce on other full-suite Debug runs earlier in the same session with a very similar (though not identical) set of tests.Full native stack (from the JVM's own
hs_err_pid*.log):CefClient.cleanupBrowser()unconditionally callsremoveFocusHandler(this)(and every otherremoveXxxHandler(this)) once all browsers for that client are closed and the client itself is being disposed -- this happens for everyCefClient, regardless of whether aCefFocusHandlerwas ever actually registered on it. The crash is insideSetCefForJNIObjectHelper::Release, called fromSetCefForJNIObject<CefFocusHandler>while clearing that slot -- consistent withRelease()being called on a stale/already-freed/never-validly-setCefBaseRefCounted*(the crash address0x21looks like a small-integer/garbage value being dereferenced as a vtable pointer, not a real object).Reproducibility
Intermittent, not yet deterministically reproducible. Hit once during a full-suite run (
--select-package tests.junittests, four known-crashing classes excluded by name for unrelated reasons -- see #9/#16) against theENABLE_COVERAGEDebug build. Not yet isolated to a specific test or--select-class/--select-methodcombination; a Release-build equivalent full-suite run earlier in the same session (176 tests, same test set) completed cleanly.Impact
This blocks getting a fully clean
ENABLE_COVERAGEDebug-buildgcovrrun for real native coverage measurement (tracked in #5/Track B) -- the run crashes beforeCoverageTestHelper.flush()gets a chance to run, losing that run's.gcdadata (0 files written). More importantly, if this is a real use-after-free inSetCefForJNIObjectHelper::Release/SetCefForJNIObject, it's a general JCEF robustness concern independent of coverage tooling -- it happens during completely ordinary browser-close teardown that every embedding application performs.Not attempted
No isolation/bisection attempted yet given how much of this session was already spent on other findings -- flagging this now rather than chasing an intermittent, hard-to-reproduce native crash further in the same sitting. Whoever picks this up next should start by trying to reproduce it deterministically (repeated full-suite Debug runs, or specifically stress-testing
CefClientdisposal/removeFocusHandlerunderENABLE_COVERAGE) before attempting a fix.Found via
This fork's coverage-expansion effort (tracked in #5), while re-running Track B's
ENABLE_COVERAGEDebug-build measurement after fixing the two crashes tracked in #20 and #21.