diff --git a/docs/02-pages/04-api-reference/04-config/01-next-config-js/turbopack.mdx b/docs/02-pages/04-api-reference/04-config/01-next-config-js/turbopack.mdx index 5b20170d44ae..6b56e41398ad 100644 --- a/docs/02-pages/04-api-reference/04-config/01-next-config-js/turbopack.mdx +++ b/docs/02-pages/04-api-reference/04-config/01-next-config-js/turbopack.mdx @@ -1,7 +1,6 @@ --- title: turbopack description: Configure Next.js with Turbopack-specific options -version: experimental source: app/api-reference/config/next-config-js/turbopack --- diff --git a/turbopack/crates/turbo-tasks-backend/Cargo.toml b/turbopack/crates/turbo-tasks-backend/Cargo.toml index 792a2c889cb0..40b923604ab0 100644 --- a/turbopack/crates/turbo-tasks-backend/Cargo.toml +++ b/turbopack/crates/turbo-tasks-backend/Cargo.toml @@ -22,6 +22,7 @@ verify_serialization = [] verify_aggregation_graph = [] verify_immutable = [] verify_determinism = ["turbo-tasks/verify_determinism"] +hanging_detection = ["turbo-tasks/hanging_detection"] task_dirty_cause = ["turbo-tasks/task_dirty_cause"] trace_aggregation_update_stats = [] trace_aggregation_update_queue = [] diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs index 55e7e04dc444..9fcfc535b8c4 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs @@ -79,7 +79,7 @@ use crate::{ ActivenessState, CellRef, CollectibleRef, CollectiblesRef, Dirtyness, InProgressCellState, InProgressState, InProgressStateInner, OutputValue, TransientTask, }, - error::TaskError, + error::{TaskError, TaskErrorItem}, kv_backing_storage::TurboBackingStorage, utils::{ dash_map_entry::{get_in_shard, get_shard, with_entry_in_shard}, @@ -1987,6 +1987,19 @@ impl TurboTasksBackend { } } + // Give the task a terminal output. `connect_children` treats a child with no output as + // "not computed yet" and marks it dirty to be scheduled, which for a canceled task means + // it stays a dirty container of its parent forever and any strongly consistent reader + // above it never settles. + task.set_output(OutputValue::Error(Arc::new(TaskError::Error(Box::new( + TaskErrorItem { + message: TurboTasksExecutionErrorMessage::PIISafe(std::borrow::Cow::Borrowed( + "task execution was canceled by shutdown", + )), + source: None, + }, + ))))); + // Mark the cancelled task as session-dependent dirty so it will be re-executed // in the next session. Without this, any reader that encounters the cancelled task // records an error in its output. That error is persisted and would poison diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs index a08ac1926761..3c1a332bb76a 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/mod.rs @@ -1624,16 +1624,37 @@ pub trait TaskGuard: Debug + TaskStorageAccessors { } } + /// A description of this task for diagnostics: `" "`. + /// + /// Is intentionally tolerant of non-resident data. fn get_task_desc_fn(&self) -> impl Fn() -> String + Send + Sync + 'static { - let task_type = self.get_task_type().to_owned(); + // Bypass `check_access`!! + // Generally it is a bad idea since accessing a `Data` field like get_persistence_task_type + // without opening the task that way is bug But this is for diagnostics and + // debugging purposes only so we can cheat. + let task_type = self + .typed() + .get_persistent_task_type() + .map(|task_type| TaskTypeRef::Cached(task_type).to_owned()) + .or_else(|| { + self.typed() + .get_transient_task_type() + .map(|task_type| TaskTypeRef::Transient(task_type).to_owned()) + }); + let task_id = self.id(); - move || format!("{task_id:?} {task_type}") + move || match &task_type { + Some(task_type) => format!("{task_id:?} {task_type}"), + None => format!("{task_id:?} task-type-not-available"), + } } + // Requires the task to have been opened with Data access fn get_task_description(&self) -> String { let task_type = self.get_task_type().to_owned(); let task_id = self.id(); format!("{task_id:?} {task_type}") } + #[cfg(feature = "trace_task_dirty")] fn get_task_name(&self) -> String { let task_type = self.get_task_type().to_owned(); diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/storage_schema.rs b/turbopack/crates/turbo-tasks-backend/src/backend/storage_schema.rs index 065f1a2813ef..0cb138e032a8 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/storage_schema.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/storage_schema.rs @@ -1789,8 +1789,11 @@ mod tests { // Schema Size Tests // ========================================================================== + // `hanging_detection` adds an `Arc String>` description to every `Event`, which + // grows `LazyField` past the size asserted here. The feature is diagnostic-only, so the sizes + // are not meaningful under it. #[test] - #[cfg(target_pointer_width = "64")] + #[cfg(all(target_pointer_width = "64", not(feature = "hanging_detection")))] fn test_schema_size() { assert_eq!( size_of::(), diff --git a/turbopack/crates/turbo-tasks-backend/tests/shutdown_cancellation.rs b/turbopack/crates/turbo-tasks-backend/tests/shutdown_cancellation.rs new file mode 100644 index 000000000000..a2931867443f --- /dev/null +++ b/turbopack/crates/turbo-tasks-backend/tests/shutdown_cancellation.rs @@ -0,0 +1,83 @@ +#![feature(arbitrary_self_types)] +#![allow(clippy::needless_return)] // tokio macro-generated code doesn't respect this + +//! Regression test for a bug where shutdown would cause a strongly consistent read to hang +//! if a task in scope was cancelled. + +mod util; + +use std::{ + sync::{Arc, LazyLock}, + time::Duration, +}; + +use anyhow::Result; +use tokio::sync::Barrier; +use turbo_tasks::{TurboTasks, Vc}; +use turbo_tasks_backend::TurboTasksBackend; + +use crate::util::create_tt; + +/// Rendezvous between the task and the test: the task waits here, the test joins once it is ready +/// to shut down, and both proceed. Two barriers rather than one so the test can shut down +/// *between* them, which is what puts the child's dispatch after `stopped` is set. +static PARKED: LazyLock = LazyLock::new(|| Barrier::new(2)); +static RELEASED: LazyLock = LazyLock::new(|| Barrier::new(2)); + +/// Launched only after shutdown has started, so it is cancelled before it executes. +#[turbo_tasks::function] +fn launched_during_shutdown() -> Vc { + Vc::cell(42) +} + +#[turbo_tasks::function(operation, root)] +async fn pauses_then_launches_a_child() -> Result> { + PARKED.wait().await; + RELEASED.wait().await; + // Dispatched after `stopped` is set, so this child is cancelled rather than run. Reading it + // must surface that rather than hang. + Ok(Vc::cell(*launched_during_shutdown().await?)) +} + +/// A strongly consistent read whose task launches a child during shutdown. +/// +/// The read is expected to fail — the child is cancelled. What must not happen is +/// `stop_and_wait` blocking forever on the reader's foreground job. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn shutdown_unblocks_read_of_canceled_child() { + let (tt, _persistence_dir) = create_tt("shutdown_unblocks_read_of_canceled_child"); + + let tt_reader: Arc> = tt.clone(); + let reader = tokio::spawn(async move { + let _ = turbo_tasks::run_once(tt_reader.clone(), async move { + // An error is the expected outcome once the child is cancelled; returning at all is + // the property under test. + let _ = pauses_then_launches_a_child() + .read_strongly_consistent() + .await; + anyhow::Ok(()) + }) + .await; + }); + + // Wait for the task to park, so shutdown lands between its two barriers. + PARKED.wait().await; + + let tt_stop = tt.clone(); + let stop = tokio::spawn(async move { tt_stop.stop_and_wait().await }); + // Let shutdown set `stopped` before releasing the task, so its child is dispatched into a + // stopping backend. The task then runs to completion — turbo-tasks does not cancel a task + // that is already executing — and only the cancelled child can strand the reader. + tokio::time::sleep(Duration::from_millis(50)).await; + RELEASED.wait().await; + + tokio::time::timeout(Duration::from_secs(30), stop) + .await + .expect("stop_and_wait hung: a cancelled task stranded a strongly consistent reader") + .unwrap(); + + tokio::time::timeout(Duration::from_secs(30), reader) + .await + .expect("the strongly consistent read never returned after shutdown") + .unwrap(); +}