Fix two bugs on process exit
The first bug is a race condition when acquiring the lock of a process's parent. An example code with race condition looks like below: ```rust let process : ProessRef = current!().process(); let parent : ProcessRef = process.parent(); let parent_guard : SgxMutexGuard<ProesssInner> = parent.inner(); // This assertion may fail because the process's parent may change to another // process before the lock is acquired assert!(parent.pid() == process.parent().pid()); ``` The second bug is that when a process exits, its children processes are not transfered to the idle process correctly.
This commit is contained in:
parent
686ec343b2
commit
35787be29d
@ -61,29 +61,49 @@ fn exit_thread(term_status: TermStatus) {
|
|||||||
fn exit_process(thread: &ThreadRef, term_status: TermStatus) {
|
fn exit_process(thread: &ThreadRef, term_status: TermStatus) {
|
||||||
let process = thread.process();
|
let process = thread.process();
|
||||||
|
|
||||||
// If the parent process is the idle process, we can release the process directly.
|
// Deadlock note: always lock parent first, then child.
|
||||||
if process.parent().pid() == 0 {
|
|
||||||
// Deadlock note: Always lock parent then child.
|
// Lock the idle process since it may adopt new children.
|
||||||
let mut parent_inner = super::IDLE.process().inner();
|
let idle_ref = super::IDLE.process().clone();
|
||||||
let mut process_inner = process.inner();
|
let mut idle_inner = idle_ref.inner();
|
||||||
|
// Lock the parent process as we want to prevent race conditions between
|
||||||
|
// current's exit() and parent's wait5().
|
||||||
|
let mut parent;
|
||||||
|
let mut parent_inner = loop {
|
||||||
|
parent = process.parent();
|
||||||
|
if parent.pid() == 0 {
|
||||||
|
// If the parent is the idle process, don't need to lock again
|
||||||
|
break None;
|
||||||
|
}
|
||||||
|
|
||||||
|
let parent_inner = parent.inner();
|
||||||
|
// To prevent the race condition that parent is changed after `parent()`,
|
||||||
|
// but before `parent().innner()`, we need to check again here.
|
||||||
|
if parent.pid() != process.parent().pid() {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
break Some(parent_inner);
|
||||||
|
};
|
||||||
|
// Lock the current process
|
||||||
|
let mut process_inner = process.inner();
|
||||||
|
|
||||||
|
// The parent is the idle process
|
||||||
|
if parent_inner.is_none() {
|
||||||
|
debug_assert!(parent.pid() == 0);
|
||||||
|
|
||||||
let pid = process.pid();
|
let pid = process.pid();
|
||||||
let main_tid = pid;
|
let main_tid = pid;
|
||||||
table::del_thread(main_tid).expect("tid must be in the table");
|
table::del_thread(main_tid).expect("tid must be in the table");
|
||||||
table::del_process(pid).expect("pid must be in the table");
|
table::del_process(pid).expect("pid must be in the table");
|
||||||
|
|
||||||
process_inner.exit(term_status);
|
process_inner.exit(term_status, &idle_ref, &mut idle_inner);
|
||||||
parent_inner.remove_zombie_child(process.pid());
|
idle_inner.remove_zombie_child(pid);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// Otherwise, we need to notify the parent process
|
// Otherwise, we need to notify the parent process
|
||||||
|
let mut parent_inner = parent_inner.unwrap();
|
||||||
|
|
||||||
// Lock the parent process to ensure that parent's wait4 cannot miss the current
|
process_inner.exit(term_status, &idle_ref, &mut idle_inner);
|
||||||
// process's exit.
|
|
||||||
// Deadlock note: Always lock parent then child.
|
|
||||||
let parent = process.parent();
|
|
||||||
let mut parent_inner = parent.inner();
|
|
||||||
process.inner().exit(term_status);
|
|
||||||
|
|
||||||
//Send SIGCHLD to parent
|
//Send SIGCHLD to parent
|
||||||
send_sigchld_to(&parent);
|
send_sigchld_to(&parent);
|
||||||
|
@ -233,15 +233,27 @@ impl ProcessInner {
|
|||||||
children.swap_remove(zombie_i)
|
children.swap_remove(zombie_i)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn exit(&mut self, term_status: TermStatus) {
|
/// Exit means two things: 1) transfer all children to a new parent; 2) update the status.
|
||||||
|
///
|
||||||
|
/// A lock guard for the new parent process is passed so that the transfer can be done
|
||||||
|
/// atomically.
|
||||||
|
pub fn exit(
|
||||||
|
&mut self,
|
||||||
|
term_status: TermStatus,
|
||||||
|
new_parent_ref: &ProcessRef,
|
||||||
|
new_parent_inner: &mut SgxMutexGuard<ProcessInner>,
|
||||||
|
) {
|
||||||
// Check preconditions
|
// Check preconditions
|
||||||
debug_assert!(self.status() == ProcessStatus::Running);
|
debug_assert!(self.status() == ProcessStatus::Running);
|
||||||
debug_assert!(self.num_threads() == 0);
|
debug_assert!(self.num_threads() == 0);
|
||||||
|
|
||||||
// When this process exits, its children are adopted by the init process
|
// When this process exits, its children are adopted by the init process
|
||||||
for child in self.children().unwrap() {
|
for child in self.children().unwrap() {
|
||||||
|
let child_inner = child.inner();
|
||||||
let mut parent = child.parent.as_ref().unwrap().write().unwrap();
|
let mut parent = child.parent.as_ref().unwrap().write().unwrap();
|
||||||
*parent = IDLE.process().clone();
|
*parent = new_parent_ref.clone();
|
||||||
|
|
||||||
|
new_parent_inner.children_mut().unwrap().push(child.clone());
|
||||||
}
|
}
|
||||||
|
|
||||||
*self = Self::Zombie { term_status };
|
*self = Self::Zombie { term_status };
|
||||||
|
Loading…
Reference in New Issue
Block a user