A shared region's last handle takes its list of mappings, and a map that finds none is refused

issues/a-region-keeps-the-address-space-of-every-process-that-ended-with-it-mapped.md

@@ -0,0 +1,45 @@
+---
+status: open
+kind: defect
+opened: 2026-10-08
+---
+
+# A region keeps the address space of every process that ended with it mapped
+
+`SharedMemObject::map_into` (`kernel/src/object/shm.rs`) records each mapping
+as `(Pid, PageTables, UserAddr)`, and `PageTables` is the `Arc` of the
+process's address space. An entry leaves the list in two places only: the
+region's zero-handle hook, when the last handle to it goes anywhere, and
+`unmap_from`, which nothing but an inbox's teardown calls
+(`kernel/src/inbox/mod.rs`). A process's own teardown
+(`teardown_resources`, `kernel/src/process.rs`) closes its handles and
+touches no region's list.
+
+So a process that maps a region somebody else also holds, and ends, leaves
+its entry behind, and the entry keeps that process's `AddressSpace` alive
+until the region's last holder lets go: its page-table pages, whatever its
+`pages` map still owns, and its user PCID, which returns to the pool only
+when the space drops (`PcidGuard`, `kernel/src/arch/x86_64/paging.rs`). The
+list grows by one entry per such process and nothing bounds it. A region
+handle carries `DUP` and `TRANSFER`, so one process can keep a region and
+have child after child map it and end; the pool holds
+`kernel::pcid::MAX_USER_PCID` tags, and with every one held
+`AddressSpace::new_user` answers `None` and every spawn on the machine is
+refused `ResourceExhausted` (`kernel/src/loader/mod.rs`).
+
+**Read from the code, not run.** Pids are never reissued
+(`kernel/pure/proclife/pids.rs`), so a later process is never answered a dead
+one's address by the list's `find`.
+
+**Exit condition**: a process's teardown leaves no entry of it in any
+region's list — and a test in which a child maps a region its parent keeps
+and ends sees the child's address space freed, its PCID back in the pool,
+while the parent still holds the handle.
+
+**Owner**: `kernel::object::shm`'s handle-driven mapping teardown, with
+`issues/the-compositor-keeps-a-committed-regions-mapping-for-as-long-as-the-client-does.md`,
+whose exit ends a mapping at the close of its process's last handle and so
+reaches this one wherever the process closed that handle, before it ended or
+in its teardown. It does not reach a process that mapped the region and moved
+its handle on by `SYS_HANDLE_SEND` (`sys_handle_send`,
+`kernel/src/syscall/ipc.rs`), which is no close.

kernel/src/object/mod.rs

@@ -37,16 +37,26 @@ impl<T> Held<T> {
         Self(Lock::new(Some(value)))
     }
 
+    /// What is held, for a release that is more than a drop; `None` after the first.
+    pub(crate) fn take(&self) -> Option<T> {
+        self.0.lock().take()
+    }
+
     /// Drops what is held, outside the lock; `T`'s destructor must not re-enter it.
     pub(crate) fn release(&self) {
-        let taken = self.0.lock().take();
-        drop(taken);
+        drop(self.take());
     }
 
     /// `f` over what is held, under the lock, or `None` after the release.
     pub(crate) fn with<R>(&self, f: impl FnOnce(&T) -> R) -> Option<R> {
         self.0.lock().as_ref().map(f)
     }
