From a3dedb0639650d910076dbdcb2a8f2760e052ee6 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Mon, 15 Jul 2024 14:33:00 +0200 Subject: [PATCH] Do proper resource cleanup for thread exit too. --- src/context/context.rs | 7 +++- src/scheme/proc.rs | 8 ++-- src/syscall/mod.rs | 24 ++++-------- src/syscall/process.rs | 84 ++++++++++++++++++++++++------------------ 4 files changed, 66 insertions(+), 57 deletions(-) diff --git a/src/context/context.rs b/src/context/context.rs index 477f021028..e8c1cca341 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -16,7 +16,8 @@ use crate::{ memory::{allocate_p2frame, deallocate_p2frame, Enomem, Frame, RaiiFrame}, paging::{RmmA, RmmArch}, percpu::PercpuBlock, - scheme::FileHandle, sync::WaitCondition, + scheme::FileHandle, + sync::WaitCondition, }; use crate::syscall::error::{Error, Result, EAGAIN, ESRCH}; @@ -43,7 +44,9 @@ pub enum Status { HardBlocked { reason: HardBlockedReason, }, - Exited { user_data: usize }, + Exited { + user_data: usize, + }, } impl Status { diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index 8e903adf57..ece17a30c5 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -1415,11 +1415,13 @@ impl ContextHandle { } Self::Status => { let user_data = buf.read_usize()?; - let mut context = context.write(); // TODO: Handle Status::HardBlocked differently? - context.status = Status::Exited { user_data }; - context.status_cond.notify(); + crate::syscall::exit_context(&context, user_data); + + if context::is_current(&context) { + context::switch(); + } Ok(mem::size_of::()) } diff --git a/src/syscall/mod.rs b/src/syscall/mod.rs index ad7a5877d2..f14e87e6f3 100644 --- a/src/syscall/mod.rs +++ b/src/syscall/mod.rs @@ -136,13 +136,10 @@ pub fn syscall( }), SYS_DUP => dup(fd, UserSlice::ro(c, d)?).map(FileHandle::into), - SYS_DUP2 => dup2(fd, FileHandle::from(c), UserSlice::ro(d, e)?) - .map(FileHandle::into), + SYS_DUP2 => dup2(fd, FileHandle::from(c), UserSlice::ro(d, e)?).map(FileHandle::into), #[cfg(target_pointer_width = "32")] - SYS_SENDFD => { - sendfd(fd, FileHandle::from(c), d, e as u64 | ((f as u64) << 32)) - } + SYS_SENDFD => sendfd(fd, FileHandle::from(c), d, e as u64 | ((f as u64) << 32)), #[cfg(target_pointer_width = "64")] SYS_SENDFD => sendfd(fd, FileHandle::from(c), d, e as u64), @@ -163,13 +160,11 @@ pub fn syscall( SYS_FRENAME => frename(fd, UserSlice::ro(c, d)?).map(|()| 0), SYS_FUNMAP => funmap(b, c), - SYS_FSYNC => { - file_op_generic(fd, |scheme, number| scheme.fsync(number).map(|()| 0)) - } + SYS_FSYNC => file_op_generic(fd, |scheme, number| scheme.fsync(number).map(|()| 0)), // TODO: 64-bit lengths on 32-bit platforms - SYS_FTRUNCATE => file_op_generic(fd, |scheme, number| { - scheme.ftruncate(number, c).map(|()| 0) - }), + SYS_FTRUNCATE => { + file_op_generic(fd, |scheme, number| scheme.ftruncate(number, c).map(|()| 0)) + } SYS_CLOSE => close(fd).map(|()| 0), @@ -183,8 +178,7 @@ pub fn syscall( ) .map(|()| 0), SYS_CLOCK_GETTIME => { - clock_gettime(b, UserSlice::wo(c, core::mem::size_of::())?) - .map(|()| 0) + clock_gettime(b, UserSlice::wo(c, core::mem::size_of::())?).map(|()| 0) } SYS_FUTEX => futex(b, c, d, e, f), SYS_GETPID => getpid().map(ProcessId::into), @@ -218,9 +212,7 @@ pub fn syscall( )?), SYS_SETPGID => setpgid(ProcessId::from(b), ProcessId::from(c)).map(|()| 0), SYS_SETREUID => setreuid(b as u32, c as u32).map(|()| 0), - SYS_SETRENS => { - setrens(SchemeNamespace::from(b), SchemeNamespace::from(c)).map(|()| 0) - } + SYS_SETRENS => setrens(SchemeNamespace::from(b), SchemeNamespace::from(c)).map(|()| 0), SYS_SETREGID => setregid(b as u32, c as u32).map(|()| 0), SYS_UMASK => umask(b), SYS_VIRTTOPHYS => virttophys(b), diff --git a/src/syscall/process.rs b/src/syscall/process.rs index b8b2d7d6c5..ff2b404040 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -1,13 +1,15 @@ use alloc::{sync::Arc, vec::Vec}; -use spinning_top::RwSpinlock; use core::{mem, num::NonZeroUsize, sync::atomic::Ordering}; +use spinning_top::RwSpinlock; use syscall::{sig_bit, SIGCHLD, SIGKILL, SIGSTOP, SIGTERM, SIGTSTP, SIGTTIN, SIGTTOU}; use rmm::Arch; use spin::RwLock; use crate::context::{ - memory::{AddrSpace, Grant, PageSpan}, process::{self, Process, ProcessId, ProcessInfo, ProcessStatus}, Context, WaitpidKey + memory::{AddrSpace, Grant, PageSpan}, + process::{self, Process, ProcessId, ProcessInfo, ProcessStatus}, + Context, WaitpidKey, }; use crate::{ @@ -27,41 +29,49 @@ use crate::{ use super::usercopy::UserSliceWo; +pub fn exit_context(context_lock: &Arc>, user_data: usize) { + // TODO: Stop context? + let close_files; + let addrspace_opt; + + { + let mut context = context_lock.write(); + close_files = Arc::try_unwrap(mem::take(&mut context.files)) + .map_or_else(|_| Vec::new(), RwLock::into_inner); + addrspace_opt = context + .set_addr_space(None) + .and_then(|a| Arc::try_unwrap(a).ok()); + drop(context.syscall_head.take()); + drop(context.syscall_tail.take()); + } + // Files must be closed while context is valid so that messages can be passed + for (_fd, file_opt) in close_files.into_iter().enumerate() { + if let Some(file) = file_opt { + let _ = file.close(); + } + } + drop(addrspace_opt); + let mut guard = context_lock.write(); + guard.status = context::Status::Exited { user_data }; + guard.status_cond.notify(); +} + pub fn exit(status: usize) -> ! { ptrace::breakpoint_callback( PTRACE_STOP_EXIT, Some(ptrace_event!(PTRACE_STOP_EXIT, status)), ); + let current_context = context::current(); + let current_process = process::current().expect("no active process during exit syscall"); + let current_pid = current_process.read().pid; + + exit_context(¤t_context, 0); + { - let context_lock = context::current(); - - let close_files; - let addrspace_opt; - - let (pid, process_lock) = { - let mut context = context_lock.write(); - close_files = Arc::try_unwrap(mem::take(&mut context.files)) - .map_or_else(|_| Vec::new(), RwLock::into_inner); - addrspace_opt = context - .set_addr_space(None) - .and_then(|a| Arc::try_unwrap(a).ok()); - drop(context.syscall_head.take()); - drop(context.syscall_tail.take()); - (context.pid, Arc::clone(&context.process)) - }; - - // Files must be closed while context is valid so that messages can be passed - for (_fd, file_opt) in close_files.into_iter().enumerate() { - if let Some(file) = file_opt { - let _ = file.close(); - } - } - drop(addrspace_opt); - // PGID and PPID must be grabbed after close, as context switches could change PGID or PPID if parent exits let (pgid, ppid) = { - let process = process_lock.read(); + let process = current_process.read(); (process.pgid, process.ppid) }; let _ = kill(ppid, SIGCHLD, true); @@ -71,16 +81,15 @@ pub fn exit(status: usize) -> ! { let processes = context::process::PROCESSES.read(); for (_child_pid, child_process_lock) in processes.iter() { let mut process = child_process_lock.write(); - if process.ppid == pid { + if process.ppid == current_pid { process.ppid = ppid; } } } - process_lock.write().status = ProcessStatus::Exited(status); - context_lock.write().status = context::Status::Exited { user_data: status }; + current_process.write().status = ProcessStatus::Exited(status); - let children = process_lock.write().waitpid.receive_all(); + let children = current_process.write().waitpid.receive_all(); { let processes = process::PROCESSES.read(); @@ -93,16 +102,16 @@ pub fn exit(status: usize) -> ! { waitpid.send( WaitpidKey { - pid: Some(pid), + pid: Some(current_pid), pgid: Some(pgid), }, - (pid, status), + (current_pid, status), ); } } // Alert any tracers waiting of this process - ptrace::close_tracee(pid); + ptrace::close_tracee(current_pid); } let _ = context::switch(); @@ -174,7 +183,10 @@ pub fn kill(pid: ProcessId, sig: usize, parent_sigchld: bool) -> Result { }, } - let mut send = |context_lock: &Arc>, process_lock: &Arc>, proc_info: &ProcessInfo| -> SendResult { + let mut send = |context_lock: &Arc>, + process_lock: &Arc>, + proc_info: &ProcessInfo| + -> SendResult { let is_self = context::is_current(context_lock); // Non-root users cannot kill arbitrarily.