From 461a5e998777620b6011c9cc49b227c2364ea3c7 Mon Sep 17 00:00:00 2001 From: Weidong Cui Date: Fri, 3 Apr 2026 16:34:56 -0700 Subject: [PATCH 1/2] FS/net/pipes layer: trait expansion, shutdown, UDP fixes, vfork-aware pipes Expand the FileSystem trait with ~15 new methods (open_at, stat_at, unlink_at, readlink_at, rename_at, mkdir_at, fd_path, rename, etc.) all with default Err(...) bodies so concrete implementations can land in later PRs. Add new error types (CreateAnonymousFileError, RenameError, ReadLinkError, SymlinkError, LinkError, ShutdownError) and extend existing enums with variants needed by the process layer (Interrupted, ClosedFd, NotADirectory, WouldBlock, SymlinkLoop). Network: add shutdown() with NetworkProxy delegation, fix UDP local_port leak (track allocation across bind/connect/sendto/close), fix TCP INADDR_ANY listen (use None instead of 0.0.0.0 for smoltcp wildcard), add ip_listen_endpoint_v4 helper. Pipes: implement Clone, add vfork-aware read with deadlock detection via fd_ref_count tracking, replace enable_fds_for_subsystem! macro with manual FdEnabledSubsystemEntry that overrides on_dup/on_close for ref counting, add readable_bytes() and Deadlock error variant. Use Release ordering on fd_ref_count mutations to pair with Acquire loads in the deadlock check. --- litebox/src/fs/devices.rs | 7 + litebox/src/fs/errors.rs | 91 +++++++++++- litebox/src/fs/layered.rs | 37 ++++- litebox/src/fs/mod.rs | 234 +++++++++++++++++++++++++++++- litebox/src/net/errors.rs | 10 ++ litebox/src/net/mod.rs | 160 ++++++++++++++------ litebox/src/net/socket_channel.rs | 16 ++ litebox/src/pipes.rs | 156 ++++++++++++++++++-- 8 files changed, 652 insertions(+), 59 deletions(-) diff --git a/litebox/src/fs/devices.rs b/litebox/src/fs/devices.rs index 9d58272b7..56c242372 100644 --- a/litebox/src/fs/devices.rs +++ b/litebox/src/fs/devices.rs @@ -27,6 +27,13 @@ const NULL_BLOCK_SIZE: usize = 0x1000; /// Block size for /dev/urandom const URANDOM_BLOCK_SIZE: usize = 0x1000; +/// Stored terminal attributes for a PTY pair. +/// +/// This is a re-export of [`crate::platform::TerminalAttributes`] — the same +/// type used by the platform abstraction for host stdio terminals. Using a +/// single type avoids duplication and simplifies conversions. +pub type PtyTermios = crate::platform::TerminalAttributes; + /// Constant node information for all 3 stdio devices: /// ```console /// $ stat -L --format 'name=%-11n dev=%d ino=%i rdev=%r' /dev/stdin /dev/stdout /dev/stderr diff --git a/litebox/src/fs/errors.rs b/litebox/src/fs/errors.rs index 3e5846596..76aec0d2f 100644 --- a/litebox/src/fs/errors.rs +++ b/litebox/src/fs/errors.rs @@ -27,6 +27,12 @@ pub enum OpenError { TruncateError(#[from] TruncateError), #[error("I/O error")] Io, + #[error("operation interrupted")] + Interrupted, + #[error("fd has been closed already")] + ClosedFd, + #[error("a component used as a directory in pathname is not, in fact, a directory")] + NotADirectory, #[error(transparent)] PathError(#[from] PathError), } @@ -34,7 +40,10 @@ pub enum OpenError { /// Possible errors from [`FileSystem::close`] #[non_exhaustive] #[derive(Error, Debug)] -pub enum CloseError {} +pub enum CloseError { + #[error("I/O error")] + Io, +} /// Possible errors from [`FileSystem::read`] #[non_exhaustive] @@ -48,6 +57,10 @@ pub enum ReadError { NotForReading, #[error("I/O error")] Io, + #[error("read would block")] + WouldBlock, + #[error("operation interrupted")] + Interrupted, } /// Possible errors from [`FileSystem::write`] @@ -62,6 +75,8 @@ pub enum WriteError { NotForWriting, #[error("I/O error")] Io, + #[error("operation interrupted")] + Interrupted, } /// Possible errors from [`FileSystem::seek`] @@ -141,6 +156,10 @@ pub enum UnlinkError { ReadOnlyFileSystem, #[error("I/O error")] Io, + #[error("fd has been closed already")] + ClosedFd, + #[error("a component used as a directory in pathname is not, in fact, a directory")] + NotADirectory, #[error(transparent)] PathError(#[from] PathError), } @@ -203,6 +222,10 @@ pub enum FileStatusError { ClosedFd, #[error("I/O error")] Io, + #[error("a component used as a directory in pathname is not, in fact, a directory")] + NotADirectory, + #[error("too many levels of symbolic links")] + SymlinkLoop, #[error(transparent)] PathError(#[from] PathError), } @@ -232,3 +255,69 @@ impl From for PathError { Self::InvalidPathname } } + +/// Possible errors from [`FileSystem::create_anonymous_file`] +#[non_exhaustive] +#[derive(Error, Debug)] +pub enum CreateAnonymousFileError { + #[error("the filesystem does not support anonymous files")] + NotSupported, + #[error("I/O error")] + Io, +} + +/// Possible errors from [`FileSystem::rename`] +#[non_exhaustive] +#[derive(Error, Debug)] +pub enum RenameError { + #[error("the parent directory does not allow write permission")] + NoWritePerms, + #[error("the named file resides on a read-only filesystem")] + ReadOnlyFileSystem, + #[error("I/O error")] + Io, + #[error("operation not supported")] + NotSupported, + #[error(transparent)] + PathError(#[from] PathError), +} + +/// Possible errors from [`FileSystem::read_link`] +#[non_exhaustive] +#[derive(Error, Debug)] +pub enum ReadLinkError { + #[error("the filesystem does not support symlinks")] + NotSupported, + #[error("I/O error")] + Io, + #[error(transparent)] + PathError(#[from] PathError), +} + +/// Possible errors from [`FileSystem::symlink`] +#[non_exhaustive] +#[derive(Error, Debug)] +pub enum SymlinkError { + #[error("the filesystem does not support symlinks")] + NotSupported, + #[error("path already exists")] + AlreadyExists, + #[error("I/O error")] + Io, + #[error(transparent)] + PathError(#[from] PathError), +} + +/// Possible errors from [`FileSystem::link`] +#[non_exhaustive] +#[derive(Error, Debug)] +pub enum LinkError { + #[error("the filesystem does not support hard links")] + NotSupported, + #[error("path already exists")] + AlreadyExists, + #[error("I/O error")] + Io, + #[error(transparent)] + PathError(#[from] PathError), +} diff --git a/litebox/src/fs/layered.rs b/litebox/src/fs/layered.rs index f1ceff00b..10035f40a 100644 --- a/litebox/src/fs/layered.rs +++ b/litebox/src/fs/layered.rs @@ -145,11 +145,17 @@ impl unreachable!(), + Err(FileStatusError::NotADirectory) => { + unimplemented!() + } + Err(FileStatusError::SymlinkLoop) => { + unimplemented!() + } Err(FileStatusError::PathError(PathError::ComponentNotADirectory)) => { unimplemented!() } @@ -208,6 +214,9 @@ impl unreachable!(), OpenError::PathError(path_error) => return Err(path_error)?, }, @@ -260,7 +269,10 @@ impl unreachable!(), + ReadError::ClosedFd + | ReadError::NotForReading + | ReadError::WouldBlock + | ReadError::Interrupted => unreachable!(), ReadError::Io => return Err(MigrationError::Io), }, } @@ -538,6 +550,9 @@ impl< | OpenError::NoWritePerms | OpenError::ReadOnlyFileSystem | OpenError::AlreadyExists + | OpenError::Interrupted + | OpenError::ClosedFd + | OpenError::NotADirectory | OpenError::TruncateError( TruncateError::IsDirectory | TruncateError::NotForWriting @@ -958,6 +973,8 @@ impl< Err(FileStatusError::Io) => return Err(ChmodError::Io), Err(FileStatusError::PathError(e)) => return Err(ChmodError::PathError(e)), Err(FileStatusError::ClosedFd) => unreachable!(), + Err(FileStatusError::NotADirectory) => unimplemented!(), + Err(FileStatusError::SymlinkLoop) => unimplemented!(), } match self.migrate_file_up(&path, true) { Ok(()) => {} @@ -1003,6 +1020,8 @@ impl< Err(FileStatusError::Io) => return Err(ChownError::Io), Err(FileStatusError::PathError(e)) => return Err(ChownError::PathError(e)), Err(FileStatusError::ClosedFd) => unreachable!(), + Err(FileStatusError::NotADirectory) => unimplemented!(), + Err(FileStatusError::SymlinkLoop) => unimplemented!(), } match self.migrate_file_up(&path, true) { Ok(()) => {} @@ -1035,6 +1054,8 @@ impl< | UnlinkError::Io | UnlinkError::IsADirectory | UnlinkError::ReadOnlyFileSystem + | UnlinkError::ClosedFd + | UnlinkError::NotADirectory | UnlinkError::PathError( PathError::ComponentNotADirectory | PathError::InvalidPathname @@ -1048,9 +1069,10 @@ impl< // We must now check if the lower level contains the file; if it does not, we // must exit with failure. Otherwise, we fallthrough to place the tombstone. match self.ensure_lower_contains(&path).map_err(|e| match e { - FileStatusError::Io => UnlinkError::Io, + FileStatusError::Io | FileStatusError::SymlinkLoop => UnlinkError::Io, FileStatusError::PathError(p) => UnlinkError::PathError(p), FileStatusError::ClosedFd => unreachable!(), + FileStatusError::NotADirectory => UnlinkError::NotADirectory, })? { FileType::RegularFile => { // fallthrough @@ -1058,7 +1080,7 @@ impl< FileType::Directory => { return Err(UnlinkError::IsADirectory); } - FileType::CharacterDevice => unimplemented!(), + FileType::CharacterDevice | FileType::Symlink => unimplemented!(), } } }, @@ -1138,6 +1160,9 @@ impl< } OpenError::NoWritePerms | OpenError::AlreadyExists + | OpenError::Interrupted + | OpenError::ClosedFd + | OpenError::NotADirectory | OpenError::TruncateError(_) => { unreachable!() } @@ -1315,7 +1340,9 @@ impl< // None of these can be handled by lower level, just quit out early return Err(e); } - FileStatusError::Io => return Err(e), + FileStatusError::Io + | FileStatusError::NotADirectory + | FileStatusError::SymlinkLoop => return Err(e), FileStatusError::PathError( PathError::NoSuchFileOrDirectory | PathError::MissingComponent, ) => { diff --git a/litebox/src/fs/mod.rs b/litebox/src/fs/mod.rs index d8b3d6a2e..6013486c8 100644 --- a/litebox/src/fs/mod.rs +++ b/litebox/src/fs/mod.rs @@ -3,7 +3,8 @@ //! File-system related functionality -use crate::fd::{FdEnabledSubsystem, TypedFd}; +use crate::event::IOPollable; +use crate::fd::{FdEnabledSubsystem, MetadataError, TypedFd}; use crate::path; use alloc::vec::Vec; @@ -24,7 +25,7 @@ mod tests; use errors::{ ChmodError, ChownError, CloseError, FileStatusError, MkdirError, OpenError, ReadDirError, - ReadError, RmdirError, SeekError, TruncateError, UnlinkError, WriteError, + ReadError, RenameError, RmdirError, SeekError, TruncateError, UnlinkError, WriteError, }; /// A private module, to help support writing sealed traits. This module should _itself_ never be @@ -44,6 +45,15 @@ mod private { /// However, users of any of these file systems might find benefit in having most of their code /// depend on this trait, rather than on any individual file system. pub trait FileSystem: private::Sealed + FdEnabledSubsystem { + /// Whether the FS backend automatically follows symlinks during walk. + /// + /// When `true`, callers should skip client-side `realpath`-like + /// canonicalization because the backend already resolves symlinks. + /// Defaults to `false` (conservative). + fn walks_follow_symlinks(&self) -> bool { + false + } + /// Opens a file /// /// The `mode` is only significant when creating a file @@ -54,6 +64,20 @@ pub trait FileSystem: private::Sealed + FdEnabledSubsystem { mode: Mode, ) -> Result, OpenError>; + /// Create an anonymous regular file that has no namespace entry. + /// + /// This is used for Linux `memfd_create`-style descriptors: the file + /// behaves like an ordinary seekable regular file, but only the returned + /// file descriptor keeps it alive. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn create_anonymous_file( + &self, + name: &str, + mode: Mode, + ) -> Result, errors::CreateAnonymousFileError> { + Err(errors::CreateAnonymousFileError::NotSupported) + } + /// Close the file at `fd`. /// /// Future operations on the `fd` will start to return `ClosedFd` errors. @@ -120,6 +144,15 @@ pub trait FileSystem: private::Sealed + FdEnabledSubsystem { /// Unlink a file fn unlink(&self, path: impl path::Arg) -> Result<(), UnlinkError>; + /// Rename (move) a file or directory + fn rename( + &self, + _old_path: impl path::Arg, + _new_path: impl path::Arg, + ) -> Result<(), RenameError> { + Err(RenameError::Io) + } + /// Create a new directory fn mkdir(&self, path: impl path::Arg, mode: Mode) -> Result<(), MkdirError>; @@ -147,6 +180,201 @@ pub trait FileSystem: private::Sealed + FdEnabledSubsystem { fn get_static_backing_data(&self, fd: &TypedFd) -> Option<&'static [u8]> { None } + + /// Check whether the given fd was opened with write access (`O_WRONLY` or + /// `O_RDWR`). + /// + /// This is a pure metadata query with no I/O side effects. The default + /// implementation conservatively returns `true`. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn is_writable(&self, fd: &TypedFd) -> bool { + true + } + + /// Synchronize per-open status flags to the backing file description. + /// + /// Most filesystem backends can ignore this because status flags are only + /// tracked by higher layers for `F_GETFL`. Device-style backends that + /// implement per-open blocking behavior should override it so `O_NONBLOCK` + /// and similar flags remain visible on the real backing fd. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn set_open_status_flags( + &self, + fd: &TypedFd, + flags: OFlags, + ) -> Result<(), MetadataError> { + Ok(()) + } + + /// Get an `IOPollable` for a file descriptor, if the underlying device supports polling. + /// + /// Returns `Some(pollable)` for device types with async event support (e.g., PTY master), + /// or `None` for regular files that don't support async I/O notifications. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn get_io_pollable(&self, fd: &TypedFd) -> Option> { + None + } + + /// Get stored PTY termios for a file descriptor. + /// + /// Returns `Some(termios)` if the fd refers to a PTY device, `None` otherwise. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn get_pty_termios(&self, fd: &TypedFd) -> Option { + None + } + + /// Set stored PTY termios for a file descriptor. + /// + /// Returns `true` if the fd refers to a PTY device and the termios was updated, + /// `false` otherwise. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn set_pty_termios(&self, fd: &TypedFd, termios: devices::PtyTermios) -> bool { + false + } + + /// Get the foreground process group for a PTY file descriptor. + /// + /// Returns `Some(pgrp)` if the fd refers to a PTY device, `None` otherwise. + /// A value of `0` means no foreground pgrp has been set yet. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn get_pty_foreground_pgrp(&self, fd: &TypedFd) -> Option { + None + } + + /// Set the foreground process group for a PTY file descriptor. + /// + /// Returns `true` if the fd refers to a PTY device and the pgrp was updated, + /// `false` otherwise. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn set_pty_foreground_pgrp(&self, fd: &TypedFd, pgrp: i32) -> bool { + false + } + + /// Read the target of a symbolic link. + /// + /// Returns the link target as a string. The default implementation returns + /// [`ReadLinkError::NotSupported`](errors::ReadLinkError::NotSupported), + /// since most in-memory filesystems don't have symlinks. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn read_link( + &self, + path: impl path::Arg, + ) -> Result { + Err(errors::ReadLinkError::NotSupported) + } + + /// Create a symbolic link. + /// + /// Creates a symlink at `linkpath` pointing to `target`. The default + /// implementation returns + /// [`SymlinkError::NotSupported`](errors::SymlinkError::NotSupported), + /// since most in-memory filesystems don't support symlinks. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn symlink( + &self, + target: impl path::Arg, + linkpath: impl path::Arg, + ) -> Result<(), errors::SymlinkError> { + Err(errors::SymlinkError::NotSupported) + } + + /// Create a hard link. + /// + /// Creates a new directory entry `newpath` that refers to the same inode + /// as `oldpath`. The default implementation returns + /// [`LinkError::NotSupported`](errors::LinkError::NotSupported). + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn link( + &self, + oldpath: impl path::Arg, + newpath: impl path::Arg, + ) -> Result<(), errors::LinkError> { + Err(errors::LinkError::NotSupported) + } + + // -- fd-relative (`*_at`) methods -- + // + // These resolve a relative path starting from a directory file descriptor. + // The path is stored in each FS Descriptor at open time; implementations + // join it with the relative component and delegate to path-based methods. + // + // Default implementations return `NotSupported` / `NotFound`-style errors + // so that the trait expansion can land before concrete implementations. + + /// Open a file relative to a directory fd. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn open_at( + &self, + dirfd: &TypedFd, + rel_path: impl path::Arg, + flags: OFlags, + mode: Mode, + ) -> Result, OpenError> { + Err(OpenError::Io) + } + + /// Obtain the status of a file relative to a directory fd. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn stat_at( + &self, + dirfd: &TypedFd, + rel_path: impl path::Arg, + follow_symlinks: bool, + ) -> Result { + Err(FileStatusError::Io) + } + + /// Unlink a file relative to a directory fd. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn unlink_at( + &self, + dirfd: &TypedFd, + rel_path: impl path::Arg, + ) -> Result<(), UnlinkError> { + Err(UnlinkError::Io) + } + + /// Read a symbolic link relative to a directory fd. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn readlink_at( + &self, + dirfd: &TypedFd, + rel_path: impl path::Arg, + ) -> Result { + Err(errors::ReadLinkError::NotSupported) + } + + /// Rename a file, with source and destination relative to directory fds. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn rename_at( + &self, + old_dirfd: &TypedFd, + old_rel: impl path::Arg, + new_dirfd: &TypedFd, + new_rel: impl path::Arg, + ) -> Result<(), RenameError> { + Err(RenameError::NotSupported) + } + + /// Create a directory relative to a directory fd. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn mkdir_at( + &self, + dirfd: &TypedFd, + rel_path: impl path::Arg, + mode: Mode, + ) -> Result<(), MkdirError> { + Err(MkdirError::Io) + } + + /// Get the path associated with an open file descriptor, if available. + /// + /// Returns the path that was used to open the file. Used by the ELF + /// patch cache and diagnostics. + #[expect(unused_variables, reason = "default body, non-underscored param names")] + fn fd_path(&self, fd: &TypedFd) -> Option { + None + } } bitflags! { @@ -198,6 +426,7 @@ pub enum FileType { RegularFile, Directory, CharacterDevice, + Symlink, } bitflags! { @@ -287,6 +516,7 @@ pub enum SeekWhence { /// elements might be added to this struct, allowing file systems to provide richer information /// about the status of files. However, users of LiteBox must not depend on the completeness or even /// layout of this particular type. +#[derive(Clone)] #[non_exhaustive] pub struct FileStatus { /// File type diff --git a/litebox/src/net/errors.rs b/litebox/src/net/errors.rs index 43b9ebc63..43813128f 100644 --- a/litebox/src/net/errors.rs +++ b/litebox/src/net/errors.rs @@ -99,6 +99,16 @@ pub enum ListenError { NoAvailableFreeEphemeralPorts, } +/// Possible errors from [`Network::shutdown`] +#[non_exhaustive] +#[derive(Error, Clone, Copy, Debug)] +pub enum ShutdownError { + #[error("Not a valid open file descriptor")] + InvalidFd, + #[error("Socket is not connected")] + NotConnected, +} + /// Possible errors from [`Network::accept`] #[non_exhaustive] #[derive(Error, Debug)] diff --git a/litebox/src/net/mod.rs b/litebox/src/net/mod.rs index c0709fcf9..f7bcb2a11 100644 --- a/litebox/src/net/mod.rs +++ b/litebox/src/net/mod.rs @@ -85,6 +85,14 @@ where Platform: platform::IPInterfaceProvider + platform::TimeProvider + sync::RawSyncPrimitivesProvider, { + fn ip_listen_endpoint_v4(addr: SocketAddrV4, port: u16) -> smoltcp::wire::IpListenEndpoint { + smoltcp::wire::IpListenEndpoint { + addr: (!addr.ip().is_unspecified()) + .then_some(smoltcp::wire::IpAddress::Ipv4(*addr.ip())), + port, + } + } + /// Construct a new `Network` instance /// /// This function is expected to only be invoked once per platform, as an initialization step, @@ -271,6 +279,8 @@ impl TcpServerSpecific { /// Socket-specific data for UDP sockets pub(crate) struct UdpSpecific { + /// A local port associated with this socket, if any. + local_port: Option, /// Remote endpoint /// /// If `connect`-ed, this is the remote endpoint to which packets are sent by default. @@ -783,6 +793,7 @@ where connect_initiated_at_us: None, }), Protocol::Udp => ProtocolSpecific::Udp(UdpSpecific { + local_port: None, remote_endpoint: None, }), Protocol::Icmp => unimplemented!(), @@ -821,6 +832,44 @@ where true } + /// Shutdown part of a full-duplex connection. + /// + /// `read` controls whether the read side is shut down, and `write` controls + /// the write side. At least one must be true. + pub fn shutdown( + &mut self, + fd: &SocketFd, + read: bool, + write: bool, + ) -> Result<(), errors::ShutdownError> { + let table = self.litebox.descriptor_table(); + let table_entry = table + .get_entry(fd) + .ok_or(errors::ShutdownError::InvalidFd)?; + let socket_handle = &table_entry.entry; + let proxy = socket_handle + .proxy + .as_ref() + .ok_or(errors::ShutdownError::NotConnected)?; + + if read { + proxy.shutdown_read(); + } + if write { + proxy.shutdown_write(); + // For TCP, initiate a graceful close (FIN) on the smoltcp socket. + if let Protocol::Tcp = socket_handle.protocol() { + let tcp_socket: &mut tcp::Socket = self.socket_set.get_mut(socket_handle.handle); + tcp_socket.close(); + } + } + + drop(table_entry); + drop(table); + self.automated_platform_interaction(PollDirection::Both); + Ok(()) + } + /// Close the socket at `fd` pub fn close( &mut self, @@ -893,6 +942,9 @@ where let _ = self.socket_set.remove(handle); } Protocol::Udp => { + if let Some(local_port) = specific.udp_mut().local_port.take() { + self.local_port_allocator.deallocate(local_port); + } let smoltcp::socket::Socket::Udp(mut socket) = self.socket_set.remove(handle) else { unreachable!() @@ -992,13 +1044,26 @@ where if addr.port() == 0 { return Err(ConnectError::Unaddressable); } - let socket: &mut udp::Socket = self.socket_set.get_mut(socket_handle.handle); - if !socket.is_open() { - let local_port = self.local_port_allocator.ephemeral_port()?; - let local_endpoint: smoltcp::wire::IpListenEndpoint = local_port.port().into(); - let Ok(()) = socket.bind(local_endpoint) else { - unreachable!("binding to a free port cannot fail") - }; + let new_local_port = { + let socket: &mut udp::Socket = self.socket_set.get_mut(socket_handle.handle); + if socket.is_open() { + None + } else { + let local_port = self.local_port_allocator.ephemeral_port()?; + let local_endpoint: smoltcp::wire::IpListenEndpoint = + local_port.port().into(); + let Ok(()) = socket.bind(local_endpoint) else { + unreachable!("binding to a free port cannot fail") + }; + Some(local_port) + } + }; + if let Some(local_port) = new_local_port { + let old_port = socket_handle.udp_mut().local_port.replace(local_port); + debug_assert!(old_port.is_none()); + if let Some(old_port) = old_port { + self.local_port_allocator.deallocate(old_port); + } } let addr: smoltcp::wire::IpEndpoint = (*addr).into(); socket_handle.udp_mut().remote_endpoint = Some(addr); @@ -1146,31 +1211,33 @@ where unimplemented!() } socket_handle.tcp_mut().server_socket = Some(TcpServerSpecific { - ip_listen_endpoint: smoltcp::wire::IpListenEndpoint { - addr: Some(smoltcp::wire::IpAddress::Ipv4(*addr.ip())), - port: new_port, - }, + ip_listen_endpoint: Self::ip_listen_endpoint_v4(*addr, new_port), backlog: None, socket_set_handles: vec![], }); } Protocol::Udp => { - let lp = self - .local_port_allocator - .allocate_local_port(addr.port()) - .map_err(|_| BindError::PortAlreadyInUse(addr.port()))?; - let local_endpoint = smoltcp::wire::IpListenEndpoint { - addr: Some(smoltcp::wire::IpAddress::Ipv4(*addr.ip())), - port: lp.port(), - }; - let socket: &mut udp::Socket = self.socket_set.get_mut(socket_handle.handle); - socket.bind(local_endpoint).map_err(|e| { - self.local_port_allocator.deallocate(lp); - match e { - udp::BindError::InvalidState => BindError::AlreadyBound, - udp::BindError::Unaddressable => unreachable!(), + let new_local_port = { + let local_port = self + .local_port_allocator + .allocate_local_port(addr.port()) + .map_err(|_| BindError::PortAlreadyInUse(addr.port()))?; + let local_endpoint = Self::ip_listen_endpoint_v4(*addr, local_port.port()); + let socket: &mut udp::Socket = self.socket_set.get_mut(socket_handle.handle); + if let Err(e) = socket.bind(local_endpoint) { + self.local_port_allocator.deallocate(local_port); + return Err(match e { + udp::BindError::InvalidState => BindError::AlreadyBound, + udp::BindError::Unaddressable => unreachable!(), + }); } - })?; + local_port + }; + let old_port = socket_handle.udp_mut().local_port.replace(new_local_port); + debug_assert!(old_port.is_none()); + if let Some(old_port) = old_port { + self.local_port_allocator.deallocate(old_port); + } } Protocol::Icmp => unimplemented!(), Protocol::Raw { protocol: _ } => unimplemented!(), @@ -1232,10 +1299,7 @@ where unimplemented!() } handle.server_socket = Some(TcpServerSpecific { - ip_listen_endpoint: smoltcp::wire::IpListenEndpoint { - addr: Some(smoltcp::wire::IpAddress::v4(0, 0, 0, 0)), - port, - }, + ip_listen_endpoint: smoltcp::wire::IpListenEndpoint { addr: None, port }, backlog: None, socket_set_handles: vec![], }); @@ -1398,20 +1462,34 @@ where let Some(remote_endpoint) = destination else { return Err(SendError::DestinationAddressRequired); }; - let udp_socket: &mut udp::Socket = self.socket_set.get_mut(socket_handle.handle); - if !udp_socket.is_open() { - let Ok(()) = udp_socket.bind(smoltcp::wire::IpListenEndpoint { - addr: None, - port: self + let new_local_port = { + let udp_socket: &mut udp::Socket = + self.socket_set.get_mut(socket_handle.handle); + if udp_socket.is_open() { + None + } else { + let local_port = self .local_port_allocator .ephemeral_port() - .map_err(SendError::PortAllocationFailure)? - .port(), - }) else { - unreachable!("binding to a free port cannot fail") - }; + .map_err(SendError::PortAllocationFailure)?; + let Ok(()) = udp_socket.bind(smoltcp::wire::IpListenEndpoint { + addr: None, + port: local_port.port(), + }) else { + unreachable!("binding to a free port cannot fail") + }; + Some(local_port) + } + }; + if let Some(local_port) = new_local_port { + let old_port = socket_handle.udp_mut().local_port.replace(local_port); + debug_assert!(old_port.is_none()); + if let Some(old_port) = old_port { + self.local_port_allocator.deallocate(old_port); + } } - udp_socket + self.socket_set + .get_mut::(socket_handle.handle) .send_slice(buf, remote_endpoint) .map(|()| buf.len()) .map_err(|e| match e { diff --git a/litebox/src/net/socket_channel.rs b/litebox/src/net/socket_channel.rs index 5d75f28cc..afcd3ba60 100644 --- a/litebox/src/net/socket_channel.rs +++ b/litebox/src/net/socket_channel.rs @@ -237,6 +237,22 @@ impl NetworkProxy NetworkProxy::Raw => unimplemented!(), } } + + /// Shutdown the read side of the socket. + pub fn shutdown_read(&self) { + match self { + NetworkProxy::Stream(channel) => channel.shutdown_read(), + NetworkProxy::Datagram(_) | NetworkProxy::Raw => {} + } + } + + /// Shutdown the write side of the socket. + pub fn shutdown_write(&self) { + match self { + NetworkProxy::Stream(channel) => channel.shutdown_write(), + NetworkProxy::Datagram(_) | NetworkProxy::Raw => {} + } + } } impl IOPollable for NetworkProxy { fn register_observer(&self, observer: alloc::sync::Weak>, mask: Events) { diff --git a/litebox/src/pipes.rs b/litebox/src/pipes.rs index 000e8efc7..64dd16aea 100644 --- a/litebox/src/pipes.rs +++ b/litebox/src/pipes.rs @@ -6,7 +6,7 @@ use core::{ num::NonZeroUsize, sync::atomic::{ - AtomicBool, AtomicU32, + AtomicBool, AtomicU32, AtomicUsize, Ordering::{self, Relaxed}, }, }; @@ -36,6 +36,14 @@ pub struct Pipes { litebox: LiteBox, } +impl Clone for Pipes { + fn clone(&self) -> Self { + Self { + litebox: self.litebox.clone(), + } + } +} + impl Pipes { /// Construct a new `Pipes` instance. /// @@ -96,6 +104,31 @@ impl Pipes { cx: &WaitContext<'_, Platform>, fd: &PipeFd, buf: &mut [u8], + ) -> Result { + self.read_inner(cx, fd, buf, false) + } + + /// Like [`read`](Self::read), but with vfork awareness. + /// + /// When `is_vfork_child` is true and a blocking read would deadlock + /// (the only remaining write-end FD belongs to the blocked parent), + /// returns `Err(ReadError::Deadlock)` instead of blocking forever. + pub fn read_vfork_aware( + &self, + cx: &WaitContext<'_, Platform>, + fd: &PipeFd, + buf: &mut [u8], + is_vfork_child: bool, + ) -> Result { + self.read_inner(cx, fd, buf, is_vfork_child) + } + + fn read_inner( + &self, + cx: &WaitContext<'_, Platform>, + fd: &PipeFd, + buf: &mut [u8], + is_vfork_child: bool, ) -> Result { let dt = self.litebox.descriptor_table(); let p = match &dt.get_entry(fd).ok_or(errors::ReadError::ClosedFd)?.entry { @@ -103,7 +136,11 @@ impl Pipes { PipeEnd::Sender(_) => return Err(errors::ReadError::NotForReading), }; drop(dt); - p.read(cx, buf).map_err(From::from) + if is_vfork_child { + p.read_vfork_aware(cx, buf).map_err(From::from) + } else { + p.read(cx, buf).map_err(From::from) + } } /// Write the values in `buf` into the pipe, returning the number of elements written. @@ -175,6 +212,16 @@ impl Pipes { PipeEnd::Sender(p) => Ok(f(p)), } } + + /// Return the number of bytes available for reading on a pipe receiver. + /// Returns 0 for sender ends. + pub fn readable_bytes(&self, fd: &PipeFd) -> Result { + let dt = self.litebox.descriptor_table(); + match &dt.get_entry(fd).ok_or(errors::ClosedError::ClosedFd)?.entry { + PipeEnd::Receiver(p) => Ok(p.endpoint.rb.lock().occupied_len()), + PipeEnd::Sender(_) => Ok(0), + } + } } /// Whether a particular pipe end is the sender half or the receiver half @@ -243,6 +290,8 @@ pub mod errors { WouldBlock, #[error("wait error")] WaitError(WaitError), + #[error("would deadlock: the only writer is the suspended vfork parent")] + Deadlock, } /// Possible errors from [`Pipes::write`] @@ -338,6 +387,11 @@ struct WriteEnd { status: AtomicU32, /// Slice length that is guaranteed to be an atomic write (i.e., non-interleaved). atomic_slice_guarantee_size: usize, + /// Number of open file descriptors referencing this write end. + /// Starts at 1 when created, incremented by fork, decremented by close. + /// When this reaches 0, the write end is shut down and the read peer + /// receives HUP — even if the Arc itself hasn't been dropped yet. + fd_ref_count: AtomicUsize, } /// Potential errors when writing or reading from a pipe @@ -352,6 +406,8 @@ enum PipeError { WouldBlock, #[error("wait error")] WaitError(WaitError), + #[error("would deadlock: the only writer is the suspended vfork parent")] + Deadlock, } impl From for errors::ReadError { @@ -363,6 +419,7 @@ impl From for errors::ReadError { } PipeError::WouldBlock => errors::ReadError::WouldBlock, PipeError::WaitError(e) => errors::ReadError::WaitError(e), + PipeError::Deadlock => errors::ReadError::Deadlock, } } } @@ -373,6 +430,7 @@ impl From for errors::WriteError { PipeError::PeerShutdown => errors::WriteError::ReadEndClosed, PipeError::WouldBlock => errors::WriteError::WouldBlock, PipeError::WaitError(e) => errors::WriteError::WaitError(e), + PipeError::Deadlock => unreachable!("deadlock detection is read-side only"), } } } @@ -394,6 +452,7 @@ impl WriteEnd ReadEnd .map_err(PipeError::from) } + /// Read with vfork deadlock avoidance. + /// + /// When the pipe buffer is empty and the only remaining write-end FD + /// belongs to a vfork-blocked parent, a normal blocking read would + /// deadlock (parent waits for child, child waits for pipe data that + /// only the parent could produce by closing its end). In this case, + /// return `Ok(0)` (EOF) instead of blocking. + fn read_vfork_aware( + &self, + cx: &WaitContext<'_, Platform>, + buf: &mut [T], + ) -> Result + where + T: Copy, + { + match self.try_read(buf) { + Ok(n) => return Ok(n), + Err(TryOpError::TryAgain) => {} + Err(e) => return Err(PipeError::from(e)), + } + + // The pipe buffer is empty and the write-end is not shut down. + // fd_ref_count tracks the number of open FDs for the write-end + // (incremented on dup/fork, decremented on close). After the vfork + // child closes its write-end copy, if fd_ref_count == 1, the sole + // remaining writer is the blocked parent — a blocking read would + // deadlock. Return EDEADLK so the caller fails loudly instead of + // silently receiving a fake EOF. + if self + .peer + .upgrade() + .is_some_and(|p| p.fd_ref_count.load(Ordering::Acquire) <= 1) + { + return Err(PipeError::Deadlock); + } + + // Standard blocking path. + self.endpoint + .pollee + .wait( + cx, + self.get_status().contains(OFlags::NONBLOCK), + Events::IN, + || self.try_read(buf), + ) + .map_err(PipeError::from) + } + common_functions_for_channel!(); } @@ -620,6 +727,43 @@ fn new_pipe( (producer, consumer) } +// Manual implementation of FD subsystem integration for pipes. +// We can't use the `enable_fds_for_subsystem!` macro here because we need +// to override `on_dup()` and `on_close()` to properly track pipe writer +// file descriptor counts across dup/fork. +#[doc(hidden)] +pub struct DescriptorEntry { + entry: PipeEnd, +} +impl crate::fd::FdEnabledSubsystem + for Pipes +{ + type Entry = DescriptorEntry; +} +impl crate::fd::FdEnabledSubsystemEntry + for DescriptorEntry +{ + fn on_dup(&self) { + if let PipeEnd::Sender(w) = &self.entry { + w.fd_ref_count.fetch_add(1, Ordering::Release); + } + } + + fn on_close(&self) { + if let PipeEnd::Sender(w) = &self.entry { + w.fd_ref_count.fetch_sub(1, Ordering::Release); + } + } +} +impl From> + for DescriptorEntry +{ + fn from(entry: PipeEnd) -> Self { + Self { entry } + } +} +pub type PipeFd = crate::fd::TypedFd>; + #[cfg(test)] mod tests { use crate::{ @@ -721,11 +865,3 @@ mod tests { }); } } - -crate::fd::enable_fds_for_subsystem! { - @Platform: { RawSyncPrimitivesProvider + TimeProvider }; - Pipes; - @Platform: { RawSyncPrimitivesProvider + TimeProvider }; - PipeEnd; - -> PipeFd; -} From 046443d79d6588b0b313aae0822ae9525358fe8d Mon Sep 17 00:00:00 2001 From: Weidong Cui Date: Fri, 3 Apr 2026 17:14:38 -0700 Subject: [PATCH 2/2] Fix doc warning and remove PTY-specific methods from FileSystem trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Backtick-escape Arc in pipes.rs doc comment (rustdoc invalid_html_tags error). - Remove get/set_pty_termios and get/set_pty_foreground_pgrp from the FileSystem trait — these are PTY-device-specific and belong in the PTY subsystem layer, not the generic FS trait. - Remove PtyTermios type alias from devices.rs (no remaining callers). --- litebox/src/fs/devices.rs | 7 ------- litebox/src/fs/mod.rs | 35 ----------------------------------- litebox/src/pipes.rs | 2 +- 3 files changed, 1 insertion(+), 43 deletions(-) diff --git a/litebox/src/fs/devices.rs b/litebox/src/fs/devices.rs index 56c242372..9d58272b7 100644 --- a/litebox/src/fs/devices.rs +++ b/litebox/src/fs/devices.rs @@ -27,13 +27,6 @@ const NULL_BLOCK_SIZE: usize = 0x1000; /// Block size for /dev/urandom const URANDOM_BLOCK_SIZE: usize = 0x1000; -/// Stored terminal attributes for a PTY pair. -/// -/// This is a re-export of [`crate::platform::TerminalAttributes`] — the same -/// type used by the platform abstraction for host stdio terminals. Using a -/// single type avoids duplication and simplifies conversions. -pub type PtyTermios = crate::platform::TerminalAttributes; - /// Constant node information for all 3 stdio devices: /// ```console /// $ stat -L --format 'name=%-11n dev=%d ino=%i rdev=%r' /dev/stdin /dev/stdout /dev/stderr diff --git a/litebox/src/fs/mod.rs b/litebox/src/fs/mod.rs index 6013486c8..6b031034a 100644 --- a/litebox/src/fs/mod.rs +++ b/litebox/src/fs/mod.rs @@ -215,41 +215,6 @@ pub trait FileSystem: private::Sealed + FdEnabledSubsystem { None } - /// Get stored PTY termios for a file descriptor. - /// - /// Returns `Some(termios)` if the fd refers to a PTY device, `None` otherwise. - #[expect(unused_variables, reason = "default body, non-underscored param names")] - fn get_pty_termios(&self, fd: &TypedFd) -> Option { - None - } - - /// Set stored PTY termios for a file descriptor. - /// - /// Returns `true` if the fd refers to a PTY device and the termios was updated, - /// `false` otherwise. - #[expect(unused_variables, reason = "default body, non-underscored param names")] - fn set_pty_termios(&self, fd: &TypedFd, termios: devices::PtyTermios) -> bool { - false - } - - /// Get the foreground process group for a PTY file descriptor. - /// - /// Returns `Some(pgrp)` if the fd refers to a PTY device, `None` otherwise. - /// A value of `0` means no foreground pgrp has been set yet. - #[expect(unused_variables, reason = "default body, non-underscored param names")] - fn get_pty_foreground_pgrp(&self, fd: &TypedFd) -> Option { - None - } - - /// Set the foreground process group for a PTY file descriptor. - /// - /// Returns `true` if the fd refers to a PTY device and the pgrp was updated, - /// `false` otherwise. - #[expect(unused_variables, reason = "default body, non-underscored param names")] - fn set_pty_foreground_pgrp(&self, fd: &TypedFd, pgrp: i32) -> bool { - false - } - /// Read the target of a symbolic link. /// /// Returns the link target as a string. The default implementation returns diff --git a/litebox/src/pipes.rs b/litebox/src/pipes.rs index 64dd16aea..32709aae6 100644 --- a/litebox/src/pipes.rs +++ b/litebox/src/pipes.rs @@ -390,7 +390,7 @@ struct WriteEnd { /// Number of open file descriptors referencing this write end. /// Starts at 1 when created, incremented by fork, decremented by close. /// When this reaches 0, the write end is shut down and the read peer - /// receives HUP — even if the Arc itself hasn't been dropped yet. + /// receives HUP — even if the `Arc` itself hasn't been dropped yet. fd_ref_count: AtomicUsize, }