next.js
deaef3cb - turbo-tasks-backend: stop pinning tasks during restores and always discard missing tasks (#99645)

Commit
38 days ago
turbo-tasks-backend: stop pinning tasks during restores and always discard missing tasks (#99645) ### What? When `task_pair` or `prepare_tasks` opens a task that is missing from disk (`MustExist`), or a restore hits an I/O error, it panics. They had already pinned the tasks involved (`transient_ref_count += 1`) and never released those pins. Because the missing task was still pinned, its entry also stayed in the map. Those pins weren't needed, so this removes them instead of making their release more careful: - `restore_task`, `prepare_tasks` and `task_pair` no longer pin a task while they use its id without holding its lock. - `handle_missing_task` always discards a missing task's entry. A missing task can't hold a transient reference, because those are only taken on tasks that exist; this is now a debug assertion. A thread still using the id creates a new entry, finds the task missing itself, and discards that entry too. - `task_pair` discards the entry it created for a missing transient endpoint before panicking. - `prepare_tasks` keeps handing off every task that does exist and reports missing tasks and I/O errors once at the end. - `prepare_tasks_with_callback` now documents that its callback must not panic. A panic there is a backend bug, expected to abort. One in Phase 1a can leave this batch's restoring claims set. ### Why? Previously I considered these panics to be fatal, so being careful about the pins leaking or the entries being discarded wasn't important. But these panics are sometimes caught by turbo-task functions. If the failure doesn't propagate then the session will complete and confuse the GC cycle later (since we have 'restored' a task that is completely blank). The pins don't protect anything: - **Outside a GC pass:** every `ExecuteContext` holds an operation guard, so GC can't run while a task id is held without its lock. - **Inside a GC pass,** where shard scans overlap with collects: - An edge that keeps the task alive (child, upper) also keeps it from being collected until that edge is removed. - A dependency edge doesn't keep it alive, but a concurrent collect is harmless then: edits during the cascade are idempotent removals, and edge additions are guarded by `assert_not_deleted`. - **Eviction** never drops a category that has a restoring claim, and the restore loop retries if a category was evicted while it waited. ### How? Unit tests in `operation/mod.rs` catch the missing-task panic and assert that no task stays pinned and no entry is left for the missing one: - `task_pair_missing_endpoint_leaves_no_pins` - `task_pair_missing_transient_endpoint_leaves_no_entry`, which fails without the fix - `prepare_tasks_missing_task_hands_off_the_rest_and_leaves_no_pins` The GC suites (`gc_stress`, `parent_count`, `gc_cross_session`, `gc_resurrection`, `gc_collection`, `gc_interrupt`, `detached`) passed 5 times in a row with debug assertions on.
Author
Parents
Loading