From ba8d08d7337edd4a56e20f89b72f5ea6b8b8d364 Mon Sep 17 00:00:00 2001 From: WangRunji Date: Thu, 9 May 2019 23:56:27 +0800 Subject: [PATCH] fix Process dropping by making `proc.parent` a weak reference --- kernel/src/process/structs.rs | 23 +++++++-- kernel/src/syscall/fs.rs | 2 +- kernel/src/syscall/proc.rs | 87 +++++++++++------------------------ 3 files changed, 46 insertions(+), 66 deletions(-) diff --git a/kernel/src/process/structs.rs b/kernel/src/process/structs.rs index 3f7c272..f19e145 100644 --- a/kernel/src/process/structs.rs +++ b/kernel/src/process/structs.rs @@ -22,6 +22,7 @@ use crate::sync::{Condvar, SpinNoIrqLock as Mutex}; use super::abi::{self, ProcInitInfo}; use core::mem::uninitialized; use rcore_fs::vfs::INode; +use crate::processor; pub struct Thread { context: Context, @@ -66,7 +67,7 @@ pub struct Process { // relationship pub pid: Pid, // i.e. tgid, usually the tid of first thread - pub parent: Option>>, + pub parent: Weak>, pub children: Vec>>, pub threads: Vec, // threads in the same process @@ -125,7 +126,7 @@ impl Thread { exec_path: String::new(), futexes: BTreeMap::default(), pid: Pid(0), - parent: None, + parent: Weak::new(), children: Vec::new(), threads: Vec::new(), child_exit: Arc::new(Condvar::new()), @@ -303,7 +304,7 @@ impl Thread { exec_path: String::from(exec_path), futexes: BTreeMap::default(), pid: Pid(0), - parent: None, + parent: Weak::new(), children: Vec::new(), threads: Vec::new(), child_exit: Arc::new(Condvar::new()), @@ -329,7 +330,7 @@ impl Thread { exec_path: proc.exec_path.clone(), futexes: BTreeMap::default(), pid: Pid(0), - parent: Some(self.proc.clone()), + parent: Arc::downgrade(&self.proc), children: Vec::new(), threads: Vec::new(), child_exit: Arc::new(Condvar::new()), @@ -403,6 +404,20 @@ impl Process { } self.futexes.get(&uaddr).unwrap().clone() } + /// Exit the process. + /// Kill all threads and notify parent with the exit code. + pub fn exit(&mut self, exit_code: usize) { + // quit all threads + for tid in self.threads.iter() { + processor().manager().exit(*tid, 1); + } + // notify parent and fill exit code + if let Some(parent) = self.parent.upgrade() { + let mut parent = parent.lock(); + parent.child_exit_code.insert(self.pid.get(), exit_code); + parent.child_exit.notify_one(); + } + } } trait ToMemoryAttr { diff --git a/kernel/src/syscall/fs.rs b/kernel/src/syscall/fs.rs index 401a950..5cbe1e2 100644 --- a/kernel/src/syscall/fs.rs +++ b/kernel/src/syscall/fs.rs @@ -524,7 +524,7 @@ impl Syscall<'_> { arg3: usize, ) -> SysResult { info!( - "ioctl: fd: {}, request: {:x}, args: {} {} {}", + "ioctl: fd: {}, request: {:#x}, args: {:#x} {:#x} {:#x}", fd, request, arg1, arg2, arg3 ); let mut proc = self.process(); diff --git a/kernel/src/syscall/proc.rs b/kernel/src/syscall/proc.rs index e5269db..89cb0f4 100644 --- a/kernel/src/syscall/proc.rs +++ b/kernel/src/syscall/proc.rs @@ -54,7 +54,6 @@ impl Syscall<'_> { let new_thread = self .thread .clone(self.tf, newsp, newtls, child_tid as usize); - // FIXME: parent pid let tid = processor().manager().add(new_thread); processor().manager().detach(tid); info!("clone: {} -> {}", thread::current().id(), tid); @@ -66,7 +65,7 @@ impl Syscall<'_> { /// Wait for the process exit. /// Return the PID. Store exit code to `wstatus` if it's not null. pub fn sys_wait4(&mut self, pid: isize, wstatus: *mut i32) -> SysResult { - //info!("wait4: pid: {}, code: {:?}", pid, wstatus); + info!("wait4: pid: {}, code: {:?}", pid, wstatus); let wstatus = if !wstatus.is_null() { Some(unsafe { self.vm().check_write_ptr(wstatus)? }) } else { @@ -75,10 +74,12 @@ impl Syscall<'_> { #[derive(Debug)] enum WaitFor { AnyChild, + AnyChildInGroup, Pid(usize), } let target = match pid { - -1 | 0 => WaitFor::AnyChild, + -1 => WaitFor::AnyChild, + 0 => WaitFor::AnyChildInGroup, p if p > 0 => WaitFor::Pid(p as usize), _ => unimplemented!(), }; @@ -86,7 +87,7 @@ impl Syscall<'_> { let mut proc = self.process(); // check child_exit_code let find = match target { - WaitFor::AnyChild => proc + WaitFor::AnyChild | WaitFor::AnyChildInGroup => proc .child_exit_code .iter() .next() @@ -102,17 +103,19 @@ impl Syscall<'_> { return Ok(pid); } // if not, check pid - let children: Vec<_> = proc - .children - .iter() - .filter_map(|weak| weak.upgrade()) - .collect(); - let invalid = match target { - WaitFor::AnyChild => children.len() == 0, - WaitFor::Pid(pid) => children + let invalid = { + let children: Vec<_> = proc + .children .iter() - .find(|p| p.lock().pid.get() == pid) - .is_none(), + .filter_map(|weak| weak.upgrade()) + .collect(); + match target { + WaitFor::AnyChild | WaitFor::AnyChildInGroup => children.len() == 0, + WaitFor::Pid(pid) => children + .iter() + .find(|p| p.lock().pid.get() == pid) + .is_none(), + } }; if invalid { return Err(SysError::ECHILD); @@ -204,7 +207,7 @@ impl Syscall<'_> { /// Kill the process pub fn sys_kill(&mut self, pid: usize, sig: usize) -> SysResult { info!( - "kill: {} killed: {} with sig {}", + "kill: thread {} kill process {} with signal {}", thread::current().id(), pid, sig @@ -215,21 +218,8 @@ impl Syscall<'_> { self.sys_exit_group(sig); } else { if let Some(proc_arc) = PROCESSES.read().get(&pid).and_then(|weak| weak.upgrade()) { - let proc = proc_arc.lock(); - // quit all threads - for tid in proc.threads.iter() { - processor().manager().exit(*tid, sig); - } - // notify parent and fill exit code - // avoid deadlock - let proc_parent = proc.parent.clone(); - let pid = proc.pid.get(); - drop(proc); - if let Some(parent) = proc_parent { - let mut parent = parent.lock(); - parent.child_exit_code.insert(pid, sig); - parent.child_exit.notify_one(); - } + let mut proc = proc_arc.lock(); + proc.exit(sig); Ok(0) } else { Err(SysError::EINVAL) @@ -252,7 +242,7 @@ impl Syscall<'_> { /// Get the parent process id pub fn sys_getppid(&mut self) -> SysResult { - if let Some(parent) = self.process().parent.as_ref() { + if let Some(parent) = self.process().parent.upgrade() { Ok(parent.lock().pid.get()) } else { Ok(0) @@ -266,26 +256,15 @@ impl Syscall<'_> { let mut proc = self.process(); proc.threads.retain(|&id| id != tid); - // for last thread, - // notify parent and fill exit code - // avoid deadlock - let exit = proc.threads.len() == 0; - let proc_parent = proc.parent.clone(); - let pid = proc.pid.get(); - drop(proc); - if exit { - if let Some(parent) = proc_parent { - let mut parent = parent.lock(); - parent.child_exit_code.insert(pid, exit_code); - parent.child_exit.notify_one(); - } + // for last thread, exit the process + if proc.threads.len() == 0 { + proc.exit(exit_code); } // perform futex wake 1 // ref: http://man7.org/linux/man-pages/man2/set_tid_address.2.html // FIXME: do it in all possible ways a thread can exit // it has memory access so we can't move it to Thread::drop? - let mut proc = self.process(); let clear_child_tid = self.thread.clear_child_tid as *mut u32; if !clear_child_tid.is_null() { info!("exit: futex {:#?} wake 1", clear_child_tid); @@ -304,24 +283,10 @@ impl Syscall<'_> { /// Exit the current thread group (i.e. process) pub fn sys_exit_group(&mut self, exit_code: usize) -> ! { - let proc = self.process(); + let mut proc = self.process(); info!("exit_group: {}, code: {}", proc.pid, exit_code); - // quit all threads - for tid in proc.threads.iter() { - processor().manager().exit(*tid, exit_code); - } - - // notify parent and fill exit code - // avoid deadlock - let proc_parent = proc.parent.clone(); - let pid = proc.pid.get(); - drop(proc); - if let Some(parent) = proc_parent { - let mut parent = parent.lock(); - parent.child_exit_code.insert(pid, exit_code); - parent.child_exit.notify_one(); - } + proc.exit(exit_code); processor().yield_now(); unreachable!();