fix some small bugs in turbo-tasks that are required for GC (#7764)

### Description * disable GC for tasks which use the streaming hack * invalidate dependent tasks when assigning to a recomputing cell ### Testing Instructions  Closes PACK-2787
vercel · Mar 22, 2024 · 48a2727 · 48a2727
1 parent 2cc8af0
commit 48a2727
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 6 deletions.
diff --git a/crates/turbo-tasks-memory/src/cell.rs b/crates/turbo-tasks-memory/src/cell.rs
@@ -220,9 +220,14 @@ impl Cell {
                 ref mut dependent_tasks,
             } => {
                 event.notify(usize::MAX);
+                // Assigning to a cell will invalidate all dependent tasks as the content might
+                // have changed.
+                if !dependent_tasks.is_empty() {
+                    turbo_tasks.schedule_notify_tasks_set(dependent_tasks);
+                }
                 *self = Cell::Value {
                     content,
-                    dependent_tasks: take(dependent_tasks),
+                    dependent_tasks: AutoSet::default(),
                 };
             }
             &mut Cell::TrackedValueless {

diff --git a/crates/turbo-tasks/src/lib.rs b/crates/turbo-tasks/src/lib.rs
@@ -87,7 +87,7 @@ pub use invalidation::{
 pub use join_iter_ext::{JoinIterExt, TryFlatJoinIterExt, TryJoinIterExt};
 pub use keyed_cell::{global_keyed_cell, keyed_cell};
 pub use manager::{
-    dynamic_call, emit, get_invalidator, mark_finished, mark_stateful, run_once,
+    dynamic_call, emit, get_invalidator, mark_finished, mark_stateful, prevent_gc, run_once,
     run_once_with_reason, spawn_blocking, spawn_thread, trait_call, turbo_tasks, CurrentCellRef,
     Invalidator, StatsType, TaskIdProvider, TurboTasks, TurboTasksApi, TurboTasksBackendApi,
     TurboTasksCallApi, Unused, UpdateInfo,

diff --git a/crates/turbo-tasks/src/manager.rs b/crates/turbo-tasks/src/manager.rs
@@ -1396,6 +1396,10 @@ pub fn mark_stateful() {
     })
 }
 
+pub fn prevent_gc() {
+    mark_stateful();
+}
+
 /// Notifies scheduled tasks for execution.
 pub fn notify_scheduled_tasks() {
     with_turbo_tasks(|tt| tt.notify_scheduled_tasks())

diff --git a/crates/turbopack-node/src/evaluate.rs b/crates/turbopack-node/src/evaluate.rs
@@ -13,8 +13,8 @@ use parking_lot::Mutex;
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use serde_json::Value as JsonValue;
 use turbo_tasks::{
-    duration_span, mark_finished, util::SharedError, Completion, RawVc, TaskInput, TryJoinIterExt,
-    Value, Vc,
+    duration_span, mark_finished, prevent_gc, util::SharedError, Completion, RawVc, TaskInput,
+    TryJoinIterExt, Value, Vc,
 };
 use turbo_tasks_bytes::{Bytes, Stream};
 use turbo_tasks_env::ProcessEnv;
@@ -243,6 +243,10 @@ pub trait EvaluateContext {
 }
 
 pub fn custom_evaluate(evaluate_context: impl EvaluateContext) -> Vc<JavaScriptEvaluation> {
+    // TODO: The way we invoke compute_evaluate_stream as side effect is not
+    // GC-safe, so we disable GC for this task.
+    prevent_gc();
+
     // Note the following code uses some hacks to create a child task that produces
     // a stream that is returned by this task.
 

diff --git a/crates/turbopack-node/src/render/render_proxy.rs b/crates/turbopack-node/src/render/render_proxy.rs
@@ -5,7 +5,9 @@ use futures::{
     pin_mut, SinkExt, StreamExt, TryStreamExt,
 };
 use parking_lot::Mutex;
-use turbo_tasks::{duration_span, mark_finished, util::SharedError, RawVc, ValueToString, Vc};
+use turbo_tasks::{
+    duration_span, mark_finished, prevent_gc, util::SharedError, RawVc, ValueToString, Vc,
+};
 use turbo_tasks_bytes::{Bytes, Stream};
 use turbo_tasks_env::ProcessEnv;
 use turbo_tasks_fs::FileSystemPath;
@@ -159,6 +161,10 @@ fn render_stream(
     body: Vc<Body>,
     debug: bool,
 ) -> Vc<RenderStream> {
+    // TODO: The way we invoke render_stream_internal as side effect is not
+    // GC-safe, so we disable GC for this task.
+    prevent_gc();
+
     // Note the following code uses some hacks to create a child task that produces
     // a stream that is returned by this task.
 

diff --git a/crates/turbopack-node/src/render/render_static.rs b/crates/turbopack-node/src/render/render_static.rs
@@ -5,7 +5,9 @@ use futures::{
     pin_mut, SinkExt, StreamExt, TryStreamExt,
 };
 use parking_lot::Mutex;
-use turbo_tasks::{duration_span, mark_finished, util::SharedError, RawVc, ValueToString, Vc};
+use turbo_tasks::{
+    duration_span, mark_finished, prevent_gc, util::SharedError, RawVc, ValueToString, Vc,
+};
 use turbo_tasks_bytes::{Bytes, Stream};
 use turbo_tasks_env::ProcessEnv;
 use turbo_tasks_fs::{File, FileSystemPath};
@@ -210,6 +212,10 @@ fn render_stream(
     data: Vc<RenderData>,
     debug: bool,
 ) -> Vc<RenderStream> {
+    // TODO: The way we invoke render_stream_internal as side effect is not
+    // GC-safe, so we disable GC for this task.
+    prevent_gc();
+
     // Note the following code uses some hacks to create a child task that produces
     // a stream that is returned by this task.