From 9e1ac78f31cf3016f46a35c753cfe3efc673d1cd Mon Sep 17 00:00:00 2001 From: Miguel Gila Date: Sat, 21 Mar 2026 17:47:30 +0100 Subject: [PATCH] fix: implement dynamic PTY resize (ResizePty) The shim's ResizePty was a no-op, causing interactive terminals to be stuck at 80x24. Implement file-based IPC between shim and runtime daemon: - Shim writes "width height" to /run/reaper//resize (or exec variant) - Runtime daemon polls the resize file every 100ms via a watcher thread - On change, daemon applies TIOCSWINSZ ioctl to the PTY master fd Supports both do_start() and exec_with_pty() PTY sessions. Closes #56 Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 2 +- docs/CURRENT_STATE.md | 2 +- docs/SHIMV2_DESIGN.md | 2 +- src/bin/containerd-shim-reaper-v2/main.rs | 28 +++++- src/bin/reaper-runtime/main.rs | 100 +++++++++++++++++++++- src/bin/reaper-runtime/state.rs | 12 +++ 6 files changed, 139 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a95e04b..704aa87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -366,7 +366,7 @@ cargo clippy --target x86_64-unknown-linux-gnu --all-targets ### 🔄 Known Limitations - Multi-container pods not fully tested -- ResizePty returns OK but is no-op (no dynamic PTY resize) +- ResizePty polling interval is 100ms (resize may not feel instant) - No cgroup resource limits (by design) - No namespace isolation (by design) - Volume mounts are shared across all workloads (no per-container isolation) diff --git a/docs/CURRENT_STATE.md b/docs/CURRENT_STATE.md index 3180541..466ec62 100644 --- a/docs/CURRENT_STATE.md +++ b/docs/CURRENT_STATE.md @@ -262,7 +262,7 @@ See [DEVELOPMENT.md](DEVELOPMENT.md) for development setup and [TESTING.md](TEST ### ⏳ Not Started - [ ] User/group ID management (currently disabled) - [ ] Signal handling robustness -- [ ] Dynamic PTY resize (ResizePty) +- [x] Dynamic PTY resize (ResizePty) - [ ] Resource monitoring (stats) - [ ] Performance optimization diff --git a/docs/SHIMV2_DESIGN.md b/docs/SHIMV2_DESIGN.md index acd94dd..61992ca 100644 --- a/docs/SHIMV2_DESIGN.md +++ b/docs/SHIMV2_DESIGN.md @@ -110,7 +110,7 @@ service Task { | Pause/Resume | ⚠️ | Returns OK but no-op (no cgroup freezer) | | Checkpoint | ⚠️ | Not implemented (no CRIU) | | Exec | ✅ | Implemented with PTY support | -| ResizePty | ⚠️ | Returns OK but no-op (no dynamic resize) | +| ResizePty | ✅ | Shim writes dimensions to resize file, runtime daemon applies via TIOCSWINSZ | | CloseIO | ⚠️ | Not implemented | | Update | ⚠️ | Not implemented (no cgroups) | diff --git a/src/bin/containerd-shim-reaper-v2/main.rs b/src/bin/containerd-shim-reaper-v2/main.rs index f9075e4..fcbc969 100644 --- a/src/bin/containerd-shim-reaper-v2/main.rs +++ b/src/bin/containerd-shim-reaper-v2/main.rs @@ -1486,8 +1486,32 @@ impl Task for ReaperTask { "resize_pty() called - container_id={}, exec_id={}, width={}, height={}", req.id, req.exec_id, req.width, req.height ); - // TODO: Propagate window size to PTY master (requires IPC with runtime daemon) - // For now, return success - terminal works but won't resize dynamically + + if req.width == 0 && req.height == 0 { + return Ok(api::Empty::new()); + } + + // Write resize dimensions to a file the runtime daemon polls. + // For exec processes, use exec-specific resize file. + let resize_file = if req.exec_id.is_empty() { + format!("{}/{}/resize", runtime_state_dir(), req.id) + } else { + format!( + "{}/{}/exec-{}-resize", + runtime_state_dir(), + req.id, + req.exec_id + ) + }; + + let content = format!("{} {}\n", req.width, req.height); + if let Err(e) = std::fs::write(&resize_file, content) { + warn!( + "resize_pty() - failed to write resize file {}: {}", + resize_file, e + ); + } + Ok(api::Empty::new()) } diff --git a/src/bin/reaper-runtime/main.rs b/src/bin/reaper-runtime/main.rs index a5bdf9a..24b9308 100644 --- a/src/bin/reaper-runtime/main.rs +++ b/src/bin/reaper-runtime/main.rs @@ -13,6 +13,8 @@ use state::{ delete as delete_state, load_exec_state, load_pid, load_state, save_exec_state, save_pid, save_state, ContainerState, OciUser, }; +#[cfg(target_os = "linux")] +use state::{exec_resize_path, resize_path}; #[cfg(target_os = "linux")] mod overlay; @@ -243,6 +245,58 @@ fn exit_code_from_status(status: std::process::ExitStatus) -> i32 { } } +/// Spawn a thread that polls a resize file and applies `TIOCSWINSZ` to the PTY master. +/// +/// The shim writes `"width height\n"` to `resize_file`. This thread polls every 100ms, +/// reads the dimensions, applies them via ioctl, then deletes the file. The thread exits +/// when `stop` is set to `true` (signaled by the caller after the child process exits). +#[cfg(target_os = "linux")] +fn spawn_resize_watcher( + master_raw_fd: i32, + resize_file: PathBuf, + stop: std::sync::Arc, +) { + std::thread::spawn(move || { + use std::sync::atomic::Ordering; + while !stop.load(Ordering::Relaxed) { + if resize_file.exists() { + if let Ok(content) = fs::read_to_string(&resize_file) { + let _ = fs::remove_file(&resize_file); + let parts: Vec<&str> = content.split_whitespace().collect(); + if parts.len() == 2 { + if let (Ok(width), Ok(height)) = + (parts[0].parse::(), parts[1].parse::()) + { + let ws = nix::libc::winsize { + ws_row: height, + ws_col: width, + ws_xpixel: 0, + ws_ypixel: 0, + }; + let ret = unsafe { + nix::libc::ioctl( + master_raw_fd, + nix::libc::TIOCSWINSZ as _, + &ws as *const nix::libc::winsize, + ) + }; + if ret < 0 { + tracing::warn!( + "TIOCSWINSZ failed: {}", + std::io::Error::last_os_error() + ); + } else { + tracing::debug!("PTY resized to {}x{}", width, height); + } + } + } + } + } + std::thread::sleep(std::time::Duration::from_millis(100)); + } + }); +} + #[allow(clippy::too_many_arguments)] fn do_create( id: &str, @@ -658,6 +712,13 @@ fn do_start(id: &str, bundle: &Path) -> Result<()> { // Close slave in parent - child has it via dup2 drop(pty.slave); + // Capture master raw fd for resize ioctl before converting to File + #[cfg(target_os = "linux")] + let master_raw_fd = { + use std::os::unix::io::AsRawFd; + pty.master.as_raw_fd() + }; + // Convert PTY master OwnedFd to File for I/O let master_file: std::fs::File = pty.master.into(); let master_clone = master_file.try_clone().unwrap_or_else(|e| { @@ -665,6 +726,17 @@ fn do_start(id: &str, bundle: &Path) -> Result<()> { std::process::exit(1); }); + // Start PTY resize watcher thread + #[cfg(target_os = "linux")] + let resize_stop = + std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + #[cfg(target_os = "linux")] + spawn_resize_watcher( + master_raw_fd, + resize_path(&container_id), + resize_stop.clone(), + ); + // Relay: stdin FIFO → PTY master (user input to process) if let Some(ref state) = io_state { if let Some(ref stdin_path) = state.stdin { @@ -746,6 +818,8 @@ fn do_start(id: &str, bundle: &Path) -> Result<()> { match child.wait() { Ok(exit_status) => { + #[cfg(target_os = "linux")] + resize_stop.store(true, std::sync::atomic::Ordering::Relaxed); let exit_code = exit_code_from_status(exit_status); if let Ok(mut state) = load_state(&container_id) { state.status = "stopped".into(); @@ -754,6 +828,8 @@ fn do_start(id: &str, bundle: &Path) -> Result<()> { } } Err(_e) => { + #[cfg(target_os = "linux")] + resize_stop.store(true, std::sync::atomic::Ordering::Relaxed); if let Ok(mut state) = load_state(&container_id) { state.status = "stopped".into(); state.exit_code = Some(1); @@ -1167,6 +1243,13 @@ fn exec_with_pty( // Close slave in parent - child has it via dup2 drop(pty.slave); + // Capture master raw fd for resize ioctl before converting to File + #[cfg(target_os = "linux")] + let master_raw_fd = { + use std::os::unix::io::AsRawFd; + pty.master.as_raw_fd() + }; + // Convert PTY master OwnedFd to File for I/O let master_file: std::fs::File = pty.master.into(); let master_clone = master_file.try_clone().unwrap_or_else(|e| { @@ -1174,6 +1257,16 @@ fn exec_with_pty( std::process::exit(1); }); + // Start PTY resize watcher thread + #[cfg(target_os = "linux")] + let resize_stop = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + #[cfg(target_os = "linux")] + spawn_resize_watcher( + master_raw_fd, + exec_resize_path(container_id, exec_id), + resize_stop.clone(), + ); + // Start relay threads // stdin FIFO → PTY master (user input to process) if let Some(ref stdin_p) = stdin_path { @@ -1226,10 +1319,13 @@ fn exec_with_pty( } // Wait for child - match child.wait() { + let exit = match child.wait() { Ok(status) => exit_code_from_status(status), Err(_) => 1, - } + }; + #[cfg(target_os = "linux")] + resize_stop.store(true, std::sync::atomic::Ordering::Relaxed); + exit } #[allow(clippy::too_many_arguments)] diff --git a/src/bin/reaper-runtime/state.rs b/src/bin/reaper-runtime/state.rs index edb0e4d..8d1507d 100644 --- a/src/bin/reaper-runtime/state.rs +++ b/src/bin/reaper-runtime/state.rs @@ -102,6 +102,18 @@ pub fn pid_path(id: &str) -> PathBuf { container_dir(id).join("pid") } +/// Path for PTY resize signaling (shim writes width/height, runtime reads) +#[cfg(target_os = "linux")] +pub fn resize_path(id: &str) -> PathBuf { + container_dir(id).join("resize") +} + +/// Path for exec PTY resize signaling +#[cfg(target_os = "linux")] +pub fn exec_resize_path(container_id: &str, exec_id: &str) -> PathBuf { + container_dir(container_id).join(format!("exec-{}-resize", exec_id)) +} + pub fn save_state(state: &ContainerState) -> anyhow::Result<()> { validate_id(&state.id)?; let dir = container_dir(&state.id);