Repository: vercel/next.js
Pull request: fix(turbo-tasks): Add a few more transient-from-persistent task assertions

Transient tasks are tasks that depend on a value that cannot be serialized. Transient tasks and anything that depend on them therefore cannot be serialized. Because of this, it's not valid for persistent (serialized) tasks to depend on transient tasks.

We're seeing issues from transient collectibles being read from persistent tasks.

This should only be possible if a persistent task emits a transient collectible. That shouldn't be possible either (where would it get the transient collectible from?), but asserting doesn't seem like a bad idea, and might get us a stack trace closer to the root-cause.

Closes PACK-4428

Changed files:
filename: turbopack/crates/turbo-tasks-backend/src/backend/mod.rs
patch: @@ -1947,6 +1947,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
         task_id: TaskId,
         turbo_tasks: &dyn TurboTasksBackendApi<TurboTasksBackend<B>>,
     ) {
+        self.assert_valid_collectible(task_id, collectible);
         if !self.should_track_children() {
             return;
         }
@@ -1977,6 +1978,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
         task_id: TaskId,
         turbo_tasks: &dyn TurboTasksBackendApi<TurboTasksBackend<B>>,
     ) {
+        self.assert_valid_collectible(task_id, collectible);
         if !self.should_track_children() {
             return;
         }
@@ -2077,6 +2079,7 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
         parent_task: TaskId,
         turbo_tasks: &dyn TurboTasksBackendApi<TurboTasksBackend<B>>,
     ) {
+        self.assert_not_persistent_calling_transient(parent_task, task, None);
         ConnectChildOperation::run(parent_task, task, self.execute_context(turbo_tasks));
     }
 
@@ -2162,20 +2165,50 @@ impl<B: BackingStorage> TurboTasksBackendInner<B> {
         cell_id: Option<CellId>,
     ) {
         let transient_reason = if let Some(child) = child {
-            format!(
+            Cow::Owned(format!(
                 " The callee is transient because it depends on:\n{}",
                 self.debug_trace_transient_task(child, cell_id),
-            )
+            ))
         } else {
-            String::new()
+            Cow::Borrowed("")
         };
         panic!(
-            "Persistent task {} is not allowed to call or read transient tasks {}.{}",
+            "Persistent task {} is not allowed to call, read, or connect to transient tasks {}.{}",
             parent.map_or("unknown", |t| t.get_name()),
             child.map_or("unknown", |t| t.get_name()),
             transient_reason,
         );
     }
+
+    fn assert_valid_collectible(&self, task_id: TaskId, collectible: RawVc) {
+        // these checks occur in a potentially hot codepath, but they're cheap
+        let RawVc::TaskCell(col_task_id, col_cell_id) = collectible else {
+            // This should never happen: The collectible APIs use ResolvedVc
+            let task_info =
+                if let Some(col_task_ty) = self.lookup_task_type(collectible.get_task_id()) {
+                    Cow::Owned(format!(" (return type of {})", col_task_ty))
+                } else {
+                    Cow::Borrowed("")
+                };
+            panic!("Collectible{task_info} must be a ResolvedVc")
+        };
+        if col_task_id.is_transient() && !task_id.is_transient() {
+            let transient_reason =
+                if let Some(col_task_ty) = self.lookup_task_type(collectible.get_task_id()) {
+                    Cow::Owned(format!(
+                        ". The collectible is transient because it depends on:\n{}",
+                        self.debug_trace_transient_task(&col_task_ty, Some(col_cell_id)),
+                    ))
+                } else {
+                    Cow::Borrowed("")
+                };
+            // this should never happen: How would a persistent function get a transient Vc?
+            panic!(
+                "Collectible is transient, transient collectibles cannot be emitted from \
+                 persistent tasks{transient_reason}",
+            )
+        }
+    }
 }
 
 impl<B: BackingStorage> Backend for TurboTasksBackend<B> {

filename: turbopack/crates/turbo-tasks-backend/tests/transient_collectible.rs
patch: @@ -0,0 +1,52 @@
+#![feature(arbitrary_self_types)]
+#![feature(arbitrary_self_types_pointers)]
+
+use serde::{Deserialize, Serialize};
+use turbo_tasks::{trace::TraceRawVcs, NonLocalValue, ResolvedVc, TaskInput};
+use turbo_tasks_testing::{register, run_without_cache_check, Registration};
+
+static REGISTRATION: Registration = register!();
+
+const EXPECTED_MSG: &str =
+    "Collectible is transient, transient collectibles cannot be emitted from persistent tasks";
+
+#[tokio::test]
+async fn test_transient_emit_from_persistent() {
+    let result = run_without_cache_check(&REGISTRATION, async {
+        emit_incorrect_task_input_operation(IncorrectTaskInput(U32Wrapper(123).resolved_cell()))
+            .read_strongly_consistent()
+            .await?;
+        anyhow::Ok(())
+    })
+    .await;
+    assert!(result
+        .unwrap_err()
+        .to_string()
+        .contains(&EXPECTED_MSG.escape_debug().to_string()));
+}
+
+#[turbo_tasks::function(operation)]
+async fn emit_incorrect_task_input_operation(value: IncorrectTaskInput) {
+    turbo_tasks::emit(ResolvedVc::upcast::<Box<dyn Number>>(value.0));
+}
+
+/// Has an intentionally incorrect `TaskInput` implementation
+#[derive(
+    Copy, Clone, Debug, PartialEq, Eq, Hash, TraceRawVcs, Serialize, Deserialize, NonLocalValue,
+)]
+struct IncorrectTaskInput(ResolvedVc<U32Wrapper>);
+
+impl TaskInput for IncorrectTaskInput {
+    fn is_transient(&self) -> bool {
+        false
+    }
+}
+
+#[turbo_tasks::value_trait]
+trait Number {}
+
+#[turbo_tasks::value]
+struct U32Wrapper(u32);
+
+#[turbo_tasks::value_impl]
+impl Number for U32Wrapper {}