fix(JS port) #5145: correct worker↔host barrier ownership model#5148
Draft
shai-almog wants to merge 3 commits into
Draft
fix(JS port) #5145: correct worker↔host barrier ownership model#5148shai-almog wants to merge 3 commits into
shai-almog wants to merge 3 commits into
Conversation
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
Cloudflare Preview
|
Collaborator
Author
|
Compared 93 screenshots: 93 matched. |
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 122 screenshots: 122 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Rework the native barrier to the C/iOS backend's model: the JS host side is a
dumb, hard-reference table that never GCs on its own; GC applies only to the
Java side, and the Java side's finalizer frees the front-end resource it owns.
1. Stop crossing the barrier for values the Java side already knows. The paint
flush and layout were calling outputCanvas.getContext('2d') /
getWidth()/getHeight() and getDisplayWidth()/Height() (-> canvas.getWidth())
on every frame -- a continuous storm of round-trips whose responses
intermittently crossed into concurrent object reads (getDocument/getContext
resuming with a width/height number -> degraded receiver -> the ButtonTheme
hard-stall). Cache displayWidth/displayHeight (set in updateCanvasSize) and
the outputCanvas 2D context (stable for a canvas) in Java fields and reuse
them; getDisplayWidth/Height return the cached values. No dimension/context
round-trips during steady-state paint.
2. Drive host-ref release from the OWNING Java object's finalizer, not from JSO
wrapper GC. NativeImage owns its backing canvas / HTMLImageElement;
registerImageResource() arms a FinalizationRegistry keyed on that resource
(a single, stable wrapper held only by the image), and on collection the
worker posts releaseHostRef for its id. The host keeps a hard ref until then.
This replaces #5143's wrapper-refcount release, which raced: the host dedups
one id across many re-created worker wrappers and a raw __jsValue marker
could outlive them, so refcount-zero released canvases still in use ->
"Missing host receiver". Single-owner keying cannot do that.
browser_bridge.releaseHostRefs now evicts whatever id the dead owner held
(canvas or image) behind the never-release singleton guard.
Refs #5145.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shai-almog
force-pushed
the
fix-jsport-barrier-model
branch
2 times, most recently
from
30c5f71 to
2df519c
Compare
…rks (#5145) The late-suite screenshot wedge (after the #5143 host-ref fix) has two roots in the worker<->host postMessage channel under a dense paint burst: 1. Response-cross: an idempotent object read (document.createElement, canvas.getContext, getImageData, measureText, canvas.toDataURL) resumes with a corrupted value -- a number, an empty {} that lost its host-ref marker, null from a never-null method, or a thrown "Missing JS member"/"Missing host receiver". The old substitute-null turned each into a hard NPE / EDT deadlock (createElement -> null canvas; or an emit-time toDataURL throw escaping a lock). 2. Lost-response: a host callback never arrives, so the green thread parks on pendingHostCalls[id] forever (hard wedge, heartbeat alive but runnable=0). Mirroring the C/iOS backend model (the host is a thin, dumb pixel sink; the worker must never blindly trust it to always respond): - Barrier-model reductions to cut cross frequency at the source: cache the document Java-side (doc(); no per-createCanvas window.getDocument()); pass the known width/height into the HTML5Graphics ctor and BufferedGraphics instead of reading canvas.getWidth()/getHeight() back across the barrier; track the scratch-buffer dims Java-side. - invokeJsoBridge retry: re-issue an idempotent read up to 12x (with a growing backoff sleep so the concurrent numeric-getter burst that caused the cross drains before retrying) on a degraded result (number / empty-{} / null-for-never-null) -- or on a transient throw for ANY round-trip method (a "Missing JS member"/"Missing host receiver" throw means the call never executed, so re-issuing is side-effect-free; this is what recovers an emit-time canvas.toDataURL() cross). - Host-call watchdog: for bounded host natives (jso bridge, DOM-element create, ui-settle, canvas->PNG capture, etc.) a lost response resumes the parked thread with a transient error so the retry re-issues / the caller advances. Unbounded natives (image load, fetch, the cn1ss WebSocket) are never aborted. Zero-cost on a healthy channel. - Re-park LightweightPickerButtons (a lightweight-popup EDT deadlock, distinct from the cross, so the retry/watchdog can't rescue it); ValidatorLightweightPicker now runs clean and stays un-parked. Worker-liveness heartbeat + host-ref counters are gated to diag-only (zero production cost). Validated in CI (javascript-screenshots). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shai-almog
force-pushed
the
fix-jsport-barrier-model
branch
2 times, most recently
from
d13fefe to
4820dc8
Compare
…om CI The screenshot harness wrote received PNGs only to a runner temp dir; nothing uploaded them, so a faithful golden could not be (re)seeded from a CI render. Copy $CN1SS_WS_DIR/*.png into $ARTIFACTS_DIR/delivered/ on both the normal and timeout exit paths so they ride along in the javascript-ui-tests artifact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
shai-almog
force-pushed
the
fix-jsport-barrier-model
branch
from
3d4c032 to
5ae7507
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reworks the native barrier to the C/iOS ownership model (per maintainer guidance): the JS host is a dumb hard-reference table that never GCs on its own; cleanup is the Java side's job via a finalizer that frees the front-end resource it owns.
1. Stop crossing the barrier for known dims/context. Cached
displayWidth/displayHeight+ theoutputCanvas2D context Java-side;getDisplayWidth/Heightand the paint flush no longer round-trip every frame. This removes the dimension/context round-trip storm whose responses crossed intogetDocument/getContextobject reads (the degraded-receiver source).2. Owning-object finalizer release.
NativeImageowns its backing canvas /HTMLImageElement;registerImageResource()arms a FinalizationRegistry keyed on that single stable resource, releasing its host id when the image is GC'd. Replaces #5143's wrapper-refcount release (which raced via re-created wrappers + raw markers → "Missing host receiver").Validated locally:
NUMBER_FOR_OBJECT=0,Missing host receiver=0,grid NPE=0across the whole run — the response-cross and receiver-deletion are eliminated.Known remaining (WIP): a separate, pre-existing mechanism-3 capture/emit stall (worker silent right after
__cn1_capture_canvas_png__resolves) still intermittently wedges; being fixed before this lands. Refs #5145.