Skip to content

Commit 572a4ac

Browse files
committed
fix(docker): stop flaking orphan-restore on PID reuse
- Canonicalize exe paths in `pid_is_our_stitch` to handle usr-merge hosts where `/bin/sleep` and `/usr/bin/sleep` are the same binary - Identify orphan processes by (pid, starttime) tuple instead of pid alone to avoid false negatives when the kernel recycles PIDs - Copy sleep binary to a unique stitch binary path during test to prevent identity collisions under parallel cargo test - Poll briefly for the original process to disappear before asserting the orphan is killed - Bump version to 0.1.130
1 parent 961126c commit 572a4ac

5 files changed

Lines changed: 50 additions & 26 deletions

File tree

.textile-monorepo-source

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
c451adbe7bf0f04d515a4c223a15afc685b2d0fe
1+
925818ee85697c7d0f3bcf7dd124b00664c3cf07

.textile-stitch-release-version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
0.1.129
1+
0.1.130

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "stitch-bot"
3-
version = "0.1.129"
3+
version = "0.1.130"
44
edition = "2021"
55
description = "Stitch — Textile filler-network operator bot; market-makes the filler order book with signed UniswapX limit orders."
66
license = "AGPL-3.0-or-later"

src/panel/docker/process_api.rs

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,13 @@ fn pid_is_our_stitch(pid: u32, starttime: Option<u64>, stitch_bin: &Path) -> boo
685685
if exe == stitch_bin {
686686
return true;
687687
}
688+
// `/bin/sleep` vs `/usr/bin/sleep` (usr-merge) — compare canonical paths.
689+
let exe_canon = std::fs::canonicalize(&exe).unwrap_or(exe);
690+
if let Ok(want_canon) = std::fs::canonicalize(stitch_bin) {
691+
if exe_canon == want_canon {
692+
return true;
693+
}
694+
}
688695
}
689696
if let Ok(cmdline) = std::fs::read(format!("/proc/{pid}/cmdline")) {
690697
let first = cmdline.split(|b| *b == 0).next().unwrap_or(&[]);
@@ -1316,26 +1323,40 @@ mod tests {
13161323

13171324
#[tokio::test]
13181325
async fn restore_kills_orphan_pid_before_respawn() {
1319-
let sleep_bin = which_sleep();
13201326
let root = temp_root("orphan");
13211327
let bots = root.join("bots");
13221328
let state = bots.join(".process-runtime");
13231329
std::fs::create_dir_all(&state).unwrap();
13241330
let host = bots.join("bot-a");
13251331
std::fs::create_dir_all(&host).unwrap();
13261332

1327-
// Spawn an orphan the new runtime should find via persisted pid.
1333+
// Unique binary path so identity checks don't collide with other tests'
1334+
// `/usr/bin/sleep` orphans under parallel cargo test.
1335+
let stitch_bin = root.join("stitch");
1336+
std::fs::copy(which_sleep(), &stitch_bin).unwrap();
1337+
#[cfg(unix)]
1338+
{
1339+
use std::os::unix::fs::PermissionsExt;
1340+
let mut perms = std::fs::metadata(&stitch_bin).unwrap().permissions();
1341+
perms.set_mode(0o755);
1342+
std::fs::set_permissions(&stitch_bin, perms).unwrap();
1343+
}
1344+
13281345
// Forget the Child so the OS reparents it (like a crashed panel would):
13291346
// if we keep the handle, a SIGTERM'd process becomes our zombie and
13301347
// kill(pid, 0) stays true until we wait().
1331-
let orphan = Command::new(&sleep_bin)
1332-
.arg("30")
1348+
let orphan = Command::new(&stitch_bin)
1349+
.arg("60")
13331350
.stdout(Stdio::null())
13341351
.stderr(Stdio::null())
13351352
.spawn()
13361353
.unwrap();
13371354
let orphan_pid = orphan.id();
13381355
let orphan_start = process_starttime(orphan_pid);
1356+
assert!(
1357+
process_alive(orphan_pid),
1358+
"precondition: orphan must be running"
1359+
);
13391360
std::mem::forget(orphan);
13401361
let record = PersistedBot {
13411362
id: "proc-stitch-bot-a".into(),
@@ -1344,7 +1365,7 @@ mod tests {
13441365
labels: HashMap::new(),
13451366
env: vec![],
13461367
binds: vec![PersistedBind::from(&BindSpec::rw(&host, RUN_DIR))],
1347-
cmd: Some(vec!["30".into()]),
1368+
cmd: Some(vec!["60".into()]),
13481369
restart_unless_stopped: true,
13491370
wanted_up: true,
13501371
created_unix: now_unix(),
@@ -1353,30 +1374,33 @@ mod tests {
13531374
};
13541375
persist_record(&state, &record).unwrap();
13551376

1356-
let rt = ProcessRuntime::new(sleep_bin, &bots).unwrap();
1357-
let (new_pid, new_start) = {
1377+
let rt = ProcessRuntime::new(stitch_bin, &bots).unwrap();
1378+
let new_pid = {
13581379
let inner = rt.inner.lock().unwrap();
1359-
let live = &inner["stitch-bot-a"];
1360-
(live.record.pid, live.record.pid_starttime)
1380+
inner["stitch-bot-a"].record.pid
13611381
};
13621382
assert!(
13631383
new_pid.is_some(),
13641384
"wanted bot must be respawned after orphan kill"
13651385
);
1366-
// Don't assert `!process_alive(orphan_pid)` alone: under load the OS can
1367-
// recycle that pid onto the respawned bot, which is still a successful
1368-
// kill+spawn. Prove the orphan is gone via pid or starttime.
1369-
if new_pid == Some(orphan_pid) {
1370-
assert_ne!(
1371-
new_start, orphan_start,
1372-
"respawn reused orphan pid; starttime must show a new process"
1373-
);
1374-
} else {
1375-
assert!(
1376-
!process_alive(orphan_pid),
1377-
"orphan from the previous panel must be terminated on restore"
1378-
);
1386+
// Identify the orphan by (pid, starttime), not pid alone. Under load the
1387+
// kernel can recycle the number onto the respawned bot or an unrelated
1388+
// process — `process_alive(orphan_pid)` would then spuriously fail the
1389+
// test even though the original orphan is gone.
1390+
let mut orphan_gone = false;
1391+
for _ in 0..100 {
1392+
let same_process =
1393+
process_alive(orphan_pid) && process_starttime(orphan_pid) == orphan_start;
1394+
if !same_process {
1395+
orphan_gone = true;
1396+
break;
1397+
}
1398+
std::thread::sleep(Duration::from_millis(50));
13791399
}
1400+
assert!(
1401+
orphan_gone,
1402+
"orphan pid {orphan_pid} still running with original starttime after restore"
1403+
);
13801404
drop(rt);
13811405
let _ = std::fs::remove_dir_all(root);
13821406
}

0 commit comments

Comments
 (0)