Skip to content

module loader: coalesce concurrent embedder fetches of the same module - #33417

Open
robobun wants to merge 5 commits into
mainfrom
farm/a0f5d423/coalesce-in-flight-module-fetches
Open

module loader: coalesce concurrent embedder fetches of the same module#33417
robobun wants to merge 5 commits into
mainfrom
farm/a0f5d423/coalesce-in-flight-module-fetches

module loader: drop the unreachable empty-JSValue check on the in-fli…

4951432
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 6, 2026 in 34m 19s

Code review found 1 potential issue

Found 1 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/jsc/bindings/ZigGlobalObject.cpp:3596-3604 Stale settle reaction can evict a post-reload in-flight entry

Annotations

Check warning on line 3604 in src/jsc/bindings/ZigGlobalObject.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

Stale settle reaction can evict a post-reload in-flight entry

The settle reaction removes the map entry by key alone, so under `--hot` a pre-reload fetch that settles after `clearInFlightModuleFetches()` has run can evict the *post*-reload entry tracked at the same key — reactions attached via `performPromiseThenWithContext` survive the map clear. Impact is only a missed coalesce (results stay correct via idempotent `provideFetch`, and pre-PR had no coalescing at all), so not blocking; a compare-before-remove — pass the promise as reaction context alongsid