|
| 1 | +use std::path::Path; |
| 2 | +use std::time::Duration; |
| 3 | + |
1 | 4 | use anyhow::Result; |
2 | 5 | use uuid::Uuid; |
3 | 6 |
|
4 | 7 | use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; |
| 8 | +use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; |
5 | 9 | use crate::services::mutation_trace::protocol; |
6 | 10 | use crate::services::mutation_trace::store::{CasResult, DurableTransition, MutationTraceStore}; |
7 | 11 | use crate::services::mutation_trace::types::{ |
8 | 12 | self, ActorKind, AttemptId, Boundary, EventId, MutationEvent, ScopeId, TreeId, WorktreeId, |
9 | 13 | }; |
10 | 14 |
|
11 | 15 | use super::git_snapshot::GitSnapshotService; |
| 16 | +use super::worktree_lock::{WorktreeLock, WorktreeLockError}; |
12 | 17 |
|
13 | 18 | const MAX_CAS_RETRY_ATTEMPTS: u32 = 5; |
14 | 19 |
|
| 20 | +const WORKTREE_LOCK_TIMEOUT: Duration = Duration::from_secs(10); |
| 21 | + |
15 | 22 | #[derive(Clone, Debug)] |
16 | 23 | pub enum RuntimeBoundary { |
17 | 24 | Start { |
@@ -104,6 +111,27 @@ impl SnapshotCapture for GitSnapshotService { |
104 | 111 | } |
105 | 112 | } |
106 | 113 |
|
| 114 | +pub fn coordinate( |
| 115 | + repository_root: &Path, |
| 116 | + db: &RepositoryAgentTraceDb, |
| 117 | + boundary: &RuntimeBoundary, |
| 118 | +) -> Result<CoordinateOutcome, CoordinateError> { |
| 119 | + let git_dir = resolve_git_dir(repository_root).map_err(CoordinateError::Other)?; |
| 120 | + |
| 121 | + let _lock = WorktreeLock::acquire(&git_dir, WORKTREE_LOCK_TIMEOUT).map_err(lock_acquisition)?; |
| 122 | + |
| 123 | + let checkout_id = get_or_create_checkout_id(&git_dir).map_err(CoordinateError::Other)?; |
| 124 | + let worktree_id = WorktreeId(checkout_id); |
| 125 | + |
| 126 | + let snapshot = GitSnapshotService::new(repository_root).map_err(CoordinateError::Other)?; |
| 127 | + |
| 128 | + coordinate_boundary(db, &snapshot, &worktree_id, boundary) |
| 129 | +} |
| 130 | + |
| 131 | +fn lock_acquisition(error: WorktreeLockError) -> CoordinateError { |
| 132 | + CoordinateError::LockAcquisition(anyhow::Error::new(error)) |
| 133 | +} |
| 134 | + |
107 | 135 | fn coordinate_boundary<C: SnapshotCapture>( |
108 | 136 | db: &RepositoryAgentTraceDb, |
109 | 137 | capture: &C, |
@@ -315,7 +343,10 @@ where |
315 | 343 | mod tests { |
316 | 344 | use std::cell::{Cell, RefCell}; |
317 | 345 | use std::collections::VecDeque; |
| 346 | + use std::process::Command; |
318 | 347 | use std::sync::atomic::{AtomicU64, Ordering}; |
| 348 | + use std::sync::mpsc; |
| 349 | + use std::thread; |
319 | 350 |
|
320 | 351 | use super::*; |
321 | 352 | use crate::services::mutation_trace::store::encode_revision; |
@@ -345,6 +376,38 @@ mod tests { |
345 | 376 | } |
346 | 377 | } |
347 | 378 |
|
| 379 | + fn unique_test_repo(label: &str) -> std::path::PathBuf { |
| 380 | + let id = NEXT_TEST_DB_ID.fetch_add(1, Ordering::Relaxed); |
| 381 | + std::env::temp_dir().join(format!( |
| 382 | + "sce-mutation-trace-coordinator-repo-{label}-{}-{id}", |
| 383 | + std::process::id() |
| 384 | + )) |
| 385 | + } |
| 386 | + |
| 387 | + fn init_repo(repo_root: &std::path::Path) { |
| 388 | + std::fs::create_dir_all(repo_root).expect("repo root should be created"); |
| 389 | + run_git(repo_root, &["init", "--quiet"]); |
| 390 | + run_git(repo_root, &["config", "user.email", "test@example.com"]); |
| 391 | + run_git(repo_root, &["config", "user.name", "Test"]); |
| 392 | + } |
| 393 | + |
| 394 | + fn run_git(repo_root: &std::path::Path, args: &[&str]) { |
| 395 | + let output = Command::new("git") |
| 396 | + .args(args) |
| 397 | + .current_dir(repo_root) |
| 398 | + .output() |
| 399 | + .expect("git command should spawn"); |
| 400 | + assert!( |
| 401 | + output.status.success(), |
| 402 | + "git {args:?} failed: {}", |
| 403 | + String::from_utf8_lossy(&output.stderr) |
| 404 | + ); |
| 405 | + } |
| 406 | + |
| 407 | + fn remove_test_repo(repo_root: &std::path::Path) { |
| 408 | + let _ = std::fs::remove_dir_all(repo_root); |
| 409 | + } |
| 410 | + |
348 | 411 | enum FakeOutcome { |
349 | 412 | Succeed(TreeId), |
350 | 413 | Fail(String), |
@@ -1228,4 +1291,57 @@ mod tests { |
1228 | 1291 |
|
1229 | 1292 | remove_test_db(&db_path); |
1230 | 1293 | } |
| 1294 | + |
| 1295 | + #[test] |
| 1296 | + fn two_threads_on_the_same_worktree_serialize() { |
| 1297 | + let repo_root = unique_test_repo("t05-serialize"); |
| 1298 | + init_repo(&repo_root); |
| 1299 | + let db_path = repo_root.join("agent-trace.db"); |
| 1300 | + |
| 1301 | + let git_dir = resolve_git_dir(&repo_root).expect("git dir should resolve"); |
| 1302 | + let held = WorktreeLock::acquire(&git_dir, Duration::from_secs(5)) |
| 1303 | + .expect("the test should acquire the worktree lock before the worker runs"); |
| 1304 | + |
| 1305 | + let (started_tx, started_rx) = mpsc::channel(); |
| 1306 | + let (done_tx, done_rx) = mpsc::channel(); |
| 1307 | + let repo_root_clone = repo_root.clone(); |
| 1308 | + let db_path_clone = db_path.clone(); |
| 1309 | + let worker = thread::spawn(move || { |
| 1310 | + let db = RepositoryAgentTraceDb::new_at(&db_path_clone).expect("worker db should open"); |
| 1311 | + started_tx |
| 1312 | + .send(()) |
| 1313 | + .expect("started signal channel should still be open"); |
| 1314 | + let outcome = coordinate(&repo_root_clone, &db, &RuntimeBoundary::Flush); |
| 1315 | + done_tx |
| 1316 | + .send(()) |
| 1317 | + .expect("done signal channel should still be open"); |
| 1318 | + outcome |
| 1319 | + }); |
| 1320 | + |
| 1321 | + started_rx |
| 1322 | + .recv_timeout(Duration::from_secs(5)) |
| 1323 | + .expect("worker thread should reach the coordinate() call"); |
| 1324 | + |
| 1325 | + assert!( |
| 1326 | + done_rx.recv_timeout(Duration::from_millis(500)).is_err(), |
| 1327 | + "coordinate() must not enter its critical section while another holder owns the worktree lock" |
| 1328 | + ); |
| 1329 | + |
| 1330 | + drop(held); |
| 1331 | + |
| 1332 | + done_rx |
| 1333 | + .recv_timeout(Duration::from_secs(5)) |
| 1334 | + .expect("coordinate() should proceed once the worktree lock is released"); |
| 1335 | + |
| 1336 | + let outcome = worker |
| 1337 | + .join() |
| 1338 | + .expect("worker thread should not panic") |
| 1339 | + .expect("coordinate() should succeed once it can acquire the worktree lock"); |
| 1340 | + assert_eq!( |
| 1341 | + outcome.revision, 0, |
| 1342 | + "the worker's first observation flush should not advance the revision" |
| 1343 | + ); |
| 1344 | + |
| 1345 | + remove_test_repo(&repo_root); |
| 1346 | + } |
1231 | 1347 | } |
0 commit comments