diff --git a/src/lib.rs b/src/lib.rs index ae6370f..1219527 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,7 +44,10 @@ use std::path::Path; /// - `tuning` controls the caching / write-back knobs (see [`Tuning`]). /// /// Invalidation requests are read from the process's stdin and answered on its stdout; use -/// [`mount_with_invalidations`] to use other streams. +/// [`mount_with_invalidations`] to use other streams. Reading stdin ties up one of the runtime's +/// blocking threads until stdin closes, so shut the runtime down with +/// [`shutdown_background`](tokio::runtime::Runtime::shutdown_background) rather than dropping it, +/// or the process can't exit while stdin stays open. pub async fn mount( transport: Box, mountpoint: &Path, diff --git a/src/main.rs b/src/main.rs index 2732474..391f308 100644 --- a/src/main.rs +++ b/src/main.rs @@ -137,8 +137,15 @@ enum Cmd { MakeShared { path: PathBuf }, } -#[tokio::main] -async fn main() -> Result<(), Box> { +fn main() -> Result<(), Box> { + let runtime = tokio::runtime::Runtime::new()?; + let result = runtime.block_on(run()); + // Reading stdin ties up a thread until stdin closes, which may never happen. Don't wait for it. + runtime.shutdown_background(); + result +} + +async fn run() -> Result<(), Box> { // Log to stderr: stdin/stdout are protocol channels (invalidation paths arrive on stdin), so // stdout must stay clean. tracing_subscriber::fmt() diff --git a/tests/diod_fuse.rs b/tests/diod_fuse.rs index 9f60d76..541e8a6 100644 --- a/tests/diod_fuse.rs +++ b/tests/diod_fuse.rs @@ -134,7 +134,7 @@ impl Diod { .stderr(Stdio::piped()) .spawn() .expect("spawn diod"); - if diod_listening(&mut child) { + if diod_listening(&mut child) && accepting(port) { return Diod { child, export, @@ -175,6 +175,19 @@ fn diod_listening(child: &mut Child) -> bool { rx.recv_timeout(Duration::from_secs(10)).unwrap_or(false) } +/// Wait until a connection to `port` succeeds. diod logs that it is listening once it has bound the +/// port but before it calls listen(), so a connection made right then can be refused. +fn accepting(port: u16) -> bool { + let deadline = Instant::now() + Duration::from_secs(10); + while Instant::now() < deadline { + if std::net::TcpStream::connect(("127.0.0.1", port)).is_ok() { + return true; + } + std::thread::sleep(Duration::from_millis(20)); + } + false +} + impl Drop for Diod { fn drop(&mut self) { let _ = self.child.kill(); @@ -211,12 +224,20 @@ fn umount(mp: &Path) { } /// Unmount `mp` and wait for its mount task to end, failing instead of hanging if it doesn't. +/// `conn` is the mount's FUSE connection, if it got far enough to have one. async fn unmount_and_join( mp: &Path, - task: tokio::task::JoinHandle>, + conn: Option, + mut task: tokio::task::JoinHandle>, ) -> Result<(), String> { umount(mp); - match tokio::time::timeout(Duration::from_secs(30), task).await { + if let Ok(joined) = tokio::time::timeout(Duration::from_secs(5), &mut task).await { + return joined.expect("mount task join"); + } + if let Some(conn) = conn { + abort_connection(conn); + } + match tokio::time::timeout(Duration::from_secs(25), task).await { Ok(joined) => joined.expect("mount task join"), Err(_) => panic!( "mount of {} still running 30s after unmounting", @@ -225,6 +246,20 @@ async fn unmount_and_join( } } +/// The number of the FUSE connection behind the mount at `mp`. +fn fuse_connection(mp: &Path) -> u32 { + use std::os::unix::fs::MetadataExt; + libc::minor(std::fs::metadata(mp).unwrap().dev()) +} + +/// End a FUSE connection whose session outlived its unmount. A process that copies the mount +/// namespace while the mount is up (a nix build sandbox, a container, `unshare -m`) keeps its own +/// copy of the mount, and so the filesystem, alive after we unmount ours. +fn abort_connection(conn: u32) { + eprintln!("mount still running 5s after unmounting; aborting FUSE connection {conn}"); + let _ = std::fs::write(format!("/sys/fs/fuse/connections/{conn}/abort"), "1"); +} + /// Which transport to reach diod through. Non-TCP transports use a tiny in-test relay that bridges /// to diod's TCP port, so all three transports exercise the same server. #[derive(Clone, Copy)] @@ -418,12 +453,13 @@ where panic!("mount ended before it became ready: {result:?}"); } if Instant::now() >= deadline { - let _ = unmount_and_join(&mp, task).await; + let _ = unmount_and_join(&mp, None, task).await; panic!("mount did not become ready within 15s"); } tokio::time::sleep(Duration::from_millis(50)).await; } + let conn = fuse_connection(&mp); let mp_body = mp.clone(); let export = diod.export().to_path_buf(); let outcome = tokio::task::spawn_blocking(move || { @@ -435,7 +471,7 @@ where }) .await; - let mount_result = unmount_and_join(&mp, task).await; + let mount_result = unmount_and_join(&mp, Some(conn), task).await; for h in relays { h.abort(); } @@ -1194,3 +1230,57 @@ async fn error_paths() { }) .await; } + +/// The binary exits once unmounted, even while whoever started it keeps its stdin open. +#[test] +fn binary_exits_after_unmount_with_stdin_open() { + if let Some(why) = skip_reason() { + eprintln!("skipping: {why}"); + return; + } + let diod = Diod::start(); + std::fs::write(diod.export().join(READY), b"1").unwrap(); + let mountpoint = tempfile::tempdir().unwrap(); + let mp = mountpoint.path(); + let mut child = Command::new(env!("CARGO_BIN_EXE_p9fuse")) + .arg("mount9p-fuse") + .arg(format!("--connect=tcp://127.0.0.1:{}", diod.port)) + .arg(format!("--uid={}", nix::unistd::geteuid())) + .arg("--aname") + .arg(diod.export()) + .arg(mp) + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .spawn() + .expect("spawn p9fuse"); + + let deadline = Instant::now() + Duration::from_secs(15); + while !mp.join(READY).exists() { + assert!(child.try_wait().unwrap().is_none(), "p9fuse exited early"); + assert!(Instant::now() < deadline, "mount did not become ready"); + std::thread::sleep(Duration::from_millis(50)); + } + let conn = fuse_connection(mp); + + umount(mp); + let exited = |child: &mut Child, secs| { + let deadline = Instant::now() + Duration::from_secs(secs); + while Instant::now() < deadline { + if let Some(status) = child.try_wait().unwrap() { + return Some(status); + } + std::thread::sleep(Duration::from_millis(50)); + } + None + }; + let status = exited(&mut child, 5).or_else(|| { + abort_connection(conn); + exited(&mut child, 10) + }); + let Some(status) = status else { + let _ = child.kill(); + let _ = child.wait(); + panic!("p9fuse still running 15s after unmounting"); + }; + assert!(status.success(), "p9fuse exited with {status}"); +}