Close remaining GPU invocation review nits
Throttle the managed probe after its event channel closes, reject option-like path operands, and document the connector test seam. Strengthen non-CRDT and Ctrl-C acceptance so socket side effects and a pre-signal surviving frontend are exercised, while avoiding cleanup signals to already-reaped daemon PIDs.
This commit is contained in:
parent
9136538df9
commit
154cb9f08d
|
|
@ -649,7 +649,10 @@ fn start_daemon_reaper(mut child: Child, facts: ManagedDaemonFacts) {
|
||||||
.expect("spawn managed daemon reaper thread");
|
.expect("spawn managed daemon reaper thread");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[allow(clippy::too_many_arguments)]
|
#[allow(
|
||||||
|
clippy::too_many_arguments,
|
||||||
|
reason = "connector, spawner, timing, and sink stay injectable for deterministic lifecycle tests"
|
||||||
|
)]
|
||||||
fn connect_managed_inner<C, S, F>(
|
fn connect_managed_inner<C, S, F>(
|
||||||
socket_path: &Path,
|
socket_path: &Path,
|
||||||
daemon_executable: &Path,
|
daemon_executable: &Path,
|
||||||
|
|
|
||||||
|
|
@ -896,6 +896,7 @@ fn run_headless_managed_probe(socket: &Path, report: &Path, daemon_executable: &
|
||||||
if disconnect.is_empty() {
|
if disconnect.is_empty() {
|
||||||
"attach event channel closed".clone_into(&mut disconnect);
|
"attach event channel closed".clone_into(&mut disconnect);
|
||||||
}
|
}
|
||||||
|
std::thread::sleep(Duration::from_millis(50));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1029,6 +1030,18 @@ OPTIONS:
|
||||||
|
|
||||||
/// Strict parser for direct, managed, and headless GPU entry points.
|
/// Strict parser for direct, managed, and headless GPU entry points.
|
||||||
fn parse_args(args: &[String]) -> Result<Mode, String> {
|
fn parse_args(args: &[String]) -> Result<Mode, String> {
|
||||||
|
if let [flag, operands @ ..] = args
|
||||||
|
&& matches!(
|
||||||
|
flag.as_str(),
|
||||||
|
"--attach" | "--managed-attach" | "--headless-probe" | "--headless-managed-probe"
|
||||||
|
)
|
||||||
|
&& let Some(operand) = operands.iter().find(|operand| operand.starts_with('-'))
|
||||||
|
{
|
||||||
|
return Err(format!(
|
||||||
|
"{flag} received option-like path operand {operand}; prefix it with ./ if it is a path"
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
match args {
|
match args {
|
||||||
[flag] if flag == "--help" || flag == "-h" => Ok(Mode::Help),
|
[flag] if flag == "--help" || flag == "-h" => Ok(Mode::Help),
|
||||||
[flag] if flag == "--version" || flag == "-V" => Ok(Mode::Version),
|
[flag] if flag == "--version" || flag == "-V" => Ok(Mode::Version),
|
||||||
|
|
@ -14314,6 +14327,15 @@ mod tests {
|
||||||
],
|
],
|
||||||
vec!["--managed-attach", "/tmp/pmacs.sock"],
|
vec!["--managed-attach", "/tmp/pmacs.sock"],
|
||||||
vec!["--headless-managed-probe", "/tmp/pmacs.sock", "/tmp/report"],
|
vec!["--headless-managed-probe", "/tmp/pmacs.sock", "/tmp/report"],
|
||||||
|
vec!["--attach", "--help"],
|
||||||
|
vec!["--managed-attach", "/tmp/pmacs.sock", "--version"],
|
||||||
|
vec!["--headless-probe", "/tmp/pmacs.sock", "--help"],
|
||||||
|
vec![
|
||||||
|
"--headless-managed-probe",
|
||||||
|
"/tmp/pmacs.sock",
|
||||||
|
"/tmp/report",
|
||||||
|
"--version",
|
||||||
|
],
|
||||||
vec!["research"],
|
vec!["research"],
|
||||||
];
|
];
|
||||||
for values in invalid {
|
for values in invalid {
|
||||||
|
|
@ -14326,6 +14348,9 @@ mod tests {
|
||||||
"accepted invalid argv: {values:?}"
|
"accepted invalid argv: {values:?}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
let error = parse_args(&["--attach".to_owned(), "--help".to_owned()])
|
||||||
|
.expect_err("option-like socket operand must fail");
|
||||||
|
assert!(error.contains("option-like path operand --help"));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
|
||||||
|
|
@ -25,8 +25,11 @@ fn write_script(path: &Path, body: &str) {
|
||||||
|
|
||||||
#[cfg(not(feature = "crdt"))]
|
#[cfg(not(feature = "crdt"))]
|
||||||
#[test]
|
#[test]
|
||||||
fn non_crdt_root_rejects_gpu_before_discovery_or_spawn() {
|
fn non_crdt_root_rejects_gpu_before_socket_io_discovery_or_spawn() {
|
||||||
let temp = secure_tempdir();
|
let temp = secure_tempdir();
|
||||||
|
let runtime = temp.path().join("runtime");
|
||||||
|
fs::create_dir(&runtime).expect("create runtime");
|
||||||
|
fs::set_permissions(&runtime, fs::Permissions::from_mode(0o700)).expect("chmod runtime 0700");
|
||||||
let fake_gpu = temp.path().join("fake-gpu");
|
let fake_gpu = temp.path().join("fake-gpu");
|
||||||
let marker = temp.path().join("spawned");
|
let marker = temp.path().join("spawned");
|
||||||
write_script(&fake_gpu, "touch \"$PMACS_TEST_MARKER\"");
|
write_script(&fake_gpu, "touch \"$PMACS_TEST_MARKER\"");
|
||||||
|
|
@ -35,18 +38,41 @@ fn non_crdt_root_rejects_gpu_before_discovery_or_spawn() {
|
||||||
.arg("--gpu")
|
.arg("--gpu")
|
||||||
.env(TEST_GPU_OVERRIDE, &fake_gpu)
|
.env(TEST_GPU_OVERRIDE, &fake_gpu)
|
||||||
.env("PMACS_TEST_MARKER", &marker)
|
.env("PMACS_TEST_MARKER", &marker)
|
||||||
|
.env("XDG_RUNTIME_DIR", &runtime)
|
||||||
.output()
|
.output()
|
||||||
.expect("run non-CRDT pmacs --gpu");
|
.expect("run non-CRDT pmacs --gpu");
|
||||||
assert!(!output.status.success());
|
assert!(!output.status.success());
|
||||||
|
assert!(!marker.exists(), "GPU executable must not be spawned");
|
||||||
assert!(
|
assert!(
|
||||||
!marker.exists(),
|
!runtime.join("pmacs/default.sock").exists(),
|
||||||
"GPU executable must not be discovered or spawned"
|
"the CRDT gate must run before default-socket creation"
|
||||||
);
|
);
|
||||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||||
assert!(
|
assert!(
|
||||||
stderr.contains("--features crdt"),
|
stderr.contains("--features crdt"),
|
||||||
"unexpected stderr: {stderr}"
|
"unexpected stderr: {stderr}"
|
||||||
);
|
);
|
||||||
|
|
||||||
|
let occupied_socket = temp.path().join("occupied.sock");
|
||||||
|
let listener =
|
||||||
|
std::os::unix::net::UnixListener::bind(&occupied_socket).expect("bind occupied socket");
|
||||||
|
let occupied = Command::new(env!("CARGO_BIN_EXE_pmacs"))
|
||||||
|
.args(["--gpu", "--socket"])
|
||||||
|
.arg(&occupied_socket)
|
||||||
|
.env(TEST_GPU_OVERRIDE, &fake_gpu)
|
||||||
|
.env("PMACS_TEST_MARKER", &marker)
|
||||||
|
.output()
|
||||||
|
.expect("run non-CRDT pmacs --gpu against occupied socket");
|
||||||
|
assert!(!occupied.status.success());
|
||||||
|
assert!(
|
||||||
|
!marker.exists(),
|
||||||
|
"live socket must not weaken the CRDT gate"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
occupied_socket.exists(),
|
||||||
|
"live socket must remain untouched"
|
||||||
|
);
|
||||||
|
drop(listener);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(feature = "crdt")]
|
#[cfg(feature = "crdt")]
|
||||||
|
|
@ -62,8 +88,10 @@ mod crdt {
|
||||||
|
|
||||||
use nix::sys::signal::{Signal, kill};
|
use nix::sys::signal::{Signal, kill};
|
||||||
use nix::unistd::Pid;
|
use nix::unistd::Pid;
|
||||||
|
use pmacs::cell::CellSize;
|
||||||
use pmacs::protocol::{
|
use pmacs::protocol::{
|
||||||
FrontendId, Hello, InstanceCapabilities, InstanceIdentity, PROTOCOL_VERSION,
|
AttachRequest, FrontendCapabilities, FrontendEvent, FrontendId, Hello,
|
||||||
|
InstanceCapabilities, InstanceIdentity, InstanceMessage, PROTOCOL_VERSION,
|
||||||
};
|
};
|
||||||
use pmacs::transport::{read_message, write_message};
|
use pmacs::transport::{read_message, write_message};
|
||||||
|
|
||||||
|
|
@ -145,6 +173,45 @@ mod crdt {
|
||||||
panic!("daemon did not listen on {}", socket.display());
|
panic!("daemon did not listen on {}", socket.display());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn attach_surviving_frontend(socket: &Path) -> (FrontendId, UnixStream) {
|
||||||
|
let mut stream = UnixStream::connect(socket).expect("connect surviving frontend");
|
||||||
|
stream
|
||||||
|
.set_read_timeout(Some(Duration::from_secs(5)))
|
||||||
|
.expect("set surviving frontend timeout");
|
||||||
|
let hello: Hello = read_message(&mut stream).expect("surviving frontend Hello");
|
||||||
|
write_message(
|
||||||
|
&mut stream,
|
||||||
|
&AttachRequest {
|
||||||
|
protocol_version: PROTOCOL_VERSION,
|
||||||
|
frontend_capabilities: FrontendCapabilities {
|
||||||
|
multi_frontend: true,
|
||||||
|
crdt_replica: true,
|
||||||
|
..FrontendCapabilities::default()
|
||||||
|
},
|
||||||
|
initial_size: CellSize::new(24, 80),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.expect("attach surviving frontend");
|
||||||
|
|
||||||
|
let deadline = Instant::now() + Duration::from_secs(5);
|
||||||
|
let mut saw_snapshot = false;
|
||||||
|
let mut saw_full_grid = false;
|
||||||
|
while !(saw_snapshot && saw_full_grid) {
|
||||||
|
assert!(
|
||||||
|
Instant::now() < deadline,
|
||||||
|
"surviving frontend did not initialize"
|
||||||
|
);
|
||||||
|
match read_message::<InstanceMessage>(&mut stream).expect("initialize survivor") {
|
||||||
|
InstanceMessage::BufferSnapshot { .. } => saw_snapshot = true,
|
||||||
|
InstanceMessage::CellDelta {
|
||||||
|
full_grid: true, ..
|
||||||
|
} => saw_full_grid = true,
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
(hello.assigned_frontend_id, stream)
|
||||||
|
}
|
||||||
|
|
||||||
fn spawn_daemon(socket: &Path, envs: &[(&str, &str)]) -> Child {
|
fn spawn_daemon(socket: &Path, envs: &[(&str, &str)]) -> Child {
|
||||||
let home = socket.parent().expect("socket parent");
|
let home = socket.parent().expect("socket parent");
|
||||||
let mut command = Command::new(pmacs_binary());
|
let mut command = Command::new(pmacs_binary());
|
||||||
|
|
@ -234,7 +301,10 @@ mod crdt {
|
||||||
self.stdin.take();
|
self.stdin.take();
|
||||||
let _ = self.child.kill();
|
let _ = self.child.kill();
|
||||||
let _ = self.child.wait();
|
let _ = self.child.wait();
|
||||||
if let Some(pid) = self.daemon_pid {
|
let daemon_reaped = fs::read_to_string(&self.report)
|
||||||
|
.ok()
|
||||||
|
.is_some_and(|report| report.lines().any(|line| line == "daemon_reaped=true"));
|
||||||
|
if !daemon_reaped && let Some(pid) = self.daemon_pid {
|
||||||
signal_pid(pid, Signal::SIGTERM);
|
signal_pid(pid, Signal::SIGTERM);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -440,13 +510,37 @@ mod crdt {
|
||||||
let mut launcher = command.spawn().expect("spawn launcher process group");
|
let mut launcher = command.spawn().expect("spawn launcher process group");
|
||||||
let facts = wait_for_fact(&report, "phase", "ready", Duration::from_secs(10));
|
let facts = wait_for_fact(&report, "phase", "ready", Duration::from_secs(10));
|
||||||
let daemon_pid = facts["daemon_pid"].parse::<u32>().expect("daemon pid");
|
let daemon_pid = facts["daemon_pid"].parse::<u32>().expect("daemon pid");
|
||||||
|
let (survivor_id, mut survivor) = attach_surviving_frontend(&socket);
|
||||||
|
|
||||||
kill(Pid::from_raw(-launcher.id().cast_signed()), Signal::SIGINT)
|
kill(Pid::from_raw(-launcher.id().cast_signed()), Signal::SIGINT)
|
||||||
.expect("signal launcher group");
|
.expect("signal launcher group");
|
||||||
let _ = wait_for_exit(&mut launcher, Duration::from_secs(5));
|
let _ = wait_for_exit(&mut launcher, Duration::from_secs(5));
|
||||||
let mut stream = UnixStream::connect(&socket).expect("daemon survived launcher Ctrl-C");
|
|
||||||
let hello: Hello = read_message(&mut stream).expect("surviving daemon Hello");
|
write_message(
|
||||||
assert_eq!(hello.protocol_version, PROTOCOL_VERSION);
|
&mut survivor,
|
||||||
|
&FrontendEvent::Resize {
|
||||||
|
frontend_id: survivor_id,
|
||||||
|
size: CellSize::new(31, 91),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.expect("resize surviving frontend after launcher Ctrl-C");
|
||||||
|
let deadline = Instant::now() + Duration::from_secs(5);
|
||||||
|
loop {
|
||||||
|
assert!(
|
||||||
|
Instant::now() < deadline,
|
||||||
|
"pre-signal frontend did not render after launcher Ctrl-C"
|
||||||
|
);
|
||||||
|
if matches!(
|
||||||
|
read_message::<InstanceMessage>(&mut survivor)
|
||||||
|
.expect("read surviving frontend after launcher Ctrl-C"),
|
||||||
|
InstanceMessage::CellDelta {
|
||||||
|
full_grid: true,
|
||||||
|
..
|
||||||
|
}
|
||||||
|
) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
signal_pid(daemon_pid, Signal::SIGTERM);
|
signal_pid(daemon_pid, Signal::SIGTERM);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -608,6 +702,7 @@ mod crdt {
|
||||||
for argv in [
|
for argv in [
|
||||||
vec!["--attach"],
|
vec!["--attach"],
|
||||||
vec!["--attach", "/tmp/x.sock", "ignored"],
|
vec!["--attach", "/tmp/x.sock", "ignored"],
|
||||||
|
vec!["--attach", "--help"],
|
||||||
vec!["unexpected"],
|
vec!["unexpected"],
|
||||||
] {
|
] {
|
||||||
let output = Command::new(gpu_binary())
|
let output = Command::new(gpu_binary())
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue