turbo-tasks-backend: assert non-transient task_ids in track_modification#91924
Merged
lukesandberg merged 1 commit intocanaryfrom Mar 26, 2026
Merged
turbo-tasks-backend: assert non-transient task_ids in track_modification#91924lukesandberg merged 1 commit intocanaryfrom
lukesandberg merged 1 commit intocanaryfrom
Conversation
Transient task IDs (high bit set, never serialized) should never be inserted into Storage::modified, which tracks tasks that need to be persisted to disk. The invariant was only enforced at the caller layer (TaskGuardImpl::track_modification and invalidate_serialization), not at the storage layer itself. Add a debug_assert! in StorageWriteGuard::track_modification to catch this violation early in debug builds, matching the downstream unreachable!() in snapshot_and_persist that fires when a transient ID is found in the modified set during the persist cycle. This was triggered by concurrent server component HMR updates in the hmr-iframe integration test. Co-Authored-By: Claude <[email protected]>
Collaborator
Tests Passed |
lukesandberg
approved these changes
Mar 26, 2026
Merging this PR will not alter performance
Comparing Footnotes
|
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.
What?
Add a
debug_assert!inStorageWriteGuard::track_modificationto enforce that transientTaskIds are never inserted into the persistence-modified set.Why?
Concurrent server component HMR updates triggered a runtime panic in Turbopack:
This
unreachable!lives insnapshot_and_persist(backend/mod.rs:1254), which is called during the persist cycle. It fires when a transientTaskIdis found inStorage::modified— the set that tracks tasks whose state needs to be written to disk.Transient task IDs (high bit set) are never serialized. The invariant was enforced only at the caller layer:
TaskGuardImpl::track_modification(operation/mod.rs:1029) guards withif !self.task_id.is_transient()TaskGuardImpl::invalidate_serialization(operation/mod.rs:970) guards withif !self.task_id.is_transient()But
StorageWriteGuard::track_modification— the public method on the storage struct itself — had no such guard. Under concurrent HMR invalidations, a transient task ID could reach this method via a code path that bypasses the caller-level checks, inserting the ID intoStorage::modifiedand causing the panic downstream during the next persist cycle.The flaky test is
test/development/app-dir/hmr-iframe/hmr-iframe.test.ts— it triggers two simultaneous server component file changes (one in an iframe, one in the parent), which races the persist cycle against concurrent invalidations.How?
Add a
debug_assert!inStorageWriteGuard::track_modification(the sole public entry point into the storage mutation path) that matches the invariant already checked at the caller layer:This enforces the invariant at the storage boundary in debug builds, surfacing the violation at the point of insertion rather than much later during the persist cycle. The message deliberately matches the existing
unreachable!downstream so both failure modes are easy to correlate.Verification:
cargo test -p turbo-tasks-backend— all unit tests passpnpm build-all— build succeedsNEXT_SKIP_ISOLATE=1 NEXT_TEST_MODE=dev pnpm testheadless test/development/app-dir/hmr-iframe/hmr-iframe.test.ts— 3/3 consecutive passes (was flaky before)