diff --git a/.alfonso/release-notes/next/callgraph-read-bound.md b/.alfonso/release-notes/next/callgraph-read-bound.md new file mode 100644 index 0000000000..2d5f3db8e5 --- /dev/null +++ b/.alfonso/release-notes/next/callgraph-read-bound.md @@ -0,0 +1 @@ +With views on, a call-graph read no longer waits for the whole call graph to rebuild after files change; it answers "building" within a few seconds, and the next read uses the new graph. diff --git a/benchmarks/aft-search/slice-descriptors/land-callgraph-read-bound.json b/benchmarks/aft-search/slice-descriptors/land-callgraph-read-bound.json new file mode 100644 index 0000000000..27e9e44660 --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-callgraph-read-bound.json @@ -0,0 +1,8 @@ +{ + "fixtures": [ + "harness-goldens" + ], + "kind": "harness", + "slice_class": "non_ranking", + "targeted_mechanism": "none" +} diff --git a/crates/aft/src/callgraph_store/join.rs b/crates/aft/src/callgraph_store/join.rs index f7a1d64ee6..eb51428a56 100644 --- a/crates/aft/src/callgraph_store/join.rs +++ b/crates/aft/src/callgraph_store/join.rs @@ -51,6 +51,7 @@ const TOP_LEVEL_SYMBOL: &str = ""; /// An error raised while decoding or assembling manifest-addressed callgraph data. #[derive(Clone, Debug, Eq, PartialEq)] pub enum ManifestJoinError { + Cancelled, UnsupportedLanguage(String), Parse(String), InvalidBlob(String), @@ -61,6 +62,7 @@ pub enum ManifestJoinError { impl fmt::Display for ManifestJoinError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + Self::Cancelled => formatter.write_str(crate::views::first_load::SUPERSEDED_PUBLISH), Self::UnsupportedLanguage(language) => { write!(formatter, "unsupported callgraph blob language {language}") } @@ -2352,6 +2354,9 @@ fn join_manifest_with_surfaces( #[cfg(test)] let mut heap_parts = [0isize; 5]; for (path, entry) in manifest.entries() { + if crate::executor::current_job_cancelled() { + return Err(ManifestJoinError::Cancelled); + } let ManifestEntry::Regular { planes, .. } = entry else { continue; }; @@ -2557,6 +2562,9 @@ fn join_manifest_with_surfaces( .collect(); let mut resolved_bindings = 0; for caller in &resolved_callers { + if crate::executor::current_job_cancelled() { + return Err(ManifestJoinError::Cancelled); + } let path = RelPath::new(caller.as_bytes().to_vec()).expect("bound manifest path"); let Some(ManifestEntry::Regular { planes, .. }) = manifest.get(&path) else { continue; diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index f5865c0c2b..fca71a88d1 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -12671,7 +12671,7 @@ mod tests { let _scope = crate::executor::install_standalone_scope(Arc::clone(&ctx)); barrier.wait(); matches!( - ctx.callgraph_store_for_ops(), + ctx.with_ready_callgraph_for_test(|| ctx.callgraph_store_for_ops()), CallgraphStoreAccess::Ready(_) ) }) @@ -12685,7 +12685,11 @@ mod tests { .filter(|event| *event == crate::views::donor::DonorEvent::Sealed) .count(); assert_eq!(sealed, 1, "one base graph build for both queries"); - let store = lazy_ready(fixture.borrower.callgraph_store_for_ops()); + let store = lazy_ready( + fixture + .borrower + .with_ready_callgraph_for_test(|| fixture.borrower.callgraph_store_for_ops()), + ); assert_eq!(store.reader_kind(), "view"); let (_cold_storage, cold) = lazy_cold_store(&fixture.linked); lazy_assert_same_graph(&store, &cold); @@ -12699,7 +12703,9 @@ mod tests { ) -> Arc { let access = { let _scope = crate::executor::install_standalone_scope(Arc::clone(&fixture.borrower)); - fixture.borrower.callgraph_store_for_ops() + fixture + .borrower + .with_ready_callgraph_for_test(|| fixture.borrower.callgraph_store_for_ops()) }; let store = lazy_ready(access); assert_eq!(store.reader_kind(), "view"); @@ -12930,9 +12936,11 @@ mod tests { lsp_hints: None, params: json!({"file": root.join("tracked.rs"), "symbol": "dirty"}), }; - let response = crate::commands::callers::handle_callers(&request, &ctx); - // The checkout view verifies the working tree for the query itself, so - // the dirty source answers at once, without waiting for publication. + let response = ctx.with_ready_callgraph_for_test(|| { + crate::commands::callers::handle_callers(&request, &ctx) + }); + // The fixture waits for a call-graph reader verified against the + // working tree, not the stale configure-publication snapshot. assert!(response.success, "{response:?}"); assert_eq!(response.data["total_callers"], 1); } @@ -13138,7 +13146,9 @@ mod tests { super::drain_deferred_configure_maintenance(&ctx); assert!(ctx.views_route_root()); - let graph_has = |file: &str, symbol: &str| match ctx.callgraph_store_for_ops() { + let graph_has = |file: &str, symbol: &str| match ctx + .with_ready_callgraph_for_test(|| ctx.callgraph_store_for_ops()) + { CallgraphStoreAccess::Ready(store) => { crate::callgraph_store::CallGraphRead::node_for(&store, Path::new(file), symbol) .is_ok() @@ -14742,7 +14752,7 @@ mod tests { // Stopping the writer alone does not bypass the pending timer. assert!(!request().success); ctx.view_load_retry_due_for_test(); - let recovered = request(); + let recovered = ctx.with_ready_callgraph_for_test(request); assert!(recovered.success, "{:?}", recovered.data); assert!(ctx.try_health_snapshot(&root).view_load_failures.is_empty()); } @@ -15183,7 +15193,7 @@ mod tests { }; let before = ctx.callgraph_view_route_count(); let expected = callgraph_views_answers(&oracle, ©_root); - let actual = callgraph_views_answers(ctx, root); + let actual = ctx.with_ready_callgraph_for_test(|| callgraph_views_answers(ctx, root)); for ((label, expected), (_, actual)) in expected.iter().zip(&actual) { assert!(expected.success, "legacy {label}: {:?}", expected.data); assert!(actual.success, "view {label}: {:?}", actual.data); @@ -15370,6 +15380,490 @@ mod tests { ); } + fn assert_callgraph_views_rematerialization_bounded(command: &str, daemon: bool) { + let _git_env = crate::test_env::hermetic_git_env_guard(); + let _serial = crate::blob_store::v3::serial_for_test(); + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("project"); + init_git_fixture(&root); + std::fs::write( + root.join("a.ts"), + "export function target() { return 1; }\n", + ) + .unwrap(); + let write_callers = |prefix: &str| { + for index in 0..64 { + std::fs::write( + root.join(format!("caller_{index}.ts")), + format!("import {{ target }} from './a';\nexport function {prefix}_{index}() {{ return target(); }}\n"), + ) + .unwrap(); + } + assert!(git_command(&root) + .args(["add", "."]) + .status() + .unwrap() + .success()); + assert!(git_command(&root) + .args([ + "-c", + "user.name=AFT Tests", + "-c", + "user.email=aft-tests@example.com", + "commit", + "--quiet", + "-m", + prefix + ]) + .status() + .unwrap() + .success()); + }; + write_callers("before"); + let root = std::fs::canonicalize(&root).unwrap(); + let ctx = Arc::new(AppContext::new( + Box::new(TreeSitterProvider::new()), + Config { + storage_dir: Some(temp.path().join("storage")), + ..Config::default() + }, + )); + let _executor = daemon.then(|| { + let executor = Arc::new(crate::executor::Executor::with_config( + crate::executor::ExecutorConfig { + pool_size: 3, + actor_cap: 2, + read_cap: 2, + heavy_permits: 1, + drr_quantum: 1, + }, + )); + executor.register_actor( + crate::path_identity::ProjectRootId::from_path(&root).unwrap(), + ctx.clone(), + ); + executor + }); + assert!( + handle_configure_for_test( + &configure_callgraph_views(&root, &temp.path().join("storage"), true), + &ctx, + ) + .success + ); + let callers_request: RawRequest = serde_json::from_value(json!({ + "id": "rematerialization-callers", "command": "callers", + "file": root.join("a.ts"), "symbol": "target", "depth": 1, + })) + .unwrap(); + let initial = ctx.with_ready_callgraph_for_test(|| { + crate::commands::callers::handle_callers(&callers_request, &ctx) + }); + assert!(initial.success, "{:?}", initial.data); + assert_eq!(initial.data["total_callers"], 64); + assert!(initial.data.to_string().contains("before_")); + write_callers("after"); + + // Hold the actual materializer until the request has answered. A timeout + // also releases it so a regression cannot leave a blocked test thread. + let (entered_tx, entered_rx) = crossbeam_channel::unbounded(); + let (release_tx, release_rx) = crossbeam_channel::bounded(1); + let (published_tx, published_rx) = crossbeam_channel::unbounded(); + let _hook = crate::storage_retention::observe_test_hook( + &root, + Arc::new(move |step| { + if step == "callgraph-materialize" { + entered_tx.send(()).unwrap(); + release_rx.recv_timeout(Duration::from_secs(30)).unwrap(); + } else if step == "callgraph-reader-acquired" { + let _ = published_tx.send(()); + } + }), + ); + let request = if command == "zoom" { + serde_json::from_value(json!({ + "id": "rematerialization-zoom", "command": "zoom", "file": root.join("a.ts"), + "symbols": ["target"], "callgraph": true, + })) + .unwrap() + } else { + callers_request.clone() + }; + let (answer_tx, answer_rx) = crossbeam_channel::unbounded(); + let spawn_read = || { + let worker_ctx = ctx.clone(); + let request = request.clone(); + let answer_tx = answer_tx.clone(); + let executor = _executor.clone(); + let root_id = crate::path_identity::ProjectRootId::from_path(&root).unwrap(); + std::thread::spawn(move || { + let started = Instant::now(); + let id = request.id.clone(); + let zoom = request.command == "zoom"; + let run = move |ctx: &AppContext| { + let _pin = ctx.pin_config(); + if zoom { + crate::commands::zoom::handle_zoom(&request, ctx) + } else { + crate::commands::callers::handle_callers(&request, ctx) + } + }; + let response = if let Some(executor) = executor { + executor + .submit_tool_call_with_cancellation_async( + root_id, + if zoom { + crate::executor::Lane::PureRead + } else { + crate::executor::Lane::HeavyInit + }, + id, + if zoom { "zoom" } else { "callgraph" }, + Box::new(run), + crate::executor::JobCancellation::new(), + ) + .blocking_recv() + .unwrap() + } else { + run(&worker_ctx) + }; + answer_tx.send((started.elapsed(), response)).unwrap(); + }) + }; + let worker = spawn_read(); + let bound = crate::views::contracts::CALLGRAPH_QUERY_WAIT; + let entered = entered_rx.recv_timeout(Duration::from_secs(10)); + // A reader queued behind the first read gets the same total budget, + // not another full wait after the first read releases the query lock. + let concurrent = spawn_read(); + let deadline = Instant::now() + bound + Duration::from_secs(1); + let first = answer_rx.recv_timeout(deadline.saturating_duration_since(Instant::now())); + let second = answer_rx.recv_timeout(deadline.saturating_duration_since(Instant::now())); + release_tx.send(()).unwrap(); + worker.join().unwrap(); + concurrent.join().unwrap(); + assert!(entered.is_ok(), "request never reached materialization"); + for bounded_answer in [first, second] { + let answered_in_bound = bounded_answer.is_ok(); + let (elapsed, answer) = bounded_answer.unwrap_or_else(|_| answer_rx.recv().unwrap()); + eprintln!("{command}: changed_files=64 read_ms={} bound_ms={} answered_before_release={answered_in_bound}", elapsed.as_millis(), bound.as_millis()); + assert!( + answered_in_bound, + "{command} waited {elapsed:?} for materialization past {bound:?}" + ); + if command == "zoom" { + assert!(answer.success, "{:?}", answer.data); + assert!(answer.data.to_string().contains("export function target")); + assert_eq!(answer.data["callgraph"]["code"], "callgraph_building"); + } else { + assert!(!answer.success, "{:?}", answer.data); + assert_eq!(answer.data["code"], "callgraph_building"); + } + } + + published_rx.recv_timeout(Duration::from_secs(10)).unwrap(); + let started = Instant::now(); + let next = ctx.with_ready_callgraph_for_test(|| { + crate::commands::callers::handle_callers(&callers_request, &ctx) + }); + eprintln!("{command}: next_read_ms={}", started.elapsed().as_millis()); + assert!(next.success, "{:?}", next.data); + assert_eq!(next.data["total_callers"], 64); + assert!(next.data.to_string().contains("after_")); + assert!(!next.data.to_string().contains("before_")); + assert!( + entered_rx.try_recv().is_err(), + "the next read started another materialization" + ); + } + + #[test] + fn callgraph_views_rematerialization_bounds_callers_then_serves_publication() { + assert_callgraph_views_rematerialization_bounded("callers", false); + } + + #[test] + fn callgraph_views_rematerialization_bounds_zoom_then_serves_publication() { + assert_callgraph_views_rematerialization_bounded("zoom", false); + } + + #[test] + fn callgraph_views_rematerialization_bounds_daemon_callers_then_serves_publication() { + assert_callgraph_views_rematerialization_bounded("callers", true); + } + + #[test] + fn callgraph_views_rematerialization_bounds_daemon_zoom_then_serves_publication() { + assert_callgraph_views_rematerialization_bounded("zoom", true); + } + + fn assert_callgraph_acquisition_lifecycle( + daemon: bool, + stop_executor: bool, + late_register: bool, + ) { + use crate::executor::{Executor, ExecutorConfig, Lane}; + let _git_env = crate::test_env::hermetic_git_env_guard(); + let _serial = crate::blob_store::v3::serial_for_test(); + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("project"); + init_git_fixture(&root); + std::fs::write( + root.join("a.ts"), + "export function target() { return 1; }\n", + ) + .unwrap(); + let root = std::fs::canonicalize(root).unwrap(); + let storage = temp.path().join("storage"); + let ctx = Arc::new(AppContext::new( + Box::new(TreeSitterProvider::new()), + Config { + storage_dir: Some(storage.clone()), + ..Config::default() + }, + )); + let root_id = crate::path_identity::ProjectRootId::from_path(&root).unwrap(); + let mut executor = daemon.then(|| { + Executor::with_config(ExecutorConfig { + pool_size: 2, + actor_cap: 1, + read_cap: 1, + heavy_permits: 1, + drr_quantum: 1, + }) + }); + if let Some(executor) = executor.as_ref().filter(|_| !late_register) { + executor.register_actor(root_id.clone(), ctx.clone()); + } + assert!( + handle_configure_for_test(&configure_callgraph_views(&root, &storage, true), &ctx) + .success + ); + let request: RawRequest = serde_json::from_value(json!({ + "id": "lifecycle-callers", "command": "callers", "file": root.join("a.ts"), + "symbol": "target", "depth": 1, + })) + .unwrap(); + assert!( + ctx.with_ready_callgraph_for_test(|| crate::commands::callers::handle_callers( + &request, &ctx + )) + .success + ); + if let Some(executor) = executor.as_ref().filter(|_| late_register) { + executor.register_actor(root_id.clone(), ctx.clone()); + } + let family = crate::search_index::artifact_cache_key(&root); + let directory = crate::blob_store::v3::view_dir( + &storage, + &family, + &crate::views::callgraph::view_scope(&root), + ) + .unwrap(); + let store = crate::views::ViewStore::existing_dir(directory.clone()).unwrap(); + let previous = store.current_generation_read_only().unwrap(); + assert!(previous.is_some()); + std::fs::write( + root.join("b.ts"), + "import { target } from './a';\nexport function caller() { return target(); }\n", + ) + .unwrap(); + + let (entered_tx, entered_rx) = crossbeam_channel::bounded(1); + let (release_tx, release_rx) = crossbeam_channel::bounded(1); + let (finished_tx, finished_rx) = crossbeam_channel::unbounded(); + let _hook = crate::storage_retention::observe_test_hook( + &root, + Arc::new(move |step| { + if step == "callgraph-reserved" { + entered_tx.send(()).unwrap(); + let cancellation = crate::executor::current_job_cancellation(); + let deadline = Instant::now() + Duration::from_secs(10); + loop { + if cancellation + .as_ref() + .is_some_and(|token| token.cancel_already_requested()) + { + break; + } + if release_rx.recv_timeout(Duration::from_millis(10)).is_ok() { + break; + } + assert!( + Instant::now() < deadline, + "materializer was neither released nor cancelled" + ); + } + } else if step == "callgraph-reader-acquired" + || step == "callgraph-acquisition-failed" + { + let _ = finished_tx.send(()); + } + }), + ); + let (answer_tx, answer_rx) = crossbeam_channel::bounded(1); + let worker_ctx = ctx.clone(); + let request_again = request.clone(); + let read = std::thread::spawn(move || { + let _pin = worker_ctx.pin_config(); + answer_tx + .send(crate::commands::callers::handle_callers( + &request, + &worker_ctx, + )) + .unwrap(); + }); + entered_rx.recv_timeout(Duration::from_secs(10)).unwrap(); + let reserved_during_build = + crate::blob_store::v3::LedgerReader::open_existing(&storage, &family) + .unwrap() + .accounting() + .unwrap() + .reserved_bytes; + + let interactive = executor.as_ref().map(|executor| { + let file = root.join("a.ts"); + executor + .submit( + root_id.clone(), + Lane::PureRead, + "same-root-read".into(), + Box::new(move |_| { + crate::protocol::Response::success( + "same-root-read", + json!({"text": std::fs::read_to_string(file).unwrap()}), + ) + }), + ) + .recv_timeout(Duration::from_millis(500)) + }); + let accounted = executor.as_ref().map(|executor| { + let deadline = Instant::now() + Duration::from_millis(500); + let jobs = loop { + let jobs = executor.try_root_job_snapshots(&root_id, None); + if jobs.is_some() || Instant::now() >= deadline { + break jobs; + } + std::thread::sleep(Duration::from_millis(1)); + }; + ( + jobs, + executor + .try_dispatch_liveness_snapshot() + .unwrap() + .running + .maintenance, + ) + }); + let shutdown_started = Instant::now(); + if stop_executor { + // Assigning drops the executor, as taking it would. + executor = None; + } else if let Some(executor) = &executor { + ctx.mark_subc_unbound(); + executor.cancel_root_maintenance(&root_id); + } else { + ctx.shutdown_callgraph_acquisition(); + } + let cancelled_answer = answer_rx.recv_timeout(Duration::from_secs(4)); + // If cancellation fails, release the blocked graph builder before + // assertions so it cannot keep running or prevent fixture teardown. + let _ = release_tx.send(()); + read.join().unwrap(); + let prompt = cancelled_answer.is_ok(); + let answer = cancelled_answer.unwrap_or_else(|_| answer_rx.recv().unwrap()); + finished_rx.recv_timeout(Duration::from_secs(10)).unwrap(); + if stop_executor { + ctx.view_load_retry_due_for_test(); + let refused = crate::commands::callers::handle_callers(&request_again, &ctx); + assert_eq!(refused.data["code"], "callgraph_building"); + assert!( + entered_rx.try_recv().is_err(), + "a stopped daemon executor fell back to a standalone worker" + ); + } + if let Some(executor) = &executor { + let deadline = Instant::now() + Duration::from_secs(2); + while !executor.actor_is_idle(&root_id) { + assert!( + Instant::now() < deadline, + "cancelled maintenance did not drain" + ); + std::thread::sleep(Duration::from_millis(1)); + } + executor.remove_actor(&root_id); + } + drop(ctx); + assert_eq!( + store.current_generation_read_only().unwrap(), + previous, + "a quiesced root published a generation" + ); + assert!( + prompt, + "shutdown did not settle the acquisition before manual release" + ); + assert!( + shutdown_started.elapsed() < Duration::from_secs(2), + "acquisition shutdown did not drain promptly" + ); + assert!( + reserved_during_build > 0, + "the fixture did not hold a real v3 reservation" + ); + assert!(!answer.success, "{:?}", answer.data); + assert_eq!(answer.data["code"], "callgraph_building"); + if let Some(interactive) = interactive { + assert!( + interactive.unwrap().success, + "same-root read stalled behind materialization" + ); + let (jobs, running) = accounted.unwrap(); + let jobs = jobs.unwrap(); + assert!( + jobs.iter().any(|job| job.lane == Lane::HeavyInit), + "acquisition was not executor heavy work: {jobs:?}" + ); + assert_eq!(running, 1, "acquisition was not executor maintenance"); + } + let reserved = crate::blob_store::v3::LedgerReader::open_existing(&storage, &family) + .unwrap() + .accounting() + .unwrap() + .reserved_bytes; + assert_eq!(reserved, 0); + let pins = directory.join("pins"); + if pins.exists() { + for entry in std::fs::read_dir(pins).unwrap() { + let name = entry.unwrap().file_name().to_string_lossy().into_owned(); + assert!( + !name.starts_with("staging-") && !name.starts_with("live-"), + "unreleased acquisition pin: {name}" + ); + } + } + } + + #[test] + fn callgraph_views_acquisition_quiesce_fences_publication_and_preserves_reads() { + assert_callgraph_acquisition_lifecycle(true, false, false); + } + + #[test] + fn callgraph_views_acquisition_standalone_shutdown_releases_protection() { + assert_callgraph_acquisition_lifecycle(false, false, false); + } + + #[test] + fn callgraph_views_acquisition_executor_shutdown_drains_protection() { + assert_callgraph_acquisition_lifecycle(true, true, false); + } + + #[test] + fn callgraph_views_acquisition_adopts_executor_after_standalone_warmup() { + assert_callgraph_acquisition_lifecycle(true, false, true); + } + #[test] fn callgraph_views_planned_load_states_retry_without_failure_or_footer() { let _git_env = crate::test_env::hermetic_git_env_guard(); @@ -15410,7 +15904,7 @@ mod tests { handle_configure_for_test(&configure_callgraph_views(&root, &storage, true), &ctx) .success ); - let ready = match ctx.callgraph_store_for_ops() { + let ready = match ctx.with_ready_callgraph_for_test(|| ctx.callgraph_store_for_ops()) { CallgraphStoreAccess::Ready(store) => store, _ => panic!("fixture must have a ready checkout graph"), }; @@ -15462,7 +15956,7 @@ mod tests { assert!(health.view_load_failure_last.is_none()); source.verified.store(true, Ordering::SeqCst); ctx.view_load_retry_due_for_test(); - let answer = callers(); + let answer = ctx.with_ready_callgraph_for_test(callers); assert!(answer.success, "{:?}", answer.data); assert!(answer.data.to_string().contains("caller"), "{:?}", answer.data); assert_eq!(source.attempts.load(Ordering::SeqCst), attempts + 1); @@ -15471,6 +15965,67 @@ mod tests { } } + #[test] + fn callgraph_views_typed_planned_errors_skip_failures_but_real_errors_count() { + let _git_env = crate::test_env::hermetic_git_env_guard(); + struct Refused(crate::views::callgraph::CallgraphViewError); + impl crate::views::callgraph::CallgraphReaderSource for Refused { + fn acquire( + &self, + ) -> Result< + Arc, + crate::views::callgraph::CallgraphViewError, + > { + Err(self.0.clone()) + } + fn record_change(&self, _: &Path) {} + } + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("project"); + init_git_fixture(&root); + let root = std::fs::canonicalize(root).unwrap(); + let ctx = test_context(); + assert!( + handle_configure_for_test( + &configure_callgraph_views(&root, &temp.path().join("storage"), true), + &ctx + ) + .success + ); + for (busy, building) in [(true, false), (false, true), (false, false)] { + // A reason that looks planned must not override a typed real failure. + let reason = if busy || building { + "opaque typed fixture error" + } else { + "legacy text and symbol import building" + }; + ctx.install_callgraph_source_for_test(Arc::new(Refused( + crate::views::callgraph::CallgraphViewError { + reason: reason.into(), + busy, + building, + }, + ))); + let before = ctx.callgraph_view_read_diagnostics().attempts; + let access = ctx.callgraph_store_for_ops(); + if building { + assert!(matches!(access, CallgraphStoreAccess::Building)); + } else { + assert!(matches!(access, CallgraphStoreAccess::Error(_))); + } + let diagnostics = ctx.callgraph_view_read_diagnostics(); + assert_eq!(diagnostics.attempts, before + 1); + assert_eq!(diagnostics.last_error.as_deref(), Some(reason)); + assert!(diagnostics.total_attempt_ms >= diagnostics.last_attempt_ms); + let health = ctx.try_health_snapshot(&root); + assert_eq!( + health.view_load_failures.values().copied().sum::(), + if busy || building { 0 } else { 1 } + ); + assert_eq!(health.view_load_failure_last.is_some(), !busy && !building); + } + } + #[test] fn callgraph_view_load_failure_fails_every_graph_op_and_counts() { let reporting_suffix = "\nThis is an AFT bug: tell the user to report it with `npx @cortexkit/aft doctor --issue`; don't run it yourself."; @@ -15684,7 +16239,7 @@ mod tests { ctx, ) }; - let answer = callers(&ctx); + let answer = ctx.with_ready_callgraph_for_test(|| callers(&ctx)); assert!(answer.success, "{:?}", answer.data); assert!(ctx.callgraph_view_route_count() > 0); @@ -15704,7 +16259,7 @@ mod tests { true, ); let _ = crate::executor::view_publication::schedule(&ctx, BTreeSet::new(), true); - let answer = callers(&ctx); + let answer = ctx.with_ready_callgraph_for_test(|| callers(&ctx)); assert!(answer.success, "{:?}", answer.data); assert_eq!(legacy_views_on_writes(&storage), Vec::::new()); assert_eq!(ctx.view_publication_attempts_for_test(), 0); @@ -15837,7 +16392,7 @@ mod tests { ) }; assert!(grep().success); - assert!(callers().success); + assert!(ctx.with_ready_callgraph_for_test(callers).success); assert!(ctx.lexical_view_is_ready() && ctx.callgraph_view_is_ready()); evict(&ctx); @@ -15849,7 +16404,7 @@ mod tests { let answer = grep(); assert!(answer.success, "{:?}", answer.data); assert_eq!(answer.data["index_status"], "Ready", "{:?}", answer.data); - let answer = callers(); + let answer = ctx.with_ready_callgraph_for_test(callers); assert!(answer.success, "{:?}", answer.data); assert_eq!( ( diff --git a/crates/aft/src/commands/configure/views_import_tests.rs b/crates/aft/src/commands/configure/views_import_tests.rs index d743f98b7c..1d33d25bf8 100644 --- a/crates/aft/src/commands/configure/views_import_tests.rs +++ b/crates/aft/src/commands/configure/views_import_tests.rs @@ -346,7 +346,8 @@ fn views_import_building_retries_callgraph_without_load_failures_then_answers() assert!(Instant::now() < deadline, "legacy import did not finish"); thread::sleep(Duration::from_millis(10)); } - let zoom = crate::commands::zoom::handle_zoom(&zoom_request, &ctx); + let zoom = ctx + .with_ready_callgraph_for_test(|| crate::commands::zoom::handle_zoom(&zoom_request, &ctx)); assert!(zoom.success, "{:?}", zoom.data); assert!(zoom.data.get("callgraph").is_none(), "{:?}", zoom.data); let health = ctx.try_health_snapshot(&root); diff --git a/crates/aft/src/context.rs b/crates/aft/src/context.rs index 8ea305f1c5..a0233836f5 100644 --- a/crates/aft/src/context.rs +++ b/crates/aft/src/context.rs @@ -4809,6 +4809,7 @@ pub struct AppContext { lexical_load_retry: parking_lot::Mutex>, lexical_routes: [AtomicU64; 3], callgraph_view: parking_lot::Mutex>>, + callgraph_executor: parking_lot::Mutex>, /// Set while this checkout is over `views.callgraph_copies` and reads /// another checkout's published call graph instead of building its own. callgraph_borrow: parking_lot::Mutex>, @@ -5142,6 +5143,11 @@ impl Drop for ForceRestrictGuard<'_> { impl Drop for AppContext { fn drop(&mut self) { + self.subc_lifecycle + .mark_unbound(self.configure_generation.as_ref()); + if let Some(view) = self.callgraph_view.get_mut().take() { + view.shutdown(); + } self.artifact_owner_lease.get_mut().take(); if let Some(runtime) = self.watcher_thread.get_mut().take() { let root = self @@ -5161,17 +5167,54 @@ impl Drop for AppContext { } } +struct CallgraphPublicationFence { + lifecycle: SubcLifecycleAdmission, + generation: Arc, + expected: u64, +} + +impl crate::views::first_load::PublishFence for CallgraphPublicationFence { + fn publish( + &self, + commit: &mut dyn FnMut() -> Result<(), crate::views::contracts::PlaneError>, + ) -> Result<(), crate::views::contracts::PlaneError> { + let refused = || crate::views::contracts::PlaneError { + plane: Some(crate::blob_store::v2::FamilyPlane::Callgraph), + reason: crate::views::first_load::SUPERSEDED_PUBLISH.into(), + }; + loop { + if crate::executor::current_job_cancelled() { + return Err(refused()); + } + // A root's bound/unbound lock may be held while retiring its view. + // A cancelled worker must leave this wait so retirement can join it. + let Some(unbound) = self + .lifecycle + .unbound + .try_lock_for(Duration::from_millis(10)) + else { + continue; + }; + if *unbound + || self.generation.load(Ordering::Acquire) != self.expected + || crate::executor::current_job_cancelled() + { + return Err(refused()); + } + return commit(); + } + } +} + /// Result of requesting the persisted callgraph store for a store-backed op. /// -/// The five edge-query ops never block the request thread on a cold build: -/// a genuine cold build is kicked off in the background and `Building` is -/// returned so the agent retries, mirroring how semantic search reports a -/// build in progress. Warm restarts open the on-disk DB synchronously, so -/// `Building` is only ever seen during a true first cold build. +/// Cold builds run in the background. Checkout-view acquisitions also run +/// off the request thread, with a bounded wait before returning `Building` +/// so the agent can retry while materialization continues. pub enum CallgraphStoreAccess { /// Store is resident and queryable. Ready(Arc), - /// A cold build is in flight (or was just started); retry shortly. + /// A build or checkout-view acquisition is in flight; retry shortly. Building, /// The durable build-death breaker refuses this domain until an explicit reset /// or a time-to-live check confirms the suspension has expired. @@ -5192,7 +5235,7 @@ pub(crate) fn checkout_view_callgraph_access( ) -> CallgraphStoreAccess { match reader { Some(Ok(store)) => CallgraphStoreAccess::Ready(store), - Some(Err(error)) if error.completion_busy => CallgraphStoreAccess::Building, + Some(Err(error)) if error.building => CallgraphStoreAccess::Building, // Pending verification and transient contention say nothing about the // graph's validity. Use the existing busy error, which every caller // presents as busy/pending with a retry. @@ -5752,6 +5795,7 @@ impl AppContext { lexical_load_retry: parking_lot::Mutex::new(Default::default()), lexical_routes: Default::default(), callgraph_view: parking_lot::Mutex::new(None), + callgraph_executor: parking_lot::Mutex::new(None), callgraph_borrow: parking_lot::Mutex::new(None), callgraph_construction_retry: parking_lot::Mutex::new(Default::default()), callgraph_view_read_diagnostics: parking_lot::Mutex::new( @@ -7088,6 +7132,7 @@ impl AppContext { pub(crate) fn mark_subc_unbound(&self) { self.subc_lifecycle .mark_unbound(self.configure_generation.as_ref()); + self.cancel_callgraph_acquisition(); let _ = self .subc_lifecycle .run_if_unbound(|| self.callgraph_corpus_memo.lock().take()); @@ -10025,6 +10070,14 @@ impl AppContext { /// A measured disk-reserve refusal is recorded separately, without adding /// to the bug count or replacing the most recent genuine failure. pub(crate) fn note_view_load_failure(&self, plane: &'static str, error: &str) { + self.note_classified_view_load_failure( + plane, + error, + crate::views::trigram::bind_is_busy(error), + ); + } + + fn note_classified_view_load_failure(&self, plane: &'static str, error: &str, planned: bool) { if let Some(condition) = crate::view_storage_reserve::DiskReserveCondition::observed( &self.storage_paths().artifacts, error, @@ -10035,7 +10088,7 @@ impl AppContext { .insert(plane.to_string(), condition); return; } - if crate::views::trigram::bind_is_busy(error) { + if planned { return; } let cause = view_load_cause(error); @@ -10241,6 +10294,7 @@ impl AppContext { } pub(crate) fn clear_view_runtime(&self) { + self.cancel_callgraph_acquisition(); self.release_view_publication_cache(); if let Some(root) = self.canonical_cache_root_opt() { crate::cold_build_limiter::progress::cancel_root(&root); @@ -12952,7 +13006,7 @@ impl AppContext { Err((reason, busy)) => Err(crate::views::callgraph::CallgraphViewError { reason, busy, - completion_busy: false, + building: false, }), }, ) @@ -13002,6 +13056,42 @@ impl AppContext { Arc::new(move || lifecycle.is_bound() && generation.load(Ordering::Acquire) == expected) } + pub(crate) fn install_callgraph_executor( + &self, + executor: crate::executor::RootMaintenanceExecutor, + ) { + *self.callgraph_executor.lock() = Some(executor); + let retired = self.callgraph_view.lock().take(); + if let Some(view) = retired { + view.shutdown(); + } + } + + fn cancel_callgraph_acquisition(&self) { + if let Some(view) = self.callgraph_view.lock().as_ref() { + view.cancel(); + } + } + + /// Fence and drain standalone call-graph acquisition before process teardown. + /// Daemon acquisitions are also cancelled, and their executor owns the drain. + pub fn shutdown_callgraph_acquisition(&self) { + self.mark_subc_unbound(); + let view = self.callgraph_view.lock().take(); + if let Some(view) = view { + view.shutdown(); + } + } + + fn callgraph_view_control(&self) -> crate::migration::v3_ledger::ImportControl { + let unbound = self.subc_lifecycle.unbound_hint.clone(); + let generation = self.configure_generation_flag(); + let expected = generation.load(Ordering::Acquire); + Arc::new(move || { + !unbound.load(Ordering::Acquire) && generation.load(Ordering::Acquire) == expected + }) + } + pub(crate) fn callgraph_view_is_ready(&self) -> bool { self.callgraph_view .lock() @@ -13047,6 +13137,35 @@ impl AppContext { } } + /// Test-only fixture adapter: block until the view's acquisition completes, + /// then pin its reader for the read-only test request (up to 120 seconds). + /// Production callers must handle Building through the normal read APIs, + /// whose interactive acquisition wait remains three seconds. + #[doc(hidden)] + pub fn with_ready_callgraph_for_test(&self, read: impl FnOnce() -> T) -> T { + if !self.callgraph_views_enabled() { + return read(); + } + let deadline = Instant::now() + Duration::from_secs(120); + let _pin = self.pin_config(); + CALLGRAPH_QUERY_READERS.with(|readers| { + readers + .borrow_mut() + .retain(|(key, _)| *key != self.config_pin_key()); + }); + self.callgraph_view_reader_with(|view| { + Ok(view.ready_reader_before_for_test(Instant::now, deadline)) + }) + .expect("fixture must route through callgraph views") + .unwrap_or_else(|error| { + panic!( + "fixture callgraph root={} failed before acquisition: {error}", + self.canonical_cache_root().display() + ) + }); + read() + } + /// The call-graph reader of this checkout's view, acquired once per /// pinned request. `None` means views do not route call-graph reads; a /// genuine acquisition error is final. Capacity borrows the same published @@ -13067,6 +13186,19 @@ impl AppContext { fn callgraph_view_reader_inner( &self, ) -> Option, crate::views::callgraph::CallgraphViewError>> + { + self.callgraph_view_reader_with(|view| view.reader()) + } + + fn callgraph_view_reader_with( + &self, + acquire: impl FnOnce( + &crate::views::callgraph::CallgraphView, + ) -> Result< + Arc, + crate::views::callgraph::CallgraphViewError, + >, + ) -> Option, crate::views::callgraph::CallgraphViewError>> { if !self.callgraph_views_enabled() { return None; @@ -13105,8 +13237,8 @@ impl AppContext { } match &borrowed { Ok(_) => self.note_view_load_success("callgraph"), - Err(error) if !error.busy => { - self.note_view_load_failure("callgraph", &error.reason) + Err(error) if !error.is_planned() => { + self.note_classified_view_load_failure("callgraph", &error.reason, false) } Err(_) => {} } @@ -13128,12 +13260,20 @@ impl AppContext { storage.clone(), family.clone(), policy, - self.views_import_control(), + self.callgraph_view_control(), self.cold_build_limiter(), ) }, ) { - Ok(view) => *slot = Some(Arc::new(view)), + Ok(mut view) => { + view.set_executor(self.callgraph_executor.lock().clone()); + view.set_publish_fence(Arc::new(CallgraphPublicationFence { + lifecycle: self.subc_lifecycle_admission(), + generation: self.configure_generation_flag(), + expected: self.configure_generation(), + })); + *slot = Some(Arc::new(view)); + } Err(error) => { drop(slot); if error.reason.contains(crate::views::trigram::BASE_CAP_BUSY) { @@ -13142,9 +13282,12 @@ impl AppContext { ) { match &borrowed { Ok(_) => self.note_view_load_success("callgraph"), - Err(error) if !error.busy => { - self.note_view_load_failure("callgraph", &error.reason) - } + Err(error) if !error.is_planned() => self + .note_classified_view_load_failure( + "callgraph", + &error.reason, + false, + ), Err(_) => {} } return Some(borrowed); @@ -13152,7 +13295,13 @@ impl AppContext { } // Like the lexical view, a failed callgraph load fails // the query: no legacy store may answer for the view. - self.note_view_load_failure("callgraph", &error.reason); + if !error.is_planned() { + self.note_classified_view_load_failure( + "callgraph", + &error.reason, + false, + ); + } return Some(Err(error)); } } @@ -13160,7 +13309,7 @@ impl AppContext { slot.as_ref().unwrap().clone() }; self.record_checkout_pending_watcher_gap(); - let result = match view.reader() { + let result = match acquire(&view) { Err(error) if error.reason.contains(crate::views::trigram::BASE_CAP_BUSY) => self .borrowed_callgraph_view_reader(&root, &storage, &family, policy, true) .unwrap_or(Err(error)), @@ -13177,7 +13326,9 @@ impl AppContext { self.callgraph_view_routes.fetch_add(1, Ordering::Relaxed); self.note_view_load_success("callgraph"); } else if let Err(error) = &result { - self.note_view_load_failure("callgraph", &error.reason); + if !error.is_planned() { + self.note_classified_view_load_failure("callgraph", &error.reason, false); + } } Some(result) } @@ -19689,7 +19840,7 @@ mod callgraph_store_for_ops_tests { let root = ctx.callgraph_project_root().unwrap(); ctx.set_canonical_cache_root(root.clone()); assert!(matches!( - ctx.callgraph_store_for_ops(), + ctx.with_ready_callgraph_for_test(|| ctx.callgraph_store_for_ops()), CallgraphStoreAccess::Ready(_) )); ctx.callgraph_view.lock().take(); @@ -19732,7 +19883,7 @@ mod callgraph_store_for_ops_tests { drop(blocker); ctx.view_load_retry_due_for_test(); assert!(matches!( - ctx.callgraph_store_for_ops(), + ctx.with_ready_callgraph_for_test(|| ctx.callgraph_store_for_ops()), CallgraphStoreAccess::Ready(_) )); let ready = ctx.callgraph_view_read_diagnostics(); @@ -21911,7 +22062,9 @@ mod callgraph_store_for_ops_tests { #[test] fn views_changed_head_callers_answer_the_current_tree_not_the_stale_legacy_store() { let fixture = ViewsCallgraphFixture::new(); - let before = fixture.callers(); + let before = fixture + .ctx + .with_ready_callgraph_for_test(|| fixture.callers()); assert!(before.success, "{:#}", before.data); assert_eq!(before.data["total_callers"], 1, "{:#}", before.data); fixture.commit_source( @@ -21928,7 +22081,9 @@ mod callgraph_store_for_ops_tests { .expect("stale legacy callers"); assert_eq!(stale.total_callers, 1, "legacy fixture must remain stale"); - let response = fixture.callers(); + let response = fixture + .ctx + .with_ready_callgraph_for_test(|| fixture.callers()); assert!(response.success, "{:#}", response.data); assert_eq!( @@ -21946,7 +22101,7 @@ mod callgraph_store_for_ops_tests { /// query must open the database, and waits for the deferred checkpoint /// so only the returned connection holds the file. fn hold_checkout_view_derived_busy(&self) -> rusqlite::Connection { - let bound = self.callers(); + let bound = self.ctx.with_ready_callgraph_for_test(|| self.callers()); assert!(bound.success, "{:#}", bound.data); self.ctx .update_config(|config| config.views.enabled = false); @@ -22035,7 +22190,9 @@ mod callgraph_store_for_ops_tests { ); drop(busy); fixture.ctx.view_load_retry_due_for_test(); - let response = fixture.callers(); + let response = fixture + .ctx + .with_ready_callgraph_for_test(|| fixture.callers()); assert!(response.success, "{:#}", response.data); } @@ -22097,7 +22254,7 @@ mod callgraph_store_for_ops_tests { drop(busy); fixture.ctx.view_load_retry_due_for_test(); - let response = inspect(); + let response = fixture.ctx.with_ready_callgraph_for_test(inspect); assert!(response.success, "{:#}", response.data); assert!( response.data["summary"]["dead_code"]["count"] @@ -22157,7 +22314,7 @@ mod callgraph_store_for_ops_tests { // Once the database is free again the same zoom reads the graph. drop(busy); fixture.ctx.view_load_retry_due_for_test(); - let response = zoom(); + let response = fixture.ctx.with_ready_callgraph_for_test(zoom); assert!(response.success, "{:#}", response.data); assert!( response.data.get("callgraph").is_none(), diff --git a/crates/aft/src/executor/mod.rs b/crates/aft/src/executor/mod.rs index 079437f71f..ea0670198f 100644 --- a/crates/aft/src/executor/mod.rs +++ b/crates/aft/src/executor/mod.rs @@ -50,6 +50,7 @@ const SCHEDULER_PROBE_WAIT: Duration = Duration::from_millis(5); /// between batches. const SCHEDULER_EVENT_BATCH_CAP: usize = 64; pub(crate) const MAINTENANCE_QUEUE_CAP: usize = 512; +const CALLGRAPH_ACQUISITION_REQUEST_ID: &str = "callgraph-view-acquire"; /// Scheduler lane for command-handler execution. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -1138,6 +1139,58 @@ pub struct Executor { inner: Arc, } +/// A root may submit maintenance without keeping its owning executor alive. +/// The weak link avoids a cycle through the executor's actor context. +#[derive(Clone)] +pub(crate) struct RootMaintenanceExecutor { + inner: std::sync::Weak, + root_id: ProjectRootId, +} + +struct SubmissionExecutor(Option); + +impl Drop for SubmissionExecutor { + fn drop(&mut self) { + let executor = self.0.take(); + if current_thread_is_executor_worker() { + // Shutdown may leave this temporary as the last executor owner. + // Other workers can depend on this worker returning, so release + // the temporary off-worker rather than joining them here. + std::thread::spawn(move || drop(executor)); + } + } +} + +impl RootMaintenanceExecutor { + pub(crate) fn submit_callgraph_acquisition( + &self, + cancellation: JobCancellation, + job: ExecutorJob, + ) -> bool { + let Some(inner) = self.inner.upgrade() else { + return false; + }; + let executor = SubmissionExecutor(Some(Executor { inner })); + let (tx, _rx) = oneshot::channel(); + executor + .0 + .as_ref() + .unwrap() + .submit_with_completion_cancellable( + self.root_id.clone(), + JobClass::Maintenance, + Lane::HeavyInit, + CALLGRAPH_ACQUISITION_REQUEST_ID.into(), + job, + CompletionSender::Async(tx), + Some(cancellation), + None, + None, + ); + true + } +} + impl Executor { pub fn new() -> Self { Self::with_config(ExecutorConfig::default()) @@ -1226,6 +1279,10 @@ impl Executor { /// new actor was inserted. pub fn register_actor(&self, root_id: ProjectRootId, ctx: Arc) -> bool { ctx.bind_status_context(); + let maintenance = RootMaintenanceExecutor { + inner: Arc::downgrade(&self.inner), + root_id: root_id.clone(), + }; let memory_root = root_id.as_path().to_path_buf(); let inserted = { let mut state = self.inner.state.lock(); @@ -1240,6 +1297,7 @@ impl Executor { } }; if inserted { + ctx.install_callgraph_executor(maintenance); let app = ctx.app(); crate::root_cache::register_live_scope(&ctx.storage_dir(), &memory_root); app.register_memory_context(memory_root, &ctx); @@ -1607,6 +1665,28 @@ impl Executor { job: ExecutorJob, cancellation: JobCancellation, ) -> oneshot::Receiver { + // Checkout callgraph views rebuild as Maintenance/HeavyInit jobs. + // Admit their tool reads as PureRead so a one-permit pool can answer + // Building while the rebuild owns the heavy permit. + let lane = if lane == Lane::HeavyInit + && matches!( + tool, + "callgraph" + | "callers" + | "call_tree" + | "impact" + | "trace_to" + | "trace_to_symbol" + | "trace_data" + ) + && self + .actor_context(&root_id) + .is_some_and(|ctx| ctx.callgraph_views_enabled()) + { + Lane::PureRead + } else { + lane + }; let (completion_tx, completion_rx) = oneshot::channel(); self.submit_labeled( root_id, @@ -2286,6 +2366,21 @@ struct ExecutorInner { impl Drop for ExecutorInner { fn drop(&mut self) { + // Stop maintenance before joining workers. Queued closures must also + // release their captured builders instead of surviving a stopped loop. + { + let mut state = self.state.lock(); + for job in state.running_jobs.values() { + if job.job_class == JobClass::Maintenance { + if let Some(token) = &job.cancellation { + token.request_cancel(); + } + } + } + for actor in state.actors.values_mut() { + actor.maintenance.cancel_queued_jobs(); + } + } let _ = self.event_tx.send(SchedulerEvent::Shutdown); if let Some(handle) = self.scheduler_handle.lock().take() { @@ -2646,6 +2741,12 @@ fn is_bind_job(job_class: JobClass, lane: Lane, request_id: &str) -> bool { job_class == JobClass::Interactive && lane == Lane::Mutating && is_configure_request(request_id) } +fn is_callgraph_acquisition(job_class: JobClass, lane: Lane, request_id: &str) -> bool { + job_class == JobClass::Maintenance + && lane == Lane::HeavyInit + && request_id == CALLGRAPH_ACQUISITION_REQUEST_ID +} + fn format_bind_blocker_reader(reader: &BindBlockerReaderSnapshot) -> String { format!( "job={} command={} lane={:?} age_ms={} execution_started={} started_before_oldest_writer={}", @@ -2720,6 +2821,8 @@ struct ActorState { /// `current_bind_gate_is_shared`). shared_bind_inflight: bool, maintenance_commit_inflight: bool, + /// Only the internal callgraph acquisition bypasses interactive capacity. + callgraph_acquisition_inflight: bool, provider_inflight: bool, mutating_inflight: Option, reader_admissions_while_promoted_writer_waited: u64, @@ -2748,6 +2851,7 @@ impl ActorState { writer_inflight: false, shared_bind_inflight: false, maintenance_commit_inflight: false, + callgraph_acquisition_inflight: false, provider_inflight: false, mutating_inflight: None, reader_admissions_while_promoted_writer_waited: 0, @@ -3565,7 +3669,11 @@ fn complete_job(state: &mut SchedulerState, event: CompletionEvent) -> Option { actor.lsp_inflight = false; } - Lane::HeavyInit => {} + Lane::HeavyInit => { + if is_callgraph_acquisition(job_class, lane, &request_id) { + actor.callgraph_acquisition_inflight = false; + } + } Lane::Mutating => { actor.writer_inflight = false; actor.shared_bind_inflight = false; @@ -4014,6 +4122,11 @@ fn try_admit_actor( AdmissionPass::Class(JobClass::Maintenance) => actor.front_lane(job_class)?, }; let mut heavy_permit = None; + let callgraph_acquisition = actor + .class_queues(job_class) + .queue(lane) + .front() + .is_some_and(|job| is_callgraph_acquisition(job_class, lane, &job.request_id)); // A bind that runs with a shared hold (see `current_bind_gate_is_shared`) // blocks other writers and maintenance like any running writer, but @@ -4054,7 +4167,7 @@ fn try_admit_actor( .interactive .first_bind_job() .is_some_and(|job| job.rerun.is_some() && !job.force_exclusive); - // MaintenanceCommit uses the executor's global maintenance capacity and + // MaintenanceCommit and callgraph acquisition use global maintenance capacity and // must not consume the per-actor slots reserved for interactive work. This // matters when a two-worker executor has actor_cap=1: the global reserve // leaves one worker idle, and this accounting makes that worker reachable @@ -4062,13 +4175,19 @@ fn try_admit_actor( let actor_interactive_inflight = actor .actor_total_inflight .saturating_sub(usize::from(actor.maintenance_commit_inflight)) + .saturating_sub(usize::from(actor.callgraph_acquisition_inflight)) .saturating_sub(usize::from(actor.provider_inflight)); let actor_has_interactive_capacity = actor_interactive_inflight < config.actor_cap; let runnable = match lane { Lane::PureRead => actor.read_inflight < config.read_cap && actor_has_interactive_capacity, Lane::SerialLspStatus => !actor.lsp_inflight && actor_has_interactive_capacity, Lane::HeavyInit => { - if !actor_has_interactive_capacity { + let capacity = if callgraph_acquisition { + !actor.callgraph_acquisition_inflight + } else { + actor_has_interactive_capacity + }; + if !capacity { false } else if let Some(permit) = heavy.try_acquire() { heavy_permit = Some(permit); @@ -4140,6 +4259,9 @@ fn try_admit_actor( actor.actor_total_inflight += 1; } Lane::HeavyInit => { + if callgraph_acquisition { + actor.callgraph_acquisition_inflight = true; + } actor.actor_total_inflight += 1; } Lane::Mutating => { diff --git a/crates/aft/src/executor/tests.rs b/crates/aft/src/executor/tests.rs index ee6a109b81..a5b0b80ab5 100644 --- a/crates/aft/src/executor/tests.rs +++ b/crates/aft/src/executor/tests.rs @@ -315,6 +315,126 @@ fn last_executor_owner_can_drop_on_worker() { assert!(recv_async(response, "last owner drop").success); } +#[test] +fn callgraph_submit_last_owner_returns_before_dependent_worker() { + let executor = test_executor(3, 2, 2, 1); + let (_a_dir, a_root) = test_root("callgraph-last-submit"); + let (_b_dir, b_root) = test_root("callgraph-dependent-worker"); + executor.register_actor(a_root.clone(), test_ctx()); + executor.register_actor(b_root.clone(), test_ctx()); + let submitter = RootMaintenanceExecutor { + inner: Arc::downgrade(&executor.inner), + root_id: a_root.clone(), + }; + let (upgraded_tx, upgraded_rx) = crossbeam_channel::bounded(1); + let (release_tx, release_rx) = crossbeam_channel::bounded(1); + *executor.inner.maintenance_root_check_hook.lock() = Some(Box::new(move || { + upgraded_tx.send(()).unwrap(); + release_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + })); + let (peer_started_tx, peer_started_rx) = crossbeam_channel::bounded(1); + let (after_submit_tx, after_submit_rx) = crossbeam_channel::bounded(1); + let (abort_tx, abort_rx) = crossbeam_channel::bounded(1); + let peer = executor.submit( + b_root, + Lane::PureRead, + "dependent-worker".into(), + Box::new(move |_| { + peer_started_tx.send(()).unwrap(); + crossbeam_channel::select! { + recv(after_submit_rx) -> _ => {}, + recv(abort_rx) -> _ => {}, + default(Duration::from_secs(5)) => panic!("dependent worker was not released"), + } + ok("dependent-worker") + }), + ); + peer_started_rx + .recv_timeout(Duration::from_secs(2)) + .unwrap(); + let (returned_tx, returned_rx) = crossbeam_channel::bounded(1); + let caller = executor.submit( + a_root, + Lane::PureRead, + "last-owner-submit".into(), + Box::new(move |_| { + assert!(submitter + .submit_callgraph_acquisition(JobCancellation::new(), Box::new(|_| ok("acquire")))); + returned_tx.send(()).unwrap(); + let _ = after_submit_tx.send(()); + ok("last-owner-submit") + }), + ); + upgraded_rx.recv_timeout(Duration::from_secs(2)).unwrap(); + // The paused submitter owns the only other strong executor reference. Its + // worker must return before the dependent worker can finish. + drop(executor); + release_tx.send(()).unwrap(); + let returned_before_abort = returned_rx.recv_timeout(Duration::from_millis(500)).is_ok(); + let _ = abort_tx.send(()); + assert!(peer.recv_timeout(Duration::from_secs(5)).unwrap().success); + assert!(caller.recv_timeout(Duration::from_secs(5)).unwrap().success); + assert!( + returned_before_abort, + "worker-side submission joined its dependent worker during last-owner drop" + ); +} + +#[test] +fn callgraph_maintenance_preserves_other_heavy_overlap() { + let executor = test_executor(6, 3, 4, 3); + let (_dir, root) = test_root("callgraph-heavy-coexistence"); + executor.register_actor(root.clone(), test_ctx()); + let submitter = RootMaintenanceExecutor { + inner: Arc::downgrade(&executor.inner), + root_id: root.clone(), + }; + let (graph_started_tx, graph_started_rx) = crossbeam_channel::bounded(1); + let (graph_release_tx, graph_release_rx) = crossbeam_channel::bounded(1); + assert!(submitter.submit_callgraph_acquisition( + JobCancellation::new(), + Box::new(move |_| { + graph_started_tx.send(()).unwrap(); + graph_release_rx + .recv_timeout(Duration::from_secs(5)) + .unwrap(); + ok("callgraph-acquisition") + }) + )); + graph_started_rx + .recv_timeout(Duration::from_secs(2)) + .unwrap(); + let (started_tx, started_rx) = crossbeam_channel::unbounded(); + let (release_tx, release_rx) = crossbeam_channel::unbounded(); + let mut replies = Vec::new(); + for number in 0..2 { + let started_tx = started_tx.clone(); + let release_rx = release_rx.clone(); + replies.push(executor.submit_maintenance_async( + root.clone(), + Lane::HeavyInit, + format!("other-heavy-{number}"), + Box::new(move |_| { + started_tx.send(()).unwrap(); + release_rx.recv_timeout(Duration::from_secs(5)).unwrap(); + ok("other-heavy") + }), + )); + } + let first = started_rx.recv_timeout(Duration::from_millis(500)); + let second = started_rx.recv_timeout(Duration::from_millis(500)); + graph_release_tx.send(()).unwrap(); + release_tx.send(()).unwrap(); + release_tx.send(()).unwrap(); + for reply in replies { + assert!(recv_async(reply, "other heavy maintenance").success); + } + assert!( + first.is_ok() && second.is_ok(), + "unrelated heavy maintenance was serialized behind callgraph acquisition" + ); +} + fn observe_max(max_seen: &AtomicUsize, value: usize) { let mut current = max_seen.load(Ordering::Acquire); while value > current { diff --git a/crates/aft/src/main.rs b/crates/aft/src/main.rs index 87a14e6596..bc643c2343 100644 --- a/crates/aft/src/main.rs +++ b/crates/aft/src/main.rs @@ -1001,6 +1001,7 @@ fn main() { aft::slog_info!("shutdown phase=runtime_cleanup_start"); let mut clients = Vec::new(); for runtime in registry.iter() { + runtime.shutdown_callgraph_acquisition(); clients.extend(runtime.lsp().take_all_clients()); runtime.bash_background().detach(); } diff --git a/crates/aft/src/migration/views_background.rs b/crates/aft/src/migration/views_background.rs index 7554cd4d94..872a8aa4d8 100644 --- a/crates/aft/src/migration/views_background.rs +++ b/crates/aft/src/migration/views_background.rs @@ -12,6 +12,7 @@ use crate::blob_store::v2::TrigramPolicy; pub(crate) struct BackgroundImport { done: Arc, + completed: crossbeam_channel::Receiver<()>, } impl BackgroundImport { @@ -30,8 +31,10 @@ impl BackgroundImport { { return None; } + let (completed_tx, completed_rx) = crossbeam_channel::bounded(1); let job = Arc::new(Self { done: Arc::new(AtomicBool::new(false)), + completed: completed_rx, }); let weak = Arc::downgrade(&job); let keep_going: ImportControl = Arc::new(move || weak.upgrade().is_some() && current()); @@ -51,6 +54,7 @@ impl BackgroundImport { crate::log_ctx::with_session(session, || { run(&request, keep_going, limiter); done.store(true, Ordering::Release); + let _ = completed_tx.send(()); }); }); if let Err(error) = spawned { @@ -63,6 +67,16 @@ impl BackgroundImport { pub(crate) fn is_building(&self) -> bool { !self.done.load(Ordering::Acquire) } + + pub(crate) fn wait_until_for_test(&self, deadline: std::time::Instant) -> bool { + if !self.is_building() { + return true; + } + let _ = self + .completed + .recv_timeout(deadline.saturating_duration_since(std::time::Instant::now())); + !self.is_building() + } } fn run( diff --git a/crates/aft/src/views/callgraph.rs b/crates/aft/src/views/callgraph.rs index 337c2699fe..d8c9f4657b 100644 --- a/crates/aft/src/views/callgraph.rs +++ b/crates/aft/src/views/callgraph.rs @@ -14,6 +14,8 @@ //! QueryWait for CALLGRAPH_QUERY_WAIT (the bounded three-second wait from //! `views::contracts`). Use its final installed snapshot and //! disclose timeout gap paths; never substitute legacy/newer mutable state. +//! Checkout-local acquisition uses the same budget, leaving its single +//! materialization worker running after a timed-out read answers Building. use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; @@ -475,14 +477,21 @@ impl PlaneAdapter for CallgraphPlane { pub(crate) struct CallgraphViewError { pub reason: String, pub busy: bool, - pub completion_busy: bool, + /// A materialization is still running, either here or under another owner. + pub building: bool, +} + +impl CallgraphViewError { + pub(crate) fn is_planned(&self) -> bool { + self.busy || self.building + } } impl From for CallgraphViewError { fn from(reason: String) -> Self { Self { busy: super::trigram::bind_is_busy(&reason), - completion_busy: reason.contains(&format!("{V3_BASE_COMPLETION_BUSY}: Busy {{")), + building: reason.contains(&format!("{V3_BASE_COMPLETION_BUSY}: Busy {{")), reason, } } @@ -507,7 +516,7 @@ impl From for CallgraphViewError { || reason.starts_with(V3_STORE_BUSY) || super::trigram::bind_is_busy(reason) || completion_busy, - completion_busy, + building: completion_busy, reason: error.to_string(), } } @@ -536,6 +545,7 @@ const V3_BASE_COMPLETION_BUSY: &str = "v3 base plane completion is owned elsewhe /// never substitute another store when acquisition fails. pub(crate) trait CallgraphReaderSource: Send + Sync { fn acquire(&self) -> Result, CallgraphViewError>; + fn set_publish_fence(&self, _: Arc) {} /// `acquire`, plus the source revision the reader's cut belongs to when /// the source tracks one. The view keeps the reader under it. fn acquire_with_revision( @@ -581,6 +591,9 @@ pub(super) fn has_unpublished_changes(snapshot: &Snapshot) -> bool { } impl CallgraphReaderSource for CheckoutCallgraphSource { + fn set_publish_fence(&self, fence: Arc) { + self.driver.set_publish_fence(fence); + } fn acquire(&self) -> Result, CallgraphViewError> { self.acquire_with_revision().map(|(store, _)| store) } @@ -593,6 +606,9 @@ impl CallgraphReaderSource for CheckoutCallgraphSource { &self, ) -> Result<(Arc, Option), CallgraphViewError> { use super::first_load::FirstLoadDriver; + if crate::executor::current_job_cancelled() { + return Err(graph_building()); + } let installed = self.driver.installed_snapshot(); // After a restart, start from this checkout's own published worker // generation; verification still reconciles it against the disk. @@ -609,7 +625,7 @@ impl CallgraphReaderSource for CheckoutCallgraphSource { return Err(CallgraphViewError { reason: error.to_string(), busy: true, - completion_busy: false, + building: false, }); } Err(error) => return Err(error.to_string().into()), @@ -621,6 +637,9 @@ impl CallgraphReaderSource for CheckoutCallgraphSource { .driver .verified_bind(&self.access, seed) .map_err(CallgraphViewError::from)?; + if crate::executor::current_job_cancelled() { + return Err(graph_building()); + } if has_unpublished_changes(&snapshot) { // Publish the edits as a new worker generation (a donor clone // patched with the diff) before the single install, as a fresh @@ -653,6 +672,9 @@ impl CallgraphReaderSource for CheckoutCallgraphSource { .with_install_cut(cut.clone()) .with_reconciled_epoch(snapshot.reconciled_epoch().unwrap_or(cut.epoch)); } + if crate::executor::current_job_cancelled() { + return Err(graph_building()); + } self.driver .install(&self.access, &snapshot, revision) .map_err(CallgraphViewError::from)?; @@ -726,7 +748,9 @@ pub(crate) struct CallgraphView { family: String, policy: crate::blob_store::v2::TrigramPolicy, source: Arc, - query: std::sync::Mutex>, + query: parking_lot::Mutex>, + acquisition: std::sync::Mutex>>, + owner: super::callgraph_acquisition::AcquisitionOwner, /// The last reader with the source revision of the cut it reads. Taken /// only under `query`. A query whose source still reports that revision /// as steady reuses the reader instead of binding again. @@ -739,6 +763,17 @@ pub(crate) struct CallgraphView { current: crate::migration::v3_ledger::ImportControl, } +pub(super) type ReaderAcquisition = + Result<(Arc, Option), CallgraphViewError>; + +pub(super) fn graph_building() -> CallgraphViewError { + CallgraphViewError { + reason: "callgraph_building".into(), + busy: true, + building: true, + } +} + /// The v3 member scope of a checkout's call-graph view. pub(crate) fn view_scope(root: &Path) -> String { format!( @@ -759,6 +794,7 @@ impl CallgraphView { && self.storage == *storage && self.family == family && self.policy == policy + && !self.owner.stopped() && (self.current)() } @@ -791,7 +827,7 @@ impl CallgraphView { return Err(CallgraphViewError { reason: "checkout is unbound".into(), busy: true, - completion_busy: false, + building: false, }); } let legacy_import = crate::migration::views_background::BackgroundImport::start( @@ -851,12 +887,14 @@ impl CallgraphView { source: Arc, ) -> Self { Self { + owner: super::callgraph_acquisition::AcquisitionOwner::new(root.clone()), root, storage, family, policy, source, - query: std::sync::Mutex::new(super::contracts::LoadRetry::default()), + query: parking_lot::Mutex::new(super::contracts::LoadRetry::default()), + acquisition: std::sync::Mutex::new(None), steady: std::sync::Mutex::new(None), #[cfg(test)] acquisitions: Default::default(), @@ -872,6 +910,25 @@ impl CallgraphView { self.source.record_change(path); } + pub(crate) fn set_executor( + &mut self, + executor: Option, + ) { + self.owner.set_executor(executor); + } + + pub(crate) fn set_publish_fence(&self, fence: Arc) { + self.source.set_publish_fence(fence); + } + + pub(crate) fn cancel(&self) { + self.owner.cancel(); + } + + pub(crate) fn shutdown(&self) { + self.owner.shutdown(); + } + pub(crate) fn record_overflow(&self) { self.ready .store(false, std::sync::atomic::Ordering::Release); @@ -893,7 +950,7 @@ impl CallgraphView { #[cfg(test)] pub(crate) fn retry_due_for_test(&self) { - if let Some((deadline, _)) = &mut self.query.lock().unwrap().pending { + if let Some((deadline, _)) = &mut self.query.lock().pending { *deadline = std::time::Instant::now(); } } @@ -909,25 +966,34 @@ impl CallgraphView { &self, now: impl Fn() -> std::time::Instant, ) -> Result, CallgraphViewError> { - if !(self.current)() { + let deadline = std::time::Instant::now() + super::contracts::CALLGRAPH_QUERY_WAIT; + self.reader_before(now, deadline) + } + + fn reader_before( + &self, + now: impl Fn() -> std::time::Instant, + deadline: std::time::Instant, + ) -> Result, CallgraphViewError> { + if self.owner.stopped() || !(self.current)() { return Err(CallgraphViewError { reason: "checkout is no longer bound to this generation".into(), busy: true, - completion_busy: false, + building: false, }); } + // Joining another request and receiving the acquisition share one + // deadline; queued readers must not get a fresh window after the lock. let mut retry = self .query - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + .try_lock_until(deadline) + .ok_or_else(graph_building)?; let mut steady = self .steady .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - if let (Some((revision, store)), Some(current)) = - (steady.as_ref(), self.source.steady_revision()) - { - if *revision == current { + if let Some((revision, store)) = steady.as_ref() { + if self.source.steady_revision() == Some(*revision) { self.ready.store(true, std::sync::atomic::Ordering::Release); return Ok(store.clone()); } @@ -945,12 +1011,20 @@ impl CallgraphView { "legacy text and symbol import building", )); } - #[cfg(test)] - self.acquisitions - .fetch_add(1, std::sync::atomic::Ordering::Relaxed); - self.source.acquire_with_revision() + self.acquire_before(deadline) }, ); + // A request timeout is not a failed load. Keep the single-flight + // receiver, but do not back off: the next read can consume its result + // as soon as publication finishes, even if the previous read timed out. + if self + .acquisition + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .is_some() + { + *retry = super::contracts::LoadRetry::default(); + } *steady = match &result { Ok((store, Some(revision))) => Some((*revision, store.clone())), _ => None, @@ -960,6 +1034,143 @@ impl CallgraphView { .store(result.is_ok(), std::sync::atomic::Ordering::Release); result } + + #[cfg(test)] + pub(crate) fn ready_reader_for_test(&self) -> Arc { + self.ready_reader_for_test_with_clock(std::time::Instant::now) + } + + #[cfg(test)] + fn ready_reader_for_test_with_clock( + &self, + now: impl Fn() -> std::time::Instant, + ) -> Arc { + use std::time::{Duration, Instant}; + let deadline = Instant::now() + Duration::from_secs(120); + self.ready_reader_before_for_test(now, deadline) + } + + pub(crate) fn ready_reader_before_for_test( + &self, + now: impl Fn() -> std::time::Instant, + deadline: std::time::Instant, + ) -> Arc { + use std::time::Instant; + let state = || { + format!( + "root={} family={} ready={} stopped={} current={} acquisition={:?} retry={:?} legacy_import_building={}", + self.root.display(), self.family, self.is_ready(), self.owner.stopped(), (self.current)(), + self.acquisition.try_lock().ok().map(|slot| slot.as_ref().map(crossbeam_channel::Receiver::len)), + self.query.try_lock().map(|retry| retry.pending.clone()), + self.legacy_import.as_ref().is_some_and(|job| job.is_building()), + ) + }; + if let Some(import) = &self.legacy_import { + assert!( + import.wait_until_for_test(deadline), + "callgraph fixture import did not finish within 120s: {}", + state() + ); + } + loop { + assert!( + Instant::now() < deadline, + "callgraph fixture did not become ready within 120s: {}", + state() + ); + let retry_at = self + .query + .try_lock_until(deadline) + .unwrap_or_else(|| { + panic!( + "callgraph fixture query stayed locked for 120s: {}", + state() + ) + }) + .pending + .as_ref() + .map(|(retry_at, _)| *retry_at); + if let Some(retry_at) = retry_at { + let wait = retry_at.saturating_duration_since(now()); + if !wait.is_zero() { + let due = crossbeam_channel::after(wait); + assert!( + due.recv_timeout(deadline.saturating_duration_since(Instant::now())) + .is_ok(), + "callgraph fixture retry did not become due within 120s: {}", + state() + ); + } + } + match self.reader_before(&now, deadline) { + Ok(reader) => return reader, + Err(error) if error.busy && !self.owner.stopped() && (self.current)() => { + // Each attempt waits on its acquisition receiver. A completed + // transient refusal may need a new verified acquisition. + } + Err(error) => panic!("callgraph fixture read failed: {error}; {}", state()), + } + } + } + + fn acquire_before(&self, deadline: std::time::Instant) -> ReaderAcquisition { + let mut acquisition = self + .acquisition + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(rx) = acquisition.as_ref() { + match rx.try_recv() { + Ok(Ok((store, revision))) => { + *acquisition = None; + // A result can sit between requests while the checkout + // changes. Reuse it only if the source's steady revision + // still matches the result's revision; otherwise bind again + // against disk, reusing the published graph rather than + // trusting an old verification. + if revision.is_some() && self.source.steady_revision() == revision { + return Ok((store, revision)); + } + } + Ok(Err(error)) => { + *acquisition = None; + return Err(error); + } + Err(crossbeam_channel::TryRecvError::Empty) => {} + Err(_) => { + *acquisition = None; + return Err(graph_building()); + } + } + } + if acquisition.is_none() { + let source = self.source.clone(); + let (tx, rx) = crossbeam_channel::bounded(1); + self.owner.spawn( + self.current.clone(), + move || source.acquire_with_revision(), + tx, + )?; + #[cfg(test)] + self.acquisitions + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); + *acquisition = Some(rx); + } + match acquisition + .as_ref() + .unwrap() + .recv_timeout(deadline.saturating_duration_since(std::time::Instant::now())) + { + Ok(result) => { + *acquisition = None; + result + } + Err(crossbeam_channel::RecvTimeoutError::Timeout) => Err(graph_building()), + Err(_) => { + *acquisition = None; + Err(graph_building()) + } + } + } } fn queryable_manifest( @@ -1048,25 +1259,6 @@ mod tests { .unwrap() } - // A cold load may not be ready while metadata or publication is pending. - // Keep the production reader's backoff instead of requiring the - // first attempt to finish, and bound the fixture's total retry time. - fn ready_reader(view: &CallgraphView) -> Arc { - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); - loop { - match view.reader() { - Ok(reader) => return reader, - Err(error) => { - assert!(error.busy, "{error}"); - let now = std::time::Instant::now(); - assert!(now < deadline, "callgraph stayed pending: {error}"); - let retry_at = view.query.lock().unwrap().pending.as_ref().unwrap().0; - std::thread::sleep(retry_at.min(deadline).saturating_duration_since(now)); - } - } - } - } - fn events(dir: &Path) -> Vec { super::super::donor::events_under(dir) } @@ -1103,6 +1295,77 @@ mod tests { } } + #[test] + fn acquisition_worker_preserves_attribute_fixture_scope() { + struct PolicySource(Option<[PathBuf; 2]>); + impl CallgraphReaderSource for PolicySource { + fn acquire(&self) -> Result, CallgraphViewError> { + assert_eq!( + super::super::verification::attribute_sources_snapshot_for_test(), + self.0 + ); + Err("fixture acquisition completed under its policy".into()) + } + fn record_change(&self, _: &Path) {} + } + let temp = tempfile::tempdir().unwrap(); + let root = checkout(&temp.path().join("checkout")); + let _attributes = super::super::verification::known_attribute_sources_for_test(temp.path()); + let expected = super::super::verification::attribute_sources_snapshot_for_test(); + assert!(expected.is_some()); + let graph = CallgraphView::with_source( + root, + crate::root_cache::StoragePaths::same(&temp.path().join("storage")), + "fixture-policy".into(), + crate::blob_store::v2::TrigramPolicy { + max_file_size: 1 << 20, + }, + Arc::new(PolicySource(expected)), + ); + let error = graph.reader().unwrap_err(); + assert_eq!( + error.reason, + "fixture acquisition completed under its policy" + ); + } + + #[test] + fn fixture_reader_joins_acquisition_after_interactive_wait_expires() { + let _serial = crate::blob_store::v3::serial_for_test(); + let temp = tempfile::tempdir().unwrap(); + let root = checkout(&temp.path().join("checkout")); + let graph = Arc::new(view(&root, &temp.path().join("storage"))); + let (entered_tx, entered_rx) = crossbeam_channel::bounded(1); + let (release_tx, release_rx) = crossbeam_channel::bounded(1); + let _hook = crate::storage_retention::observe_test_hook( + &root, + Arc::new(move |step| { + if step == "callgraph-materialize" { + entered_tx.send(()).unwrap(); + release_rx + .recv_timeout(std::time::Duration::from_secs(120)) + .unwrap(); + } + }), + ); + let request_graph = graph.clone(); + let request = std::thread::spawn(move || request_graph.reader()); + entered_rx + .recv_timeout(std::time::Duration::from_secs(120)) + .unwrap(); + let fixture_graph = graph.clone(); + let fixture = std::thread::spawn(move || fixture_graph.ready_reader_for_test()); + let answer = request.join().unwrap(); + release_tx.send(()).unwrap(); + let ready = fixture.join().unwrap(); + assert!( + answer.unwrap_err().building, + "the interactive read must retain its production bound" + ); + assert_eq!(callers(&ready, "a.ts", "target").len(), 1); + assert!(graph.is_ready()); + } + #[test] fn policy_query_timeout_does_not_become_checkout_evidence() { let _serial = crate::blob_store::v3::serial_for_test(); @@ -1134,8 +1397,28 @@ mod tests { assert_eq!(error.kind(), std::io::ErrorKind::WouldBlock, "{error}"); } - let graph = view(&root, &temp.path().join("storage")); - let _timeout = crate::git_index_reader::timeout_next_config_for_test(); + struct PolicyTimeoutOnce { + inner: Arc, + timeout: std::sync::atomic::AtomicBool, + } + impl CallgraphReaderSource for PolicyTimeoutOnce { + fn acquire(&self) -> Result, CallgraphViewError> { + // The fault is thread-local; install it where acquisition runs. + let _timeout = self + .timeout + .swap(false, std::sync::atomic::Ordering::AcqRel) + .then(crate::git_index_reader::timeout_next_config_for_test); + self.inner.acquire() + } + fn record_change(&self, path: &Path) { + self.inner.record_change(path); + } + } + let mut graph = view(&root, &temp.path().join("storage")); + graph.source = Arc::new(PolicyTimeoutOnce { + inner: graph.source.clone(), + timeout: std::sync::atomic::AtomicBool::new(true), + }); let pending = graph.reader().unwrap_err(); assert!(pending.busy, "{pending}"); assert!(!pending.reason.contains("changed"), "{pending}"); @@ -1146,7 +1429,10 @@ mod tests { crate::callgraph_store::CallGraphStoreError::Sqlite(rusqlite::Error::SqliteFailure(error, _)) ) if error.code == rusqlite::ErrorCode::DatabaseBusy )); - assert_eq!(callers(&ready_reader(&graph), "a.ts", "target").len(), 1); + assert_eq!( + callers(&graph.ready_reader_for_test(), "a.ts", "target").len(), + 1 + ); assert!(graph.is_ready()); before.validate(&root).unwrap(); } @@ -1181,18 +1467,12 @@ mod tests { assert!(error.busy); for seconds in [1, 2, 4, 8, 16, 30, 30] { let deadline = now + std::time::Duration::from_secs(seconds); - assert_eq!( - graph.query.lock().unwrap().pending.as_ref().unwrap().0, - deadline - ); + assert_eq!(graph.query.lock().pending.as_ref().unwrap().0, deadline); let error = graph .reader_with_clock(|| deadline - std::time::Duration::from_nanos(1)) .unwrap_err(); assert_eq!(error.reason, "legacy text and symbol import building"); - assert_eq!( - graph.query.lock().unwrap().pending.as_ref().unwrap().0, - deadline - ); + assert_eq!(graph.query.lock().pending.as_ref().unwrap().0, deadline); now = deadline; assert!(graph.reader_with_clock(|| now).is_err()); } @@ -1206,11 +1486,11 @@ mod tests { std::thread::sleep(std::time::Duration::from_millis(10)); } now += std::time::Duration::from_secs(30); - let ready = graph.reader_with_clock(|| now).unwrap(); + let ready = graph.ready_reader_for_test_with_clock(|| now); assert_eq!(callers(&ready, "a.ts", "target").len(), 1); assert!(graph.is_ready()); - assert!(graph.query.lock().unwrap().pending.is_none()); - assert!(graph.query.lock().unwrap().delay.is_zero()); + assert!(graph.query.lock().pending.is_none()); + assert!(graph.query.lock().delay.is_zero()); } #[test] @@ -1221,7 +1501,10 @@ mod tests { let storage = temp.path().join("storage"); let root = checkout(&temp.path().join("checkout")); let graph = view(&root, &storage); - assert_eq!(callers(&graph.reader().unwrap(), "a.ts", "target").len(), 1); + assert_eq!( + callers(&graph.ready_reader_for_test(), "a.ts", "target").len(), + 1 + ); drop(graph); let dir = crate::blob_store::v3::view_dir(&storage, "donor-reuse", &view_scope(&root)).unwrap(); @@ -1282,13 +1565,13 @@ mod tests { assert!(busy, "{reason}"); assert!(matches!( crate::context::checkout_view_callgraph_access(Some(Err(CallgraphViewError { - reason, busy, completion_busy: false, + reason, busy, building: false, }))), crate::context::CallgraphStoreAccess::Error(error) if error.is_transient_lock_contention() )); let graph = view(&root, &storage); assert!(!graph.is_ready()); - let ready = graph.reader().unwrap(); + let ready = graph.ready_reader_for_test(); assert!(graph.is_ready()); assert_eq!(callers(&ready, "a.ts", "target").len(), 1); let rebuilt = store.current_generation_read_only().unwrap().unwrap(); @@ -1360,8 +1643,14 @@ mod tests { crate::watcher_backend::InitialCoverage::Complete, ); let graph = view(&root, &storage); - assert_eq!(callers(&graph.reader().unwrap(), "a.ts", "target").len(), 1); - assert_eq!(callers(&graph.reader().unwrap(), "a.ts", "target").len(), 1); + assert_eq!( + callers(&graph.ready_reader_for_test(), "a.ts", "target").len(), + 1 + ); + assert_eq!( + callers(&graph.ready_reader_for_test(), "a.ts", "target").len(), + 1 + ); assert_eq!( graph.acquisitions.load(Relaxed), 1, @@ -1375,7 +1664,10 @@ mod tests { ) .unwrap(); graph.record_change(&added); - assert_eq!(callers(&graph.reader().unwrap(), "b.ts", "caller").len(), 1); + assert_eq!( + callers(&graph.ready_reader_for_test(), "b.ts", "caller").len(), + 1 + ); assert_eq!( graph.acquisitions.load(Relaxed), 2, @@ -1389,10 +1681,137 @@ mod tests { ) .unwrap(); crate::watcher_backend::subscription::gap(&root); - assert_eq!(callers(&graph.reader().unwrap(), "c.ts", "top").len(), 1); + assert_eq!( + callers(&graph.ready_reader_for_test(), "c.ts", "top").len(), + 1 + ); assert_eq!(graph.acquisitions.load(Relaxed), 3, "a watcher gap binds"); } + #[test] + fn completed_acquisition_reverifies_an_unwatched_checkout_before_reuse() { + let _serial = crate::blob_store::v3::serial_for_test(); + let temp = tempfile::tempdir().unwrap(); + let root = checkout(&temp.path().join("checkout")); + let graph = view(&root, &temp.path().join("storage")); + let (entered_tx, entered_rx) = crossbeam_channel::bounded(1); + let (release_tx, release_rx) = crossbeam_channel::bounded(1); + let (published_tx, published_rx) = crossbeam_channel::bounded(1); + let hook = crate::storage_retention::observe_test_hook( + &root, + Arc::new(move |step| match step { + "callgraph-materialize" => { + entered_tx.send(()).unwrap(); + release_rx + .recv_timeout(std::time::Duration::from_secs(10)) + .unwrap(); + } + "callgraph-reader-acquired" => { + published_tx.send(()).unwrap(); + } + _ => {} + }), + ); + // An expired receive budget starts the worker without waiting for it. + let result = graph.acquire_before(std::time::Instant::now()); + let entered = entered_rx.recv_timeout(std::time::Duration::from_secs(10)); + release_tx.send(()).unwrap(); + assert!(entered.is_ok()); + assert!(result.unwrap_err().building); + published_rx + .recv_timeout(std::time::Duration::from_secs(10)) + .unwrap(); + drop(hook); + + // No watcher reports the edit made after the earlier acquisition + // finished. The acquisition result waiting for the next read was verified + // before this edit, so it cannot be considered verified now. + std::fs::write( + root.join("c.ts"), + "import { caller } from './b';\nexport function latest() { return caller(); }\n", + ) + .unwrap(); + let ready = graph.ready_reader_for_test(); + let names = callers(&ready, "b.ts", "caller"); + assert_eq!(names.len(), 1); + assert!(names[0].contains("latest"), "{names:?}"); + assert_eq!( + graph + .acquisitions + .load(std::sync::atomic::Ordering::Relaxed), + 2 + ); + } + + #[test] + fn cancel_reaches_completed_and_running_acquisition_tokens() { + let _serial = crate::blob_store::v3::serial_for_test(); + let temp = tempfile::tempdir().unwrap(); + let root = checkout(&temp.path().join("checkout")); + let graph = Arc::new(view(&root, &temp.path().join("storage"))); + let tail_seen = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let (tokens_tx, tokens_rx) = crossbeam_channel::unbounded(); + let (release_tx, release_rx) = crossbeam_channel::unbounded(); + let _hook = crate::storage_retention::observe_test_hook( + &root, + Arc::new(move |step| { + let hold_tail = step == "callgraph-reader-acquired" + && !tail_seen.swap(true, std::sync::atomic::Ordering::AcqRel); + let hold_work = step == "callgraph-materialize" + && tail_seen.load(std::sync::atomic::Ordering::Acquire); + if hold_tail || hold_work { + let token = crate::executor::current_job_cancellation().unwrap(); + tokens_tx.send(token.clone()).unwrap(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !token.cancel_already_requested() { + if release_rx + .recv_timeout(std::time::Duration::from_millis(10)) + .is_ok() + { + break; + } + assert!( + std::time::Instant::now() < deadline, + "acquisition gate was not released" + ); + } + } + }), + ); + drop(graph.ready_reader_for_test()); + let old = tokens_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .unwrap(); + let added = root.join("c.ts"); + std::fs::write( + &added, + "import { caller } from './b';\nexport function latest() { return caller(); }\n", + ) + .unwrap(); + graph.record_change(&added); + let next_graph = graph.clone(); + let next = std::thread::spawn(move || next_graph.reader()); + let current = tokens_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .unwrap(); + assert_ne!(old.progress_key(), current.progress_key()); + graph.cancel(); + let old_cancelled = old.wait_for_cancellation(std::time::Duration::from_millis(250)); + let current_cancelled = current.cancel_already_requested(); + let _ = release_tx.send(()); + let answer = next.join().unwrap(); + drop(graph); + assert!(answer.unwrap_err().building); + assert!( + current_cancelled, + "running acquisition token was not cancelled" + ); + assert!( + old_cancelled, + "the earlier completion tail lost its cancellation token" + ); + } + #[test] fn v3_checkpoint_contention_reaches_checkout_reader_as_busy() { use crate::blob_store::v3; @@ -1402,7 +1821,10 @@ mod tests { let storage = temp.path().join("storage"); let root = checkout(&temp.path().join("checkout")); let graph = view(&root, &storage); - assert_eq!(callers(&graph.reader().unwrap(), "a.ts", "target").len(), 1); + assert_eq!( + callers(&graph.ready_reader_for_test(), "a.ts", "target").len(), + 1 + ); let plane = v3::plane_path(&storage, "donor-reuse", FamilyPlane::Callgraph).unwrap(); let blocker = IdentityConnection::open_with_flags( &plane, @@ -1421,10 +1843,30 @@ mod tests { ) .unwrap(); graph.record_change(&added); + let (failed_tx, failed_rx) = crossbeam_channel::bounded(1); + let _hook = crate::storage_retention::observe_test_hook( + &root, + Arc::new(move |step| { + if step == "callgraph-acquisition-failed" { + failed_tx.send(()).unwrap(); + } + }), + ); let error = match graph.reader() { Ok(_) => panic!("a pinned WAL reader should prevent the truncating checkpoint"), Err(error) => error, }; + // The request can time out before SQLite's longer checkpoint wait. + // Keep the blocker until the worker finishes, then require the reader + // to preserve the checkpoint's original busy cause on the next read. + let error = if error.building { + failed_rx + .recv_timeout(std::time::Duration::from_secs(10)) + .unwrap(); + graph.reader().unwrap_err() + } else { + error + }; assert!(error.busy, "{error}"); assert!(error.reason.contains(V3_STORE_BUSY), "{error}"); assert!(error.reason.contains("checkpoint WAL"), "{error}"); @@ -1436,7 +1878,10 @@ mod tests { blocker.execute_batch("ROLLBACK").unwrap(); drop(blocker); graph.retry_due_for_test(); - assert_eq!(callers(&graph.reader().unwrap(), "b.ts", "caller").len(), 1); + assert_eq!( + callers(&graph.ready_reader_for_test(), "b.ts", "caller").len(), + 1 + ); } #[test] @@ -1535,7 +1980,12 @@ mod tests { blocker.execute_batch("ROLLBACK").unwrap(); drop(blocker); assert_eq!( - callers(&construct().unwrap().reader().unwrap(), "a.ts", "target").len(), + callers( + &construct().unwrap().ready_reader_for_test(), + "a.ts", + "target" + ) + .len(), 1 ); } @@ -1547,7 +1997,7 @@ mod tests { format!("{V3_BASE_COMPLETION_BUSY}: CapReached {{ live_bases: 4 }}"), )); assert!(!error.busy); - assert!(!error.completion_busy); + assert!(!error.building); assert!(matches!( crate::context::checkout_view_callgraph_access(Some(Err(error))), crate::context::CallgraphStoreAccess::Error(_) @@ -1619,17 +2069,11 @@ mod tests { for (attempt, seconds) in [1, 2, 4, 8, 16, 30, 30].into_iter().enumerate() { let deadline = now + std::time::Duration::from_secs(seconds); - assert_eq!( - graph.query.lock().unwrap().pending.as_ref().unwrap().0, - deadline - ); + assert_eq!(graph.query.lock().pending.as_ref().unwrap().0, deadline); assert!(graph .reader_with_clock(|| deadline - std::time::Duration::from_nanos(1)) .is_err()); - assert_eq!( - graph.query.lock().unwrap().pending.as_ref().unwrap().0, - deadline - ); + assert_eq!(graph.query.lock().pending.as_ref().unwrap().0, deadline); now = deadline; if attempt < 6 { assert!(matches!( @@ -1643,14 +2087,16 @@ mod tests { claim.release().unwrap(); let crate::context::CallgraphStoreAccess::Ready(ready) = - crate::context::checkout_view_callgraph_access(Some(graph.reader_with_clock(|| now))) + crate::context::checkout_view_callgraph_access(Some(Ok( + graph.ready_reader_for_test_with_clock(|| now) + ))) else { panic!("the completed base must serve a ready call graph") }; assert_eq!(callers(&ready, "a.ts", "target").len(), 1); assert!(graph.is_ready()); - assert!(graph.query.lock().unwrap().pending.is_none()); - assert!(graph.query.lock().unwrap().delay.is_zero()); + assert!(graph.query.lock().pending.is_none()); + assert!(graph.query.lock().delay.is_zero()); } #[test] @@ -1751,9 +2197,7 @@ mod tests { // another driver can finish the graph only if the failed bind released // its completion claim rather than waiting for process-death takeover. let graph = view(&root, &storage); - let ready = graph - .reader() - .expect("a failed bind must not strand a live claim"); + let ready = graph.ready_reader_for_test(); assert_eq!(callers(&ready, "a.ts", "target").len(), 1); assert_eq!(failed.installed_snapshot().generation().name(), "empty"); } @@ -1815,10 +2259,10 @@ mod tests { max_file_size: 1 << 20, }; let main_view = view(&main, &storage); - let expected = callers(&main_view.reader().unwrap(), "a.ts", "target"); + let expected = callers(&main_view.ready_reader_for_test(), "a.ts", "target"); assert!(!expected.is_empty()); let worker_view = view(&worker, &storage); - worker_view.reader().unwrap(); + worker_view.ready_reader_for_test(); let scope = view_scope(&main); let (borrowed, _) = super::super::callgraph_copies::open_published( &storage, family, policy, &scope, &worker, @@ -1880,12 +2324,12 @@ mod tests { "the worker's shared commit base must survive" ); assert_eq!( - callers(&worker_view.reader().unwrap(), "a.ts", "target"), + callers(&worker_view.ready_reader_for_test(), "a.ts", "target"), expected ); let rebuilt = view(&main, &storage); assert_eq!( - callers(&rebuilt.reader().unwrap(), "a.ts", "target"), + callers(&rebuilt.ready_reader_for_test(), "a.ts", "target"), expected ); } @@ -1950,7 +2394,7 @@ mod tests { }; let main_view = view(&main, &storage); - main_view.reader().unwrap(); + main_view.ready_reader_for_test(); assert_eq!( admit(&main, 0), Admission::Own, @@ -1958,7 +2402,7 @@ mod tests { ); assert_eq!(admit(&first, 1), Admission::Own); let first_view = view(&first, &storage); - first_view.reader().unwrap(); + first_view.ready_reader_for_test(); assert_eq!( admit(&first, 1), Admission::Own, @@ -1973,7 +2417,7 @@ mod tests { .unwrap(); assert_eq!( callers(&borrowed, "a.ts", "target"), - callers(&main_view.reader().unwrap(), "a.ts", "target"), + callers(&main_view.ready_reader_for_test(), "a.ts", "target"), ); drop(borrowed); @@ -2098,7 +2542,7 @@ mod tests { let linked = std::fs::canonicalize(&linked).unwrap(); let main_view = view(&main, &storage); - let main_store = main_view.reader().unwrap(); + let main_store = main_view.ready_reader_for_test(); assert_eq!(callers(&main_store, "a.ts", "target").len(), 1); assert!( main_view.extraction_count() > 0, @@ -2106,7 +2550,7 @@ mod tests { ); let linked_view = view(&linked, &storage); - let linked_store = linked_view.reader().unwrap(); + let linked_store = linked_view.ready_reader_for_test(); assert_eq!( callers(&linked_store, "a.ts", "target"), callers(&main_store, "a.ts", "target") @@ -2232,11 +2676,11 @@ mod tests { let storage = temp.path().join("storage"); let main_view = view(&main, &storage); assert_eq!( - callers(&ready_reader(&main_view), "a.ts", "target").len(), + callers(&main_view.ready_reader_for_test(), "a.ts", "target").len(), 1 ); let linked_view = view(&linked, &storage); - let patched = callers(&ready_reader(&linked_view), "a.ts", "target"); + let patched = callers(&linked_view.ready_reader_for_test(), "a.ts", "target"); assert_eq!(patched.len(), 2, "{patched:?}"); assert_ne!(base_dir(&storage, &linked), base_dir(&storage, &main)); let base_events = events(&base_dir(&storage, &linked)); @@ -2256,7 +2700,7 @@ mod tests { let cold_storage = temp.path().join("cold-storage"); let cold_view = view(&linked, &cold_storage); assert_eq!( - callers(&ready_reader(&cold_view), "a.ts", "target"), + callers(&cold_view.ready_reader_for_test(), "a.ts", "target"), patched ); assert!(!events(&base_dir(&cold_storage, &linked)) @@ -2279,7 +2723,7 @@ mod tests { // A fresh checkout bootstraps the base through the base builder, // completes and seals its graph, then clones the donor for itself. let first_view = view(&first, &storage); - let first_store = first_view.reader().unwrap(); + let first_store = first_view.ready_reader_for_test(); assert_eq!(callers(&first_store, "a.ts", "target").len(), 1); assert_eq!(first_view.extraction_count(), 3, "a.ts, b.ts, package.json"); let all = events(&storage); @@ -2332,7 +2776,7 @@ mod tests { // A second checkout of the same bytes reuses the base's blobs and its // sealed donor: nothing is parsed and no graph is built cold. let second_view = view(&second, &storage); - let second_store = second_view.reader().unwrap(); + let second_store = second_view.ready_reader_for_test(); assert_eq!(second_view.extraction_count(), 0); assert_eq!( callers(&second_store, "a.ts", "target"), @@ -2351,7 +2795,7 @@ mod tests { ) .unwrap(); second_view.record_change(&second.join("c.ts")); - let edited = second_view.reader().unwrap(); + let edited = second_view.ready_reader_for_test(); assert_eq!(callers(&edited, "b.ts", "caller").len(), 1); assert!(callers(&first_store, "b.ts", "caller").is_empty()); assert_eq!(second_view.extraction_count(), 1); @@ -2367,7 +2811,7 @@ mod tests { // base never had it. std::fs::remove_file(second.join("c.ts")).unwrap(); second_view.record_change(&second.join("c.ts")); - let deleted = second_view.reader().unwrap(); + let deleted = second_view.ready_reader_for_test(); assert!(callers(&deleted, "b.ts", "caller").is_empty()); // A reader acquired before the deletion keeps answering for its own // generation: publication never reclaims a generation a reader holds. @@ -2381,7 +2825,7 @@ mod tests { // or rebuilding anything. drop((first_store, first_view)); let restarted = view(&first, &storage); - let reopened = restarted.reader().unwrap(); + let reopened = restarted.ready_reader_for_test(); assert_eq!(callers(&reopened, "a.ts", "target").len(), 1); assert_eq!(restarted.extraction_count(), 0); assert_eq!(events(&member(&first)).len(), 1); diff --git a/crates/aft/src/views/callgraph_acquisition.rs b/crates/aft/src/views/callgraph_acquisition.rs new file mode 100644 index 0000000000..6cbb179fde --- /dev/null +++ b/crates/aft/src/views/callgraph_acquisition.rs @@ -0,0 +1,172 @@ +//! Ownership of a checkout's graph acquisition, independent of any one read. +use std::path::PathBuf; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; +use std::thread::JoinHandle; + +use super::callgraph::{graph_building, CallgraphViewError, ReaderAcquisition}; +use crate::executor::{JobCancellation, RootMaintenanceExecutor}; + +pub(super) struct AcquisitionOwner { + root: PathBuf, + executor: Option, + stopped: Arc, + active: Mutex>, + standalone: Mutex>>, +} + +struct ActiveAcquisition { + cancellation: JobCancellation, + finished: Arc, +} + +struct AcquisitionCompletion(Arc); + +impl Drop for AcquisitionCompletion { + fn drop(&mut self) { + self.0.store(true, Ordering::Release); + } +} + +impl AcquisitionOwner { + pub(super) fn new(root: PathBuf) -> Self { + Self { + root, + executor: None, + stopped: Arc::new(AtomicBool::new(false)), + active: Mutex::new(Vec::new()), + standalone: Mutex::new(Vec::new()), + } + } + + pub(super) fn set_executor(&mut self, executor: Option) { + self.executor = executor; + } + + pub(super) fn stopped(&self) -> bool { + self.stopped.load(Ordering::Acquire) + } + + pub(super) fn cancel(&self) { + self.stopped.store(true, Ordering::Release); + for active in self.active.lock().unwrap().iter() { + active.cancellation.request_cancel(); + } + } + + pub(super) fn shutdown(&self) { + self.cancel(); + // Standalone has no executor to drain its workers. Keep their handles + // so shutdown can cancel and join every acquisition before returning. + let handles = std::mem::take(&mut *self.standalone.lock().unwrap()); + for handle in handles { + let _ = handle.join(); + } + } + + pub(super) fn spawn( + &self, + current: crate::migration::v3_ledger::ImportControl, + acquire: impl FnOnce() -> ReaderAcquisition + Send + 'static, + tx: crossbeam_channel::Sender, + ) -> Result<(), CallgraphViewError> { + if self.stopped() { + return Err(graph_building()); + } + let cancellation = JobCancellation::new().with_root(&self.root); + let finished = Arc::new(AtomicBool::new(false)); + // Capturing the guard also completes the record if queued work is + // dropped without running, or if source acquisition unwinds. + let completion = AcquisitionCompletion(finished.clone()); + { + let mut active = self.active.lock().unwrap(); + if self.stopped() { + return Err(graph_building()); + } + // The view's result receiver prevents overlapping source calls. + // An older worker can still be returning after sending its result, + // so keep its token until its closure finishes. + active.retain(|job| !job.finished.load(Ordering::Acquire)); + active.push(ActiveAcquisition { + cancellation: cancellation.clone(), + finished, + }); + } + let token = cancellation.clone(); + let stopped = self.stopped.clone(); + #[cfg(test)] + let root = self.root.clone(); + // Fixture attribute-source overrides are thread-local. Carry the + // caller's overrides onto the acquisition worker. + #[cfg(test)] + let attribute_sources = super::verification::attribute_sources_snapshot_for_test(); + let run = move || { + let _completion = completion; + #[cfg(test)] + let _attributes = + super::verification::install_attribute_sources_for_test(attribute_sources); + let _cancel = crate::executor::current_job_cancellation() + .is_none() + .then(|| crate::executor::install_job_cancellation(token)); + let cancelled = || { + crate::executor::current_job_cancelled() + || stopped.load(Ordering::Acquire) + || !current() + }; + let result = if cancelled() { + Err(graph_building()) + } else { + acquire() + }; + let result = if cancelled() { + Err(graph_building()) + } else { + result + }; + #[cfg(test)] + let finished = if result.is_ok() { + "callgraph-reader-acquired" + } else { + "callgraph-acquisition-failed" + }; + let _ = tx.send(result); + #[cfg(test)] + crate::storage_retention::test_hook(&root, finished); + }; + if let Some(executor) = &self.executor { + // A configured daemon submitter that has shut down must refuse; + // falling back to a new thread would reactivate a retired root. + if !executor.submit_callgraph_acquisition( + cancellation, + Box::new(move |_| { + run(); + crate::protocol::Response::success( + "callgraph-view-acquire", + serde_json::Value::Null, + ) + }), + ) { + return Err(graph_building()); + } + } else { + let mut handles = self.standalone.lock().unwrap(); + if self.stopped() { + return Err(graph_building()); + } + handles.retain(|handle| !handle.is_finished()); + handles.push( + std::thread::Builder::new() + .name("callgraph-view-acquire".into()) + .spawn(run) + .map_err(|error| CallgraphViewError::from(error.to_string()))?, + ); + } + Ok(()) + } +} + +impl Drop for AcquisitionOwner { + fn drop(&mut self) { + self.shutdown(); + } +} diff --git a/crates/aft/src/views/callgraph_copies.rs b/crates/aft/src/views/callgraph_copies.rs index 85c9b52aa3..85586e1d3b 100644 --- a/crates/aft/src/views/callgraph_copies.rs +++ b/crates/aft/src/views/callgraph_copies.rs @@ -578,7 +578,7 @@ mod tests { policy(), ) .unwrap(); - main_view.reader().unwrap(); + main_view.ready_reader_for_test(); Fixture { _temp: temp, _attributes: attributes, @@ -729,7 +729,7 @@ mod tests { policy(), ) .unwrap(); - held.reader().unwrap(); + held.ready_reader_for_test(); let peer = f._temp.path().join("peer"); git( &f.main, @@ -782,7 +782,7 @@ mod tests { policy(), ) .unwrap(); - main_view.reader().unwrap(); + main_view.ready_reader_for_test(); let held_root = f._temp.path().join("held"); std::fs::create_dir(&held_root).unwrap(); std::fs::write(held_root.join("held.ts"), "export function held() {}\n").unwrap(); @@ -793,7 +793,7 @@ mod tests { policy(), ) .unwrap(); - held.reader().unwrap(); + held.ready_reader_for_test(); let mut config = crate::config::Config::default(); config.project_root = Some(f.worker.clone()); config.storage_dir = Some(f.paths.artifacts.clone()); @@ -807,7 +807,7 @@ mod tests { ); ctx.set_canonical_cache_root(f.worker.clone()); let reader = ctx - .callgraph_view_reader() + .with_ready_callgraph_for_test(|| ctx.callgraph_view_reader()) .expect("views route") .expect("must not refuse"); assert!(reader.file_identities().unwrap().contains_key("a.ts")); diff --git a/crates/aft/src/views/cap_tests.rs b/crates/aft/src/views/cap_tests.rs index d9712095b0..4456bc6a81 100644 --- a/crates/aft/src/views/cap_tests.rs +++ b/crates/aft/src/views/cap_tests.rs @@ -88,7 +88,10 @@ impl HeldFamily { config.clone(), ); main_ctx.set_canonical_cache_root(main.clone()); - main_ctx.callgraph_view_reader().unwrap().unwrap(); + main_ctx + .with_ready_callgraph_for_test(|| main_ctx.callgraph_view_reader()) + .unwrap() + .unwrap(); let family = main_ctx.memoized_artifact_cache_key(&main); with_base_barrier(&storage, &family, |tx| { let count: i64 = diff --git a/crates/aft/src/views/first_load.rs b/crates/aft/src/views/first_load.rs index 9c4e9ef8ed..1094e386fe 100644 --- a/crates/aft/src/views/first_load.rs +++ b/crates/aft/src/views/first_load.rs @@ -488,6 +488,14 @@ pub trait PublishFence: Send + Sync { -> Result<(), PlaneError>; } +fn check_job_cancelled() -> Result<(), PlaneError> { + if crate::executor::current_job_cancelled() { + Err(SiblingLoader::error(SUPERSEDED_PUBLISH.to_owned())) + } else { + Ok(()) + } +} + /// Derived graphs cannot be copied between checkouts until tests prove their /// rows contain no source-root-dependent data. Per-file blobs remain reusable. pub const DERIVED_CALLGRAPH_SEEDING: bool = false; @@ -3091,6 +3099,7 @@ impl CheckoutDriver { snapshot: &Snapshot, header: super::manifest_v2::BaseHeader, ) -> Result, PlaneError> { + check_job_cancelled()?; let failed = |error: super::base_refs::BaseRefError| SiblingLoader::error(error.to_string()); let route = self.owner.v3_route(); @@ -3220,6 +3229,7 @@ impl CheckoutDriver { .entries .clone(); for plane in &self.planes { + check_job_cancelled()?; plane.materialize_v3( &self.owner, &route, @@ -3229,6 +3239,7 @@ impl CheckoutDriver { &mut delta, &mut live, )?; + check_job_cancelled()?; } // `Wold`: the installed v3 generation of this checkout, or the // durable v3 pointer when the installed bundle is not one. @@ -3281,6 +3292,8 @@ impl CheckoutDriver { Ok(()) }, &mut || { + check_job_cancelled() + .map_err(|error| super::base_refs::BaseRefError::Unavailable(error.reason))?; if let Some(guard) = snapshot .install_cut() .and_then(|cut| cut.verification.as_ref()) @@ -3299,6 +3312,7 @@ impl CheckoutDriver { .publication_transition .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); + check_job_cancelled()?; actual = Some(commit().map_err(|error| SiblingLoader::error(error.to_string()))?); Ok(()) @@ -5041,6 +5055,9 @@ impl CompositePlane for CallgraphBridge { }; #[cfg(test)] let _profile = crate::blob_store::v3::work_profile::span("callgraph_materialize"); + #[cfg(test)] + crate::storage_retention::test_hook(_owner.root(), "callgraph-materialize"); + check_job_cancelled()?; let mut v3_readers = V3PlaneReaders::new(route); let mut attachments = Vec::new(); for (path, entry) in observed { @@ -5112,7 +5129,15 @@ impl CompositePlane for CallgraphBridge { .saturating_add(1 << 20); let schema = FamilyPlane::Callgraph.payload_schema(); crate::blob_store::v3::publish(&route.storage, &route.family, request, None, |p| { + #[cfg(test)] + crate::storage_retention::test_hook(_owner.root(), "callgraph-reserved"); + if crate::executor::current_job_cancelled() { + return Err(crate::blob_store::v3::Error::Refused(SUPERSEDED_PUBLISH)); + } for (_, key, bytes) in &payloads { + if crate::executor::current_job_cancelled() { + return Err(crate::blob_store::v3::Error::Refused(SUPERSEDED_PUBLISH)); + } p.put(key, bytes, schema)?; } Ok(()) @@ -5135,6 +5160,7 @@ impl CompositePlane for CallgraphBridge { } drop((payloads, attachments)); // Serialization and parsing buffers no longer belong to this phase. + check_job_cancelled()?; #[cfg(any(target_os = "linux", target_os = "macos"))] crate::memory::relieve_allocator_pressure(); // The derived graph covers the composed generation. While the base's diff --git a/crates/aft/src/views/materialization.rs b/crates/aft/src/views/materialization.rs index 4b022be522..5c9a56b0f8 100644 --- a/crates/aft/src/views/materialization.rs +++ b/crates/aft/src/views/materialization.rs @@ -299,6 +299,16 @@ pub(crate) fn apply_manifest_diff_from_blob_reader( .map(|(stats, _)| stats) } +fn check_job_cancelled() -> Result<()> { + if crate::executor::current_job_cancelled() { + Err(CallGraphStoreError::Unavailable( + super::first_load::SUPERSEDED_PUBLISH.into(), + )) + } else { + Ok(()) + } +} + fn materialize_reading( database_path: &Path, manifest: &crate::views::Manifest, @@ -306,6 +316,7 @@ fn materialize_reading( reader: &impl join::ManifestBlobReader, blob_connection: Option<&Connection>, ) -> Result<(MaterializeStats, profile::PhaseTimings)> { + check_job_cancelled()?; let shared_reader = join::SharedBlobReader { inner: reader, decoded: Default::default(), @@ -657,6 +668,7 @@ fn materialize_reading( let mut owned_nodes = Vec::new(); let mut cold_module_facts = HashMap::new(); for (path, entry) in manifest.entries() { + check_job_cancelled()?; if changed .as_ref() .is_some_and(|paths| !paths.contains(path.as_bytes())) @@ -948,6 +960,7 @@ fn materialize_reading( let mut changed_dependencies = Vec::new(); let mut binding_inserts = Vec::new(); for (path, binding) in &joined.bindings { + check_job_cancelled()?; if joined.rebuilt_surface_paths.contains(path) { if let Some(payload) = binding .surface_json() @@ -1149,6 +1162,7 @@ fn materialize_reading( #[cfg(test)] let mut control_lookup = transaction.prepare("SELECT EXISTS(SELECT 1 FROM refs WHERE ref_id=?1 AND caller_node IS ?2 AND status=?3 AND target_node IS ?4 AND target_file IS ?5 AND target_symbol IS ?6)")?; for row in joined.result.rows { + check_job_cancelled()?; let caller_path = String::from_utf8(row.caller_path).map_err(|_| { CallGraphStoreError::Unavailable("non-UTF-8 manifest caller path".to_string()) })?; diff --git a/crates/aft/src/views/materialization/dispatch.rs b/crates/aft/src/views/materialization/dispatch.rs index d1cfe9eac6..381469e9a5 100644 --- a/crates/aft/src/views/materialization/dispatch.rs +++ b/crates/aft/src/views/materialization/dispatch.rs @@ -667,6 +667,7 @@ pub(super) fn emit_from_decoded( let mut file_dependencies = BTreeMap::>::new(); resolver.take_dependencies(); for path in &selected { + super::check_job_cancelled()?; let Some(row) = rows.get(path) else { continue; }; diff --git a/crates/aft/src/views/mod.rs b/crates/aft/src/views/mod.rs index 283fafa014..15d6bf0bb5 100644 --- a/crates/aft/src/views/mod.rs +++ b/crates/aft/src/views/mod.rs @@ -52,6 +52,7 @@ pub mod symbol_store; // Plane and runtime modules of the per-checkout views. Each starts as a // documented empty module that its plane fills in. pub mod callgraph; +mod callgraph_acquisition; pub(crate) mod callgraph_copies; #[cfg(test)] diff --git a/crates/aft/src/views/semantic_runtime_tests.rs b/crates/aft/src/views/semantic_runtime_tests.rs index 4bca40864c..0c481f499e 100644 --- a/crates/aft/src/views/semantic_runtime_tests.rs +++ b/crates/aft/src/views/semantic_runtime_tests.rs @@ -3134,7 +3134,7 @@ fn build_independent_callgraph(root: &Path, storage: &Path) { }, ) .unwrap(); - let reader = graph.reader().unwrap(); + let reader = graph.ready_reader_for_test(); assert_eq!( reader .callers_of(Path::new("a.ts"), "target", 1) diff --git a/crates/aft/src/views/verification.rs b/crates/aft/src/views/verification.rs index c3d29c037b..04144260c6 100644 --- a/crates/aft/src/views/verification.rs +++ b/crates/aft/src/views/verification.rs @@ -31,7 +31,19 @@ pub(crate) fn known_attribute_sources_for_test(root: &Path) -> AttributeSourcesG root.join("absent-global-attributes"), ]; assert!(sources.iter().all(|p| !p.exists())); - AttributeSourcesGuard(ATTRIBUTE_SOURCES.with(|paths| paths.replace(Some(sources)))) + install_attribute_sources_for_test(Some(sources)) +} + +#[cfg(test)] +pub(crate) fn attribute_sources_snapshot_for_test() -> Option<[PathBuf; 2]> { + ATTRIBUTE_SOURCES.with(|paths| paths.borrow().clone()) +} + +#[cfg(test)] +pub(crate) fn install_attribute_sources_for_test( + sources: Option<[PathBuf; 2]>, +) -> AttributeSourcesGuard { + AttributeSourcesGuard(ATTRIBUTE_SOURCES.with(|paths| paths.replace(sources))) } use crate::blob_store::v2::{ContentHash, FamilyKey, FamilyPlane}; diff --git a/crates/aft/tests/integration/callgraph_worktree_test.rs b/crates/aft/tests/integration/callgraph_worktree_test.rs index e0937db3d7..36317e8c02 100644 --- a/crates/aft/tests/integration/callgraph_worktree_test.rs +++ b/crates/aft/tests/integration/callgraph_worktree_test.rs @@ -330,7 +330,8 @@ fn callgraph_restart_reconciles_a_dirty_view_after_git_restore() { publish(&checkout, &storage); git(&checkout, &["restore", "index.ts"]); let ctx = configure(&checkout, &storage, true); - let response = query(&ctx, &checkout, "callers", "ownerCaller"); + let response = + ctx.with_ready_callgraph_for_test(|| query(&ctx, &checkout, "callers", "ownerCaller")); assert!(response.success, "{response:?}"); assert_eq!(response.data["total_callers"], 1); assert!( @@ -361,7 +362,8 @@ fn callgraph_worktree_publishes_unique_source_without_an_owner_writer_or_overlay assert!(ctx.shared_artifacts_read_only()); assert!(!ctx.callgraph_writer()); assert!(!ctx.ram_overlay_active()); - let response = query(&ctx, &checkout, "callers", "workerCaller"); + let response = + ctx.with_ready_callgraph_for_test(|| query(&ctx, &checkout, "callers", "workerCaller")); assert!( response.success, "worktree-only source must publish without an owner writer: {response:?}" @@ -612,7 +614,8 @@ fn callgraph_unindexed_path_differs_from_missing_symbol() { "{operation}: {response:?}" ); } - let response = query_file(&ctx, &unindexed, "callers", "newCaller"); + let response = + ctx.with_ready_callgraph_for_test(|| query_file(&ctx, &unindexed, "callers", "newCaller")); assert!(response.success, "{response:?}"); let response = callgraph_when_ready("callers", || { aft::commands::callers::handle_callers( @@ -667,14 +670,16 @@ fn callgraph_worktree_published_owner_assembles_its_own_changed_source() { // With views on, the first query binds this worktree's own checkout view // from its own source, before configure maintenance has run: it never // answers from the owner's graph and never reports the owner's callers. - match ctx.callgraph_store_for_ops() { - aft::context::CallgraphStoreAccess::Ready(store) => { - assert_eq!(store.reader_kind(), "view"); - assert_eq!(store.project_root(), checkout); + let response = ctx.with_ready_callgraph_for_test(|| { + match ctx.callgraph_store_for_ops() { + aft::context::CallgraphStoreAccess::Ready(store) => { + assert_eq!(store.reader_kind(), "view"); + assert_eq!(store.project_root(), checkout); + } + _ => panic!("expected this checkout's view reader"), } - _ => panic!("expected this checkout's view reader"), - } - let response = query(&ctx, &checkout, "callers", "checkoutCaller"); + query(&ctx, &checkout, "callers", "checkoutCaller") + }); assert!(response.success, "{response:?}"); assert_eq!(response.data["total_callers"], 1); assert!(response.data.to_string().contains("checkoutCaller"));