[turbopack] Enforce root attribute for strongly consistent reads and collectibles#93114
Open
[turbopack] Enforce root attribute for strongly consistent reads and collectibles#93114
root attribute for strongly consistent reads and collectibles#93114Conversation
…d collectibles Replace the silent "make root node" promotion with a panic when trying to read a non-root task strongly consistent, read task collectibles, or remove collectibles. Tasks must now be explicitly marked with the `root` attribute (e.g. `#[turbo_tasks::function(root)]`) to be eligible for these operations. - Replace 3 "make root node" loops in the backend with `NativeFunction.is_root` checks that panic with descriptive messages including both task descriptions - Support `root` attribute on `#[turbo_tasks::value_impl]` methods and `#[turbo_tasks::value_trait]` default methods - Add `root` to all existing operation functions, regular functions, and methods that are read with strong consistency - Add test verifying the panic fires for non-root tasks
Merging this PR will degrade performance by 3.31%
Performance Changes
Comparing 
Footnotes
|
…ait/impl matching - Add `root` to ~50 operations across napi bindings, next-api, turbopack-core, turbopack-dev-server, turbopack-cli, and tests that are read with strong consistency or have collectibles taken (`take_effects`, `strongly_consistent_catch_collectables`). This unblocks the production build panic on `has_deferred_entrypoints_operation` plus the cargo unit/bench fan-out (fetch tests, read_glob tests, module_graph tests, tracing tests, etc). - Enforce `root` attribute matching between `#[turbo_tasks::value_trait]` methods and their `#[turbo_tasks::value_impl]` implementations: store `is_root` on `TraitMethod` and panic at lazy-init in `register_all_trait_methods` if a trait method's `is_root` differs from any impl method's `is_root`. - Removed `root` from `AssetHashesManifestAsset::Asset::content` impl (`Asset::content` trait method does not declare `root`).
lukesandberg
approved these changes
Apr 22, 2026
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
2 participants
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.
What?
Replaces the silent "make root node" promotion in the turbo-tasks backend with a panic that enforces tasks to already have the
rootattribute before performing strongly consistent reads, reading task collectibles, or removing collectibles.Why?
Previously, the backend would silently promote any non-root task to a root node (aggregation number
u32::MAX) when these operations were requested. This masked incorrect task configuration — tasks that needed root-level aggregation weren't explicitly declared as such, making it harder to reason about the aggregation graph and hiding potential performance issues from implicit promotions.How?
Backend enforcement (3 locations):
backend/mod.rs): ChecksNativeFunction.is_rooton the target task. If it's a persistent task withoutroot, panics with both the target and reader task descriptions.backend/mod.rs): Same check when reading collectibles from a task.operation/update_collectible.rs): Same check when removing collectibles (count < 0).All three locations drop the task guard before panicking to avoid deadlocking with
debug_get_task_description.Macro support for
rooton methods:value_impl_macro.rs: Now reads therootattribute from#[turbo_tasks::function(root)]on inherent impl and trait impl methods (previously hardcoded tofalse).value_trait_macro.rs: Same for trait default methods.rootattribute additions:#[turbo_tasks::function(operation)]in test files, benchmarks, and fuzz codecrates/next-api/that are read with strong consistency.strongly_consistent()value_implandvalue_traitblocks used with strong consistencyNew test:
non_root_task_panic.rs: Verifies the panic fires when attempting a strongly consistent read on a non-root operation task. Captures the panic from the worker thread via a panic hook since the panic propagates as a channel error to the test thread.