next.js
3d2bab29 - [turbopack] fix a debug_assert failure (#92581)

Commit
19 days ago
[turbopack] fix a debug_assert failure (#92581) ## What? Fix a `debug_assert` failure in `SnapshotShardIter::next` and simplify the snapshot iterator control flow by removing impossible error cases. ## Why? **Bug:** When a task was modified before a snapshot started and then modified *again* between `take_snapshot`'s shard scan and the iterator consuming the task, the iterator hit: ``` debug_assert!(!inner.flags.any_modified(), "cannot already be modified") ``` The task was in the `modified` list (added during the scan when `data_modified=true`). After the scan, a concurrent modification triggered the `(true, true)` branch in `track_modification_internal`, which created a snapshot copy but didn't clear the live `data_modified` flag. When the iterator later checked `any_modified_during_snapshot()=true` and entered the snapshot-copy path, the assert fired because `data_modified` was still set on the live task. **Fix:** Move the clear of the live modified flags (`data_modified`, `meta_modified`, `new_task`) into `SnapshotShardIter::next`, right before encoding and promoting — the same place where all other flag cleanup happens. This also fixes a subtler consequence: `promote_during_snapshot_flags` only increments `shard_modified_counts` when `already_modified=false`, so leaving those flags set caused the during-snapshot modification to be silently dropped from the count, meaning the next snapshot cycle would have missed re-persisting the task. **Simplification:** The existing `is_empty()` error paths in the iterator assumed encoding could fail and return an empty `SnapshotItem`. Investigation shows this is impossible in practice: `TurboBincodeWriter` writes to a `SmallVec` and always returns `Ok(())`, and the only real failure mode — a `TypedSharedReference` with a type that has no bincode impl — represents a programmer bug (storing an unserializable type in persistent cell data). Encoding failures now `panic!` immediately with a clear message instead of silently retrying forever. This lets the iterator drop all error-handling branches and become straightforward `if let` / return sequences. ## How? - `track_modification_internal` (`storage.rs`): Remove the flag clears from the `(true, true)` branch. The snapshot copy still carries the modified bits so the iterator knows which categories to persist; the live task retains its flags until the iterator processes it. - `SnapshotShardIter::next` (`storage.rs`): Clear live flags and call `promote_during_snapshot_flags` unconditionally after encoding in all paths. `while` loops become `if let` since every branch now returns. Remove all `is_empty()` checks and error-recovery code. - `encode_category` (`mod.rs`): Replace `eprintln!` + `None` with `panic!` — encoding failure means a type without bincode support was stored in persistent cell data, which is a bug. - `SnapshotItem::is_empty` (`backing_storage.rs`): Removed, no longer used. - Add a regression test covering the exact race: modify → start snapshot → `take_snapshot` (scan) → modify again → consume iterator. Asserts correct item count, flag state, and `shard_modified_counts` after the cycle. <!-- NEXT_JS_LLM_PR -->
Author
Parents
Loading