Skip to content

turbo-tasks-backend: assert non-transient task_ids in track_modification#91924

Merged
lukesandberg merged 1 commit intocanaryfrom
sokra/flaky-test-8
Mar 26, 2026
Merged

turbo-tasks-backend: assert non-transient task_ids in track_modification#91924
lukesandberg merged 1 commit intocanaryfrom
sokra/flaky-test-8

Conversation

@sokra
Copy link
Copy Markdown
Member

@sokra sokra commented Mar 26, 2026

What?

Add a debug_assert! in StorageWriteGuard::track_modification to enforce that transient TaskIds are never inserted into the persistence-modified set.

Why?

Concurrent server component HMR updates triggered a runtime panic in Turbopack:

internal error: entered unreachable code: transient task_ids should never be enqueued to be persisted

This unreachable! lives in snapshot_and_persist (backend/mod.rs:1254), which is called during the persist cycle. It fires when a transient TaskId is found in Storage::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 with if !self.task_id.is_transient()
  • TaskGuardImpl::invalidate_serialization (operation/mod.rs:970) guards with if !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 into Storage::modified and 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! in StorageWriteGuard::track_modification (the sole public entry point into the storage mutation path) that matches the invariant already checked at the caller layer:

debug_assert!(
    !self.inner.key().is_transient(),
    "transient task_ids should never be enqueued to be persisted"
);

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 pass
  • pnpm build-all — build succeeds
  • NEXT_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)

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]>
@nextjs-bot nextjs-bot added created-by: Turbopack team PRs by the Turbopack team. Turbopack Related to Turbopack with Next.js. labels Mar 26, 2026
@sokra sokra requested a review from lukesandberg March 26, 2026 00:24
@sokra sokra marked this pull request as ready for review March 26, 2026 00:24
@nextjs-bot
Copy link
Copy Markdown
Collaborator

nextjs-bot commented Mar 26, 2026

Tests Passed

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 26, 2026

Merging this PR will not alter performance

✅ 17 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing sokra/flaky-test-8 (1daa46d) with canary (56993a6)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@lukesandberg lukesandberg merged commit 1693574 into canary Mar 26, 2026
281 of 292 checks passed
@lukesandberg lukesandberg deleted the sokra/flaky-test-8 branch March 26, 2026 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

created-by: Turbopack team PRs by the Turbopack team. Turbopack Related to Turbopack with Next.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants