Skip to content

Commit 5608a7e

Browse files
committed
Test that an interrupt inside the guard's drop still removes apply.lock
The drop now stays in the interrupt table until after its own unlink, but nothing exercised a real signal landing there. Add a debug-only `apply_lock.releasing` failpoint just before R1 and an e2e test that parks `apply` in the drop, sends SIGTERM, and checks the process dies by it without leaving apply.lock. The test fails if the guard leaves the table before the unlink, as it did before. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1881e15 commit 5608a7e

2 files changed

Lines changed: 23 additions & 3 deletions

File tree

‎crates/socket-patch-cli/tests/e2e_safety_lock.rs‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,12 +469,17 @@ mod interrupted_holder {
469469
/// `ignore_sigint`, the child starts with SIGINT ignored, as under
470470
/// `nohup` or a launcher that ignores Ctrl-C.
471471
fn spawn_holding_lock(root: &Path, ignore_sigint: bool) -> Child {
472+
spawn_parked_at(root, "apply_lock.acquired", ignore_sigint)
473+
}
474+
475+
/// Spawn `apply` in `root` and wait until it parks at `failpoint`.
476+
fn spawn_parked_at(root: &Path, failpoint: &str, ignore_sigint: bool) -> Child {
472477
let ready = root.join("failpoint-ready");
473478
let mut cmd = hermetic_command(&binary());
474479
jvm_env::isolate_cli(&mut cmd);
475480
cmd.args(["apply", "--json"])
476481
.current_dir(root)
477-
.env("SOCKET_PATCH_FAILPOINT", "apply_lock.acquired~pause")
482+
.env("SOCKET_PATCH_FAILPOINT", format!("{failpoint}~pause"))
478483
.env("SOCKET_PATCH_FAILPOINT_READY", &ready)
479484
.stdin(Stdio::null())
480485
.stdout(Stdio::null())
@@ -511,12 +516,16 @@ mod interrupted_holder {
511516
}
512517

513518
fn assert_interrupt_cleans_up(sig: libc::c_int) {
519+
assert_interrupt_at_cleans_up("apply_lock.acquired", sig);
520+
}
521+
522+
fn assert_interrupt_at_cleans_up(failpoint: &str, sig: libc::c_int) {
514523
let dir = tempfile::tempdir().unwrap();
515524
let socket_dir = dir.path().join(".socket");
516525
setup_socket_dir(&socket_dir);
517526
let lock = socket_dir.join("apply.lock");
518527

519-
let mut child = spawn_holding_lock(dir.path(), false);
528+
let mut child = spawn_parked_at(dir.path(), failpoint, false);
520529
assert!(lock.is_file(), "the parked apply holds apply.lock");
521530

522531
signal(&child, sig);
@@ -551,6 +560,15 @@ mod interrupted_holder {
551560
assert_interrupt_cleans_up(libc::SIGHUP);
552561
}
553562

563+
/// An interrupt that lands while the guard's drop is already under
564+
/// way — before the drop's own unlink — still removes the file: the
565+
/// handler ends the process without resuming the drop, so the guard
566+
/// must stay in the interrupt table until the drop has unlinked it.
567+
#[test]
568+
fn interrupt_during_release_removes_the_lock_file() {
569+
assert_interrupt_at_cleans_up("apply_lock.releasing", libc::SIGTERM);
570+
}
571+
554572
/// A process started with SIGINT ignored keeps ignoring it (no handler
555573
/// is installed over SIG_IGN), and SIGTERM still cleans up.
556574
#[test]

‎crates/socket-patch-core/src/patch/apply_lock.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,9 @@ impl Drop for LockGuard {
210210
// the barrier, which can take seconds over a large vendored tree,
211211
// or during the identity probe) still removes the file: the CLI's
212212
// handler ends the process, so this drop never gets to finish.
213-
//
213+
// Debug-only hook: the interrupt tests park the process here and
214+
// signal it.
215+
crate::utils::failpoint::hit("apply_lock.releasing");
214216
// R1: unlink while still holding the lock — but only the file we
215217
// hold. The unlink is gated on the path still naming the held
216218
// inode: after a non-cooperating `rm` + `touch`, the path names a

0 commit comments

Comments
 (0)