fix: pre-resolve paths option to avoid invoke_sync deadlock#8518
Merged
IWANABETHATGUY merged 2 commits intomainfrom Mar 2, 2026
Merged
fix: pre-resolve paths option to avoid invoke_sync deadlock#8518IWANABETHATGUY merged 2 commits intomainfrom
invoke_sync deadlock#8518IWANABETHATGUY merged 2 commits intomainfrom
Conversation
Member
Author
How to use the Graphite Merge QueueAdd the label graphite: merge-when-ready to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
invoke_sync deadlock
f93d40e to
9afc87d
Compare
Member
Author
Merge activity
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a potential deadlock/hang in the JS binding by removing the synchronous invoke_sync callback path for the output.paths option, and instead pre-resolving paths asynchronously before entering sync rendering code.
Changes:
- Change
PathsFunctionto async and introducePathsOutputOption::resolve_all()to pre-resolve mappings. - Remove
invoke_syncfromJsCallbackExtand update binding normalization to useinvoke_async. - Thread a pre-resolved
resolved_pathsthrough generate/finalize/render paths and use it for external import path rendering.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rolldown_common/src/inner_bundler_options/types/output_option/paths.rs | Makes paths callbacks async-capable and adds async pre-resolution into an FxHashMap. |
| crates/rolldown_binding/src/utils/normalize_binding_options.rs | Switches paths normalization from invoke_sync to invoke_async (boxed future). |
| crates/rolldown_binding/src/types/js_callback.rs | Removes the invoke_sync API to eliminate the deadlock-prone blocking path. |
| crates/rolldown/src/utils/chunk/mod.rs | Uses pre-resolved paths when producing rendered chunk metadata for external imports. |
| crates/rolldown/src/utils/chunk/finalize_chunks.rs | Threads resolved_paths into asset finalization for external import lists. |
| crates/rolldown/src/types/generator.rs | Adds resolved_paths to GenerateContext for sync rendering code. |
| crates/rolldown/src/stages/generate_stage/render_chunk_to_assets.rs | Passes resolved_paths into finalize_assets and generator contexts. |
| crates/rolldown/src/stages/generate_stage/mod.rs | Pre-resolves paths once during generation and stores it on the stage. |
| crates/rolldown/src/stages/generate_stage/finalize_modules.rs | Passes resolved_paths into the scope-hoisting finalizer context. |
| crates/rolldown/src/module_finalizers/mod.rs | Switches external import path computations to use resolved_paths. |
| crates/rolldown/src/module_finalizers/finalizer_context.rs | Extends finalizer context with resolved_paths. |
| crates/rolldown/src/ecmascript/format/umd.rs | Uses resolved_paths for UMD external dependency strings. |
| crates/rolldown/src/ecmascript/format/esm.rs | Uses resolved_paths for ESM external import/export path rendering. |
| crates/rolldown/src/ecmascript/format/cjs.rs | Uses resolved_paths for CJS require(...) path rendering. |
Comments suppressed due to low confidence (1)
crates/rolldown_common/src/inner_bundler_options/types/output_option/paths.rs:41
- In
resolve_all, the loop usesidafter awaitingf(id), which forces the async state machine to hold a borrowed&stracross an.await. Convertingidto an ownedStringbefore the await (and reusing that for both the callback input and the map key) avoids borrow-across-await and makes this helper more flexible for future call sites.
let mut resolved = FxHashMap::default();
for id in ids {
if let Ok(path) = f(id).await {
resolved.insert(id.to_string(), path);
}
crates/rolldown_common/src/inner_bundler_options/types/output_option/paths.rs
Outdated
Show resolved
Hide resolved
Contributor
Benchmarks Rust |
9afc87d to
d3a28ad
Compare
sapphi-red
reviewed
Mar 2, 2026
sapphi-red
approved these changes
Mar 2, 2026
Contributor
|
@IWANABETHATGUY I've opened a new pull request, #8520, to work on those changes. Once the pull request is ready, I'll request review from you. |
shulaoda
added a commit
that referenced
this pull request
Mar 5, 2026
## [1.0.0-rc.7] - 2026-03-05 ⚡ Smarter Code Generation Defaults - DCE-only minification and smart constant inlining are now enabled by default - Produces cleaner, smaller output bundles without requiring explicit configuration 💡 LLM-Friendly Bundle Analyzer Reports - New markdown output format for the bundle analyzer plugin with bundle summaries, module graphs, dependency chains, and optimization suggestions - Optimization suggestions now also recommend using the entriesAware option when common chunks contain modules only reachable from specific entries ### 💥 BREAKING CHANGES - enable minify: 'dce-only' by default (#8465) by @IWANABETHATGUY - settings `inlineConst: { mode: 'smart', pass: 1}` by default (#8444) by @IWANABETHATGUY ### 🚀 Features - binding: add original getter to BindingMagicString (#8533) by @IWANABETHATGUY - native-magic-string: add `offset` property support (#8531) by @IWANABETHATGUY - add `output.strict` option to control `"use strict"` directive emission (#8489) by @Copilot - watch: expose `watcher.compareContentsForPolling` (#8526) by @hyf0 - watch: use new watcher to support watch mode (#8475) by @hyf0 - rust/watch: handle bulk-change (#8466) by @hyf0 - add LLM-friendly markdown output format to bundle analyzer plugin (#8242) by @IWANABETHATGUY ### 🐛 Bug Fixes - expose `plugins` on `NormalizedInputOptions` for `buildStart` hook (#8521) by @Copilot - only uppercase facade symbols in JSX preserve mode (#8519) by @IWANABETHATGUY - binding: export BindingResult in generated dts header (#8537) by @minsoo-web - pre-resolve paths option to avoid `invoke_sync` deadlock (#8518) by @IWANABETHATGUY - remove debug-only jsx_preset and UntranspiledSyntaxError (#8511) by @IWANABETHATGUY - apply `topLevelVar` to exported `const`/`let` declarations (#8507) by @IWANABETHATGUY - rolldown_plugin_vite_web_worker_post: avoid replacing `new.target` (#8488) by @sapphi-red - update copyright year to 2026 (#8486) by @maciekzygmunt ### 🚜 Refactor - rust: use Oxc's SymbolFlags::ConstVariable instead of custom IsConst flag (#8543) by @Dunqing - rust: remove FacadeScoping, use Scoping::create_symbol for facade symbols (#8540) by @Dunqing - rust/watch: remove hacky `reset_closed_for_watch_mode` (#8530) by @hyf0 - binding: return &str instead of String in filename() getter (#8534) by @IWANABETHATGUY - rust: remove old watch mode implementation (#8525) by @hyf0 - rust/watch: simply watch logic in the binding layer (#8516) by @hyf0 - rust/watch: tweak struct/function names (#8464) by @hyf0 ### 📚 Documentation - explain how external modules work in rolldown (#8457) by @sapphi-red - add some diagrams using graphviz (#8499) by @sapphi-red - use `vitepress-plugin-graphviz` (#8498) by @sapphi-red - list s390x/ppc64le prebuilt binaries (#8495) by @crusty-voidzero - fix error type for `RolldownBuild.generate` and others (#8490) by @sapphi-red ### ⚡ Performance - string_wizard: reduce allocations and add ASCII fast paths (#8541) by @IWANABETHATGUY - use IndexBitSet to replace IndexVec<XXXIdx, bool> for module/stmt inclusion tracking (#8503) by @IWANABETHATGUY - plugin: use IndexBitSet to optimize skipped plugins checking (#8497) by @ShroXd - rust/tla: skip compute_tla if there is no module use TLA (#8487) by @ShroXd ### 🧪 Testing - node/watch: make watch tests run in concurrent and retry-able (#8512) by @hyf0 - add test case for static flag tree-shaking (#8476) by @IWANABETHATGUY - migrate post-banner sourcemap-with-shebang to Rust (#8477) by @Copilot ### ⚙️ Miscellaneous Tasks - vscode: `formatOnSave` for markdown files using oxc formatter (#8536) by @minsoo-web - deps: update test262 submodule for tests (#8528) by @sapphi-red - remove `retry` workaround from output paths test fixtures (#8520) by @Copilot - docs: add Shuyuan Wang (h-a-n-a) and remove from acknowledgements (#8509) by @Copilot - consolidate top_level_var test cases using configVariants (#8508) by @IWANABETHATGUY - add s390x and ppc64le linux gnu targets (#8493) by @Brooooooklyn ###◀️ Revert - fix(rolldown): increase tokio blocking threads size for watch mode (#8517) by @hyf0 ### ❤️ New Contributors * @minsoo-web made their first contribution in [#8536](#8536) * @crusty-voidzero made their first contribution in [#8495](#8495) * @maciekzygmunt made their first contribution in [#8486](#8486) Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
7 tasks
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
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.
closed #7280
Description: Pre-resolves the
pathsoutput option asynchronously before entering sync rendering code, eliminating the deadlock-proneinvoke_syncpattern that blocks threads waiting for JS callbacks viaMutex + Condvar. ChangesPathsFunctionto async (matching other options likeglobals), adds aresolve_all()method, and removesinvoke_syncentirely.