You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I didn't see a .github/workflows/ setup here, so while building coverage tooling
on a personal fork I put together a GitHub Actions CI setup (Linux/macOS/Windows
builds, JUnit tests, JaCoCo+LLVM coverage, CodeQL) in case it's useful. Along the
way it surfaced 9 real bugs, all fixed, verified against a live CI run with all
checks green and a processed Codecov report.
No action is required from anyone here. Everything below is in one squashed,
CI-validated commit at Thrameos#35, with the full, unsquashed commit-by-
commit history preserved at the backport/phase1-ci-infra-full-history tag in the
same fork. Whatever's useful is available: merge the PR as-is if you want the whole
thing, hand it to a volunteer to review at whatever pace works, or cherry-pick
whichever individual fixes are wanted straight out of the full-history tag and
ignore the rest. Just flagging what's there.
Enhancements
GitHub Actions CI: build jobs for Linux/macOS/Windows, a JUnit test job, a
coverage job, and CodeQL analysis (C++ and Java).
ENABLE_LLVM_COVERAGE: a Clang source-based native coverage CMake option.
gcov-style instrumentation was tried and rejected during development --
its .gcda writer isn't fork-safe against CEF's zygote-style process
model, and silently produces wrong (usually near-zero) numbers for
whichever files a test happens to hit a fork on -- so it was never carried
into the branch's shipped configuration.
JaCoCo wired into the Java JUnit run (pinned to 0.8.15 for JDK 25 class-file
support -- 0.8.12 can't parse it).
A CI wrapper (tools/run_tests_ci.sh / tools/run_coverage_ci.sh) that tells
a real, reportable JUnit test failure apart from a known, already-diagnosed
native crash: a real failure is never silently retried, while a known crash
signature is retried within a bounded budget, or accepted as passing once
it's been verified that the crash doesn't lose any test or coverage data.
The point is to avoid two opposite failure modes -- a flaky job that's
ignored because it's "always red," and a job that looks green by quietly
swallowing real failures.
A leak-checker / process-isolation test harness (LeakChecker, LeakTargets, IsolatedRunner) for RSS-growth-based leak sweeps, plus roughly 25 tests ported
from CEF's own ceftests suite and other coverage work (value objects, browser/
context/request/response/download-item coverage, OSR/windowed smoke tests, drag-
data, cookies, message router, print/PDF settings, etc.).
Current baseline on this branch: ~38% combined coverage.
Bugs found and fixed
CefMessageRouter leak on router removal.RemoveMessageRouter() never
called CancelPending() on outstanding persistent-query callbacks, which hold
a CefRefPtr back to the router. If a browser is removed mid-query rather than
actually closing, nothing else ever releases that reference -- the router, and
transitively the browser context it keeps alive, leaks for the process's life.
This looks like the same issue Preserve persistent query callbacks #541 and Support persistent & binary queries/callbacks #528 are addressing.
CefRequestContext double-caching leak.getGlobalContextNative() was
missing a final else, so every call after the first cached (and leaked) a
second native reference.
Global CefRequestContext/CefCookieManager never disposed at shutdown.
The cached global instance outlived CefShutdown(), tripping DCHECK(all_.empty()) in browser_context.cc's exit-time leak detector
(Debug builds only -- invisible in Release, since that check is compiled out
there).
CefClient.onGotFocus() synchronous infinite recursion.setFocus(true)
synchronously re-fires OnWebContentsFocused -> onGotFocus() again for the
same browser; CEF's own OnSetFocus() guards itself against this kind of
reentrancy but OnGotFocus/OnWebContentsFocused has no equivalent guard. An
already-focused browser recurses until the thread's stack overflows.
ScopedJNIObject<T> mixed locked/unlocked accessors. A real SIGSEGV during
ordinary handler-removal teardown, plus a related check-then-create race in GetOrCreateCefObject().
Two browser-close hangs. OSR mode never fires OnBeforeClose for a browser
whose renderer already died (e.g. after a deliberate chrome://crash);
windowed mode had no fallback for the equivalent case either. Both now get a
bounded fallback instead of hanging indefinitely.
doMessageLoopWork's self-perpetuating Timer outliving native shutdown.
An independently-scheduled Timer tick (up to ~33ms out) can still fire after N_Shutdown() has already destroyed the native Context singleton --
harmless in Release, a real SIGSEGV in Debug builds where the resulting null- this DCHECK actually runs.
CefClientHandler handler-removal methods used the abstract interface type
instead of the concrete wrapper type. Follow-up to the ScopedJNIObject
crash above.
Two smaller fixes: a null-guard gap causing Debug-only DCHECK aborts in
several _N.cpp setters (CefRequest/CefResponse/CefPostDataElement and
others), and a CodeQL medium-severity finding (a world-readable temp file on
POSIX).
Public API notes
Two new public static methods were added, both purely additive -- no existing
public signature was changed, removed, or had its contract broken:
CefRequestContext.disposeGlobalContext()
CefCookieManager.disposeGlobalManager()
Both are documented as internal-use-only (called from CefApp's shutdown sequence
to release the cached global instance before native CEF shutdown runs), but are
callable by any embedding app since they're public. Nothing else in the public org.cef surface changed.
Credits
This work was done with substantial AI assistance (Claude, Anthropic) under my
direction and review -- disclosing that up front since it materially shaped how
this was produced.
Bug #5 above (ScopedJNIObject<T> mixed locked/unlocked accessors) was found by
diffing this project's teardown code against JetBrains/jcef (branch 261,
CEF 137), which independently added a locking mechanism around the same native-
pointer accessor call sites with a comment describing the identical crash
signature we hit. The fix here is adapted from that fork's approach.
Known limitation
The underlying browser_context.cc:44DCHECK(all_.empty()) shutdown-time leak
detector is not fully fixed by #2/#3 above -- it still fires via at least one
further mechanism, confirmed independent of reference counting (ADDREF/RELEASE and
JNI ref counts both balance perfectly when it fires). The coverage CI job
recognizes this specific, well-understood signature and tolerates it rather than
treating it as a fresh failure, since it only ever fires after every test in a run
has already completed. Root-causing it further is still open.
The offer, up front
I didn't see a
.github/workflows/setup here, so while building coverage toolingon a personal fork I put together a GitHub Actions CI setup (Linux/macOS/Windows
builds, JUnit tests, JaCoCo+LLVM coverage, CodeQL) in case it's useful. Along the
way it surfaced 9 real bugs, all fixed, verified against a live CI run with all
checks green and a processed Codecov report.
No action is required from anyone here. Everything below is in one squashed,
CI-validated commit at Thrameos#35, with the full, unsquashed commit-by-
commit history preserved at the
backport/phase1-ci-infra-full-historytag in thesame fork. Whatever's useful is available: merge the PR as-is if you want the whole
thing, hand it to a volunteer to review at whatever pace works, or cherry-pick
whichever individual fixes are wanted straight out of the full-history tag and
ignore the rest. Just flagging what's there.
Enhancements
coverage job, and CodeQL analysis (C++ and Java).
ENABLE_LLVM_COVERAGE: a Clang source-based native coverage CMake option.gcov-style instrumentation was tried and rejected during development --
its
.gcdawriter isn't fork-safe against CEF's zygote-style processmodel, and silently produces wrong (usually near-zero) numbers for
whichever files a test happens to hit a fork on -- so it was never carried
into the branch's shipped configuration.
support -- 0.8.12 can't parse it).
tools/run_tests_ci.sh/tools/run_coverage_ci.sh) that tellsa real, reportable JUnit test failure apart from a known, already-diagnosed
native crash: a real failure is never silently retried, while a known crash
signature is retried within a bounded budget, or accepted as passing once
it's been verified that the crash doesn't lose any test or coverage data.
The point is to avoid two opposite failure modes -- a flaky job that's
ignored because it's "always red," and a job that looks green by quietly
swallowing real failures.
LeakChecker,LeakTargets,IsolatedRunner) for RSS-growth-based leak sweeps, plus roughly 25 tests portedfrom CEF's own
ceftestssuite and other coverage work (value objects, browser/context/request/response/download-item coverage, OSR/windowed smoke tests, drag-
data, cookies, message router, print/PDF settings, etc.).
Bugs found and fixed
CefMessageRouterleak on router removal.RemoveMessageRouter()nevercalled
CancelPending()on outstanding persistent-query callbacks, which holda
CefRefPtrback to the router. If a browser is removed mid-query rather thanactually closing, nothing else ever releases that reference -- the router, and
transitively the browser context it keeps alive, leaks for the process's life.
This looks like the same issue Preserve persistent query callbacks #541 and Support persistent & binary queries/callbacks #528 are addressing.
CefRequestContextdouble-caching leak.getGlobalContextNative()wasmissing a final
else, so every call after the first cached (and leaked) asecond native reference.
CefRequestContext/CefCookieManagernever disposed at shutdown.The cached global instance outlived
CefShutdown(), trippingDCHECK(all_.empty())inbrowser_context.cc's exit-time leak detector(Debug builds only -- invisible in Release, since that check is compiled out
there).
CefClient.onGotFocus()synchronous infinite recursion.setFocus(true)synchronously re-fires
OnWebContentsFocused->onGotFocus()again for thesame browser; CEF's own
OnSetFocus()guards itself against this kind ofreentrancy but
OnGotFocus/OnWebContentsFocusedhas no equivalent guard. Analready-focused browser recurses until the thread's stack overflows.
ScopedJNIObject<T>mixed locked/unlocked accessors. A real SIGSEGV duringordinary handler-removal teardown, plus a related check-then-create race in
GetOrCreateCefObject().OnBeforeClosefor a browserwhose renderer already died (e.g. after a deliberate
chrome://crash);windowed mode had no fallback for the equivalent case either. Both now get a
bounded fallback instead of hanging indefinitely.
doMessageLoopWork's self-perpetuating Timer outliving native shutdown.An independently-scheduled Timer tick (up to ~33ms out) can still fire after
N_Shutdown()has already destroyed the nativeContextsingleton --harmless in Release, a real SIGSEGV in Debug builds where the resulting null-
thisDCHECK actually runs.CefClientHandlerhandler-removal methods used the abstract interface typeinstead of the concrete wrapper type. Follow-up to the
ScopedJNIObjectcrash above.
several
_N.cppsetters (CefRequest/CefResponse/CefPostDataElementandothers), and a CodeQL medium-severity finding (a world-readable temp file on
POSIX).
Public API notes
Two new
public staticmethods were added, both purely additive -- no existingpublic signature was changed, removed, or had its contract broken:
CefRequestContext.disposeGlobalContext()CefCookieManager.disposeGlobalManager()Both are documented as internal-use-only (called from
CefApp's shutdown sequenceto release the cached global instance before native CEF shutdown runs), but are
callable by any embedding app since they're public. Nothing else in the public
org.cefsurface changed.Credits
This work was done with substantial AI assistance (Claude, Anthropic) under my
direction and review -- disclosing that up front since it materially shaped how
this was produced.
Bug #5 above (
ScopedJNIObject<T>mixed locked/unlocked accessors) was found bydiffing this project's teardown code against JetBrains/jcef (branch 261,
CEF 137), which independently added a locking mechanism around the same native-
pointer accessor call sites with a comment describing the identical crash
signature we hit. The fix here is adapted from that fork's approach.
Known limitation
The underlying
browser_context.cc:44DCHECK(all_.empty())shutdown-time leakdetector is not fully fixed by #2/#3 above -- it still fires via at least one
further mechanism, confirmed independent of reference counting (ADDREF/RELEASE and
JNI ref counts both balance perfectly when it fires). The coverage CI job
recognizes this specific, well-understood signature and tolerates it rather than
treating it as a fresh failure, since it only ever fires after every test in a run
has already completed. Root-causing it further is still open.