mirror of
https://github.com/hanzoai/next.js.git
synced 2026-10-30 14:46:38 +00:00
5d0dab5fc6
### What? Replaces the silent "make root node" promotion in the turbo-tasks backend with a panic that enforces tasks to already have the `root` attribute before performing strongly consistent reads, reading task collectibles, or removing collectibles. ### Why? Previously, the backend would silently promote any non-root task to a root node (aggregation number `u32::MAX`) when these operations were requested. This masked incorrect task configuration — tasks that needed root-level aggregation weren't explicitly declared as such, making it harder to reason about the aggregation graph and hiding potential performance issues from implicit promotions. ### How? **Backend enforcement (3 locations):** - **Strongly consistent read** (`backend/mod.rs`): Checks `NativeFunction.is_root` on the target task. If it's a persistent task without `root`, panics with both the target and reader task descriptions. - **Read task collectibles** (`backend/mod.rs`): Same check when reading collectibles from a task. - **Remove collectibles** (`operation/update_collectible.rs`): Same check when removing collectibles (count < 0). All three locations drop the task guard before panicking to avoid deadlocking with `debug_get_task_description`. **Macro support for `root` on methods:** - `value_impl_macro.rs`: Now reads the `root` attribute from `#[turbo_tasks::function(root)]` on inherent impl and trait impl methods (previously hardcoded to `false`). - `value_trait_macro.rs`: Same for trait default methods. **`root` attribute additions:** - All `#[turbo_tasks::function(operation)]` in test files, benchmarks, and fuzz code - Production operations in `crates/next-api/` that are read with strong consistency - Regular functions and methods in tests that use `.strongly_consistent()` - Trait methods in `value_impl` and `value_trait` blocks used with strong consistency **New test:** - `non_root_task_panic.rs`: Verifies the panic fires when attempting a strongly consistent read on a non-root operation task. Captures the panic from the worker thread via a panic hook since the panic propagates as a channel error to the test thread. <!-- NEXT_JS_LLM_PR --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
66 lines
2.1 KiB
Rust
66 lines
2.1 KiB
Rust
#![feature(arbitrary_self_types)]
|
|
#![feature(arbitrary_self_types_pointers)]
|
|
#![allow(clippy::needless_return)]
|
|
|
|
use anyhow::Result;
|
|
use turbo_tasks::Vc;
|
|
use turbo_tasks_testing::{Registration, register, run_once_without_cache_check};
|
|
|
|
static REGISTRATION: Registration = register!();
|
|
|
|
#[turbo_tasks::value]
|
|
#[derive(Clone, Debug)]
|
|
struct Value {
|
|
value: u32,
|
|
}
|
|
|
|
// NOT marked with `root` — this is the key
|
|
#[turbo_tasks::function(operation)]
|
|
async fn non_root_operation() -> Result<Vc<Value>> {
|
|
Ok(Value { value: 42 }.cell())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn test_strongly_consistent_read_of_non_root_task_panics() {
|
|
// The panic happens on a worker thread. Capture the message via a panic hook.
|
|
let panic_message = std::sync::Arc::new(std::sync::Mutex::new(None::<String>));
|
|
let panic_message_clone = panic_message.clone();
|
|
|
|
let prev_hook = std::panic::take_hook();
|
|
std::panic::set_hook(Box::new(move |info| {
|
|
let msg = if let Some(s) = info.payload().downcast_ref::<String>() {
|
|
s.clone()
|
|
} else if let Some(s) = info.payload().downcast_ref::<&str>() {
|
|
s.to_string()
|
|
} else {
|
|
format!("{info}")
|
|
};
|
|
if msg.contains("root") {
|
|
*panic_message_clone.lock().unwrap() = Some(msg);
|
|
}
|
|
}));
|
|
|
|
// Spawn to catch the unwinding panic from the channel close on the test task.
|
|
let handle = tokio::task::spawn(async move {
|
|
run_once_without_cache_check(®ISTRATION, async move {
|
|
non_root_operation().read_strongly_consistent().await
|
|
})
|
|
.await
|
|
});
|
|
// The spawned task will fail because the worker thread panics and the channel closes.
|
|
let _result = handle.await;
|
|
|
|
std::panic::set_hook(prev_hook);
|
|
|
|
let msg = panic_message.lock().unwrap().take();
|
|
assert!(
|
|
msg.is_some(),
|
|
"Expected a panic about missing `root` attribute on the worker thread"
|
|
);
|
|
let msg = msg.unwrap();
|
|
assert!(
|
|
msg.contains("root"),
|
|
"Panic message should mention 'root', got: {msg}"
|
|
);
|
|
}
|