diff --git a/bootstrap/src/initnsmgr.rs b/bootstrap/src/initnsmgr.rs index 12d678c4..cf1d8cf8 100644 --- a/bootstrap/src/initnsmgr.rs +++ b/bootstrap/src/initnsmgr.rs @@ -1,8 +1,6 @@ -use alloc::rc::Rc; use alloc::string::{String, ToString}; use alloc::sync::Arc; use alloc::vec::Vec; -use core::cell::RefCell; use core::fmt::Debug; use core::mem; use hashbrown::HashMap; @@ -11,6 +9,13 @@ use log::{error, warn}; use redox_path::RedoxPath; use redox_path::RedoxScheme; use redox_rt::proc::FdGuard; +// Namespace state is shared between the dispatcher and (from the worker-offload +// step) the open workers, so it uses redox_rt's Send/Sync Mutex instead of the +// single-threaded Rc. NOTE: this Mutex is a spinlock -- never hold it +// across a blocking syscall; resolve the cap_fd under the lock, clone the Arc, +// and do the blocking openat with the lock released. See +// local/docs/INITNSMGR-CONCURRENCY-DESIGN.md. +use redox_rt::sync::Mutex; use redox_scheme::{ CallerCtx, OpenResult, RequestKind, Response, SendFdRequest, SignalBehavior, Socket, scheme::{SchemeState, SchemeSync}, @@ -59,9 +64,11 @@ impl Namespace { } } -#[derive(Debug, Clone)] +// No `Debug` derive: redox_rt's Mutex wraps an UnsafeCell and is not Debug. +// `Clone` clones the Arc (a refcount bump), not the namespace. +#[derive(Clone)] struct NamespaceAccess { - namespace: Rc>, + namespace: Arc>, permission: NsPermissions, } @@ -71,15 +78,15 @@ impl NamespaceAccess { } } -#[derive(Debug, Clone)] +#[derive(Clone)] struct SchemeRegister { - target_namespace: Rc>, + target_namespace: Arc>, scheme_name: String, } impl SchemeRegister { fn register(&self, fd: FdGuard) -> Result<()> { - let mut ns = self.target_namespace.borrow_mut(); + let mut ns = self.target_namespace.lock(); if ns.schemes.contains_key(&self.scheme_name) { return Err(Error::new(EEXIST)); } @@ -88,7 +95,7 @@ impl SchemeRegister { } } -#[derive(Debug, Clone)] +#[derive(Clone)] enum Handle { Access(NamespaceAccess), Register(SchemeRegister), @@ -122,7 +129,7 @@ impl<'sock> NamespaceScheme<'sock> { fn add_namespace(&mut self, id: usize, schemes: Namespace, permission: NsPermissions) { let handle = Handle::Access(NamespaceAccess { - namespace: Rc::new(RefCell::new(schemes)), + namespace: Arc::new(Mutex::new(schemes)), permission, }); self.handles.insert(id, handle); @@ -183,9 +190,9 @@ impl<'sock> NamespaceScheme<'sock> { Ok(scheme_fd) } - fn fork_namespace(&mut self, namespace: Rc>, names: &[u8]) -> Result { + fn fork_namespace(&mut self, namespace: Arc>, names: &[u8]) -> Result { let new_id = self.next_id; - let new_namespace = namespace.borrow().fork(names).map_err(|e| { + let new_namespace = namespace.lock().fork(names).map_err(|e| { error!("Failed to fork namespace {}: {}", new_id, e); e })?; @@ -258,8 +265,13 @@ impl<'sock> SchemeSync for NamespaceScheme<'sock> { flags: NewFdFlags::empty(), }); } + // STEP 1 (mechanical): the lock is held across the blocking openat + // inside open_scheme_resource. That is safe only because this is + // still single-threaded. STEP 3 must resolve+clone the cap_fd under + // a short lock here and run the openat with the lock released, or + // the spinlock would burn a worker while a provider is slow. _ => self.open_scheme_resource( - &ns_access.namespace.borrow(), + &ns_access.namespace.lock(), scheme.as_ref(), reference.as_ref(), flags, @@ -329,7 +341,7 @@ impl<'sock> SchemeSync for NamespaceScheme<'sock> { error!("Namespace with ID {} not found", fd); Error::new(ENOENT) })?; - let mut ns = ns_access.namespace.borrow_mut(); + let mut ns = ns_access.namespace.lock(); let redox_path = RedoxPath::from_absolute(path).ok_or(Error::new(EINVAL))?; let (scheme, reference) = redox_path.as_parts().ok_or(Error::new(EINVAL))?; @@ -411,7 +423,7 @@ impl<'sock> SchemeSync for NamespaceScheme<'sock> { return Err(Error::new(EACCES)); } - let ns = ns_access.namespace.borrow(); + let ns = ns_access.namespace.lock(); let opaque_offset = opaque_offset as usize; for (i, (name, _)) in ns.schemes.iter().enumerate().skip(opaque_offset) {