+
+    /// [`with`](Self::with) for an `f` that changes what is held: the change and the release
+    /// are one lock's, so nothing is added to what a release already took.
+    pub(crate) fn with_mut<R>(&self, f: impl FnOnce(&mut T) -> R) -> Option<R> {
+        self.0.lock().as_mut().map(f)
+    }
 }
 
 impl<T: Clone> Held<T> {

kernel/src/object/shm.rs

@@ -1,8 +1,12 @@
 //! A region of memory more than one process can see.
 //!
 //! Holding a handle allows mapping it; giving one away is `SYS_HANDLE_SEND`.
-//! Mappings are torn down when the last handle goes; pages are freed when
-//! the last `Arc` goes, always later since a handle holds an `Arc`.
+//! The handle count owns the mappings and the `Arc` count owns the pages: the
+//! last handle takes the list of mappings away for good and tears each one
+//! down, and the pages are freed when the last `Arc` goes, always later since
+//! a handle holds an `Arc`. An `Arc` a syscall cloned out of its table can
+//! outlive the last handle, so a map through one finds no list and is refused
+//! under the lock the teardown took it with.
 
 use alloc::sync::Arc;
 use alloc::vec::Vec;
@@ -12,10 +16,9 @@ use toyos_abi::syscall::SyscallError;
 use crate::mm::policy::{CachePolicy, Prot};
 use crate::mm::{align_2m_checked, pmm, Unmapped, PAGE_2M};
 use crate::process::{PageTables, Pid};
-use crate::sync::Lock;
 use crate::{DirectMap, UserAddr};
 
-use super::{KObjectVariant, ObjectCore, ZeroHandles};
+use super::{Held, KObjectVariant, ObjectCore, ZeroHandles};
 
 /// Physical pages a region keeps alive; behind an `Arc` since one page set
 /// can back several objects.
@@ -52,8 +55,8 @@ impl Region {
 pub struct SharedMemObject {
     pub(super) core: ObjectCore,
     region: Region,
-    /// Where this region is mapped, per process; the zero-handle hook empties it.
-    mapped_in: Lock<Vec<(Pid, PageTables, UserAddr)>>,
+    /// Where this region is mapped, per process; released by the zero-handle hook.
+    mapped_in: Held<Vec<(Pid, PageTables, UserAddr)>>,
 }
 
 impl SharedMemObject {
@@ -68,7 +71,7 @@ impl SharedMemObject {
         Arc::new(Self {
             core: Self::new_core(),
             region,
-            mapped_in: Lock::new(Vec::new()),
+            mapped_in: Held::new(Vec::new()),
         })
     }
 
@@ -119,58 +122,66 @@ impl SharedMemObject {
     /// could otherwise write the same bytes while the kernel is still initialising.
     pub fn phys_before_mapping(&self) -> DirectMap {
         assert!(
-            self.mapped_in.lock().is_empty(),
+            self.mapped_nowhere(),
             "shm koid {}: the region is mapped into a process already, so a kernel \
              write through the direct map is not exclusive",
             self.core.koid().raw(),
         );
         self.region.phys
     }
 
+    fn mapped_nowhere(&self) -> bool {
+        self.mapped_in.with(|mapped| mapped.is_empty()).unwrap_or(true)
+    }
+
     /// Map into `pt`, or answer the address it is already mapped at;
-    /// idempotent per process.
+    /// idempotent per process. `Gone` once the last handle has gone.
     pub fn map_into(&self, pid: Pid, pt: &PageTables) -> Result<u64, SyscallError> {
-        let mut mapped = self.mapped_in.lock();
-        if let Some((_, _, vaddr)) = mapped.iter().find(|(p, _, _)| *p == pid) {
-            return Ok(vaddr.raw());
-        }
-        let (addr, _) = pt
-            .lock()
-            .alloc_and_map(self.region.phys.phys(), self.region.size, Prot::ReadWrite, self.region.cache)
-            .ok_or(SyscallError::ResourceExhausted)?;
-        // Logged only for a non-default policy: this process is the one
-        // paying for it. Read back the installed policy, not the request,
-        // so the line describes the mapping.
-        if self.region.cache != CachePolicy::Normal {
-            let installed = pt.lock().user_policy(addr).expect("shm: just mapped");
-            crate::log!(
-                "shm: {:#x} mapped {:?} into pid {}",
-                self.region.phys.phys(),
-                installed,
-                pid
-            );
-        }
-        mapped.push((pid, Arc::clone(pt), addr));
-        Ok(addr.raw())
+        let map = |mapped: &mut Vec<(Pid, PageTables, UserAddr)>| {
+            if let Some((_, _, vaddr)) = mapped.iter().find(|(p, _, _)| *p == pid) {
+                return Ok(vaddr.raw());
+            }
+            let (addr, _) = pt
+                .lock()
+                .alloc_and_map(self.region.phys.phys(), self.region.size, Prot::ReadWrite, self.region.cache)
+                .ok_or(SyscallError::ResourceExhausted)?;
+            // Logged only for a non-default policy: this process is the one
+            // paying for it. Read back the installed policy, not the request,
+            // so the line describes the mapping.
+            if self.region.cache != CachePolicy::Normal {
+                let installed = pt.lock().user_policy(addr).expect("shm: just mapped");
+                crate::log!(
+                    "shm: {:#x} mapped {:?} into pid {}",
+                    self.region.phys.phys(),
+                    installed,
+                    pid
+                );
+            }
+            mapped.push((pid, Arc::clone(pt), addr));
+            Ok(addr.raw())
+        };
+        self.mapped_in.with_mut(map).unwrap_or(Err(SyscallError::Gone))
     }
 
     /// Take this process's mapping away, if it has one; the caller owes a
     /// shootdown before the freed address can be reissued.
     #[must_use = "the caller owes a shootdown before the address can be reissued"]
     pub fn unmap_from(&self, pid: Pid) -> Option<Unmapped<()>> {
-        let mut mapped = self.mapped_in.lock();
-        let pos = mapped.iter().position(|(p, _, _)| *p == pid)?;
-        let (_, pt, vaddr) = mapped.swap_remove(pos);
-        pt.lock().free_and_unmap(vaddr);
-        Some(Unmapped::new(()))
+        self.mapped_in.with_mut(|mapped| {
+            let pos = mapped.iter().position(|(p, _, _)| *p == pid)?;
+            let (_, pt, vaddr) = mapped.swap_remove(pos);
+            pt.lock().free_and_unmap(vaddr);
+            Some(Unmapped::new(()))
+        })?
     }
 }
 
-/// Every mapping goes, flushed here; the pages do not, since a handle holds
-/// an `Arc` and `Region::pages` frees them only when the last one drops.
+/// Every mapping goes, flushed here, and with the list gone no later map can
+/// add one; the pages do not, since a handle holds an `Arc` and
+/// `Region::pages` frees them only when the last one drops.
 impl ZeroHandles for SharedMemObject {
     fn on_zero_handles(&self) {
-        let mapped = core::mem::take(&mut *self.mapped_in.lock());
+        let mapped = self.mapped_in.take().expect("the zero-handle hook runs once");
         if mapped.is_empty() {
             return;
         }
@@ -184,9 +195,9 @@ impl ZeroHandles for SharedMemObject {
 impl Drop for SharedMemObject {
     fn drop(&mut self) {
         debug_assert!(
-            self.mapped_in.lock().is_empty(),
-            "shm koid {} freed with a live mapping: the zero-handle hook did \
-             not run, so the pages go back to the PMM under somebody's window",
+            self.mapped_nowhere(),
+            "shm koid {} freed with a live mapping, so the pages go back to the \
+             PMM under somebody's window",
             self.core.koid().raw(),
         );
     }

toyos-abi/src/syscall.rs

@@ -1981,7 +1981,8 @@ pub fn shm_create(size: usize) -> Result<RawHandle, SyscallError> {
 
 /// Map the region `shm` names into this process. Needs [`Rights::MAP`].
 ///
-/// Idempotent: a second call answers the first call's address.
+/// Idempotent: a second call answers the first call's address. [`SyscallError::Gone`]
+/// when the region's last handle was closed under the call: nothing is mapped.
 ///
 /// [`Rights::MAP`]: crate::handle::Rights::MAP
 ///