Skip to content

fix: don't re-enter configure() while an async gl factory is pending#3783

Open
tyrauber wants to merge 1 commit into
pmndrs:masterfrom
tyrauber:fix/async-configure-reentrant
Open

fix: don't re-enter configure() while an async gl factory is pending#3783
tyrauber wants to merge 1 commit into
pmndrs:masterfrom
tyrauber:fix/async-configure-reentrant

Conversation

@tyrauber

Copy link
Copy Markdown

Fixes #3782.

Why:

configure() runs from the Canvas layout effect on every commit and guards renderer creation with if (!state.gl). That check runs before the await of an async gl factory resolves. When the owner re-renders while the factory is still pending (a setState from a sibling effect is enough), a second configure() slips past the guard and invokes the factory again. With a WebGPU renderer that means two renderers on one canvas, and every frame fails GPU validation on a depth-buffer size mismatch. We also traced silent WebGPU canvas death on client-side navigation in static exports to this same race.

The fix:

serialize configure() on a per-root latch. A re-entrant call waits for the in-flight configure to settle, then runs against settled state, sees state.gl is set, and skips renderer creation. The latch clears synchronously at the end, so a synchronous gl config stays synchronous and the existing XR path is unaffected.

Test plan:

  • New regression test: two concurrent configure() calls with an async gl factory invoke it once, not twice. Fails on master (factory called twice), passes with the fix.
  • Full suite green (169 passed), typecheck and lint clean.

Transparency note: this issue and its PR were drafted with AI assistance. I'm a human, I've personally verified the bug and the fix, and I'm happy to answer any questions or concerns directly. Thanks for the great library.

configure() runs from the Canvas layout effect on every commit. When an
async gl factory (e.g. a WebGPURenderer that awaits init) is still
initializing, a re-render re-enters the body past the `if (!state.gl)`
guard and invokes the factory a second time: two renderers share one
canvas and every frame fails GPU validation. Re-entrant calls now wait
for the in-flight configure to settle, so they run against settled state
and every one-time guard holds. Synchronous gl configs are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codesandbox-ci

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit fe21c1b:

Sandbox Source
example Configuration

@tyrauber

Copy link
Copy Markdown
Author

One deliberate scoping note for reviewers. The latch clears on the success path, so if the async gl factory itself rejects (e.g. WebGPU requestAdapter fails), it stays set and a later configure() retry would hang. I kept this PR tight to the re-entrancy fix and left that out on purpose: on master a rejected factory already leaves the root unusable (pending never resolves, render() hangs), so this isn't a new failure mode for the normal case. It only affects a catch-the-init-failure-and-retry path.

Making it fully throw-safe is a one-liner in spirit: wrap the body in try/finally and clear the latch there. I skipped it here only because it reindents the whole configure() body and buries a 12-line fix in a ~370-line whitespace diff. Happy to send it as a small follow-up, or fold it in here if you'd rather have it in one commit. Your call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-render during async gl factory invokes it twice and corrupts the renderer

1 participant