Fix thread exiting but not being freed
This commit is contained in:
		
							parent
							
								
									23329efe14
								
							
						
					
					
						commit
						a7400ca6dc
					
				| @ -15,6 +15,7 @@ unsafe impl Send for ChildProcessFilter {} | |||||||
| pub fn do_exit(exit_status: i32) { | pub fn do_exit(exit_status: i32) { | ||||||
|     let current_ref = get_current(); |     let current_ref = get_current(); | ||||||
|     let mut current = current_ref.lock().unwrap(); |     let mut current = current_ref.lock().unwrap(); | ||||||
|  |     let parent_ref = current.get_parent().clone(); | ||||||
|     // Update current
 |     // Update current
 | ||||||
|     current.exit_status = exit_status; |     current.exit_status = exit_status; | ||||||
|     current.status = Status::ZOMBIE; |     current.status = Status::ZOMBIE; | ||||||
| @ -34,8 +35,15 @@ pub fn do_exit(exit_status: i32) { | |||||||
|         futex_wake(ctid as *const i32, 1); |         futex_wake(ctid as *const i32, 1); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |     // If the process is detached, no need to notify the parent
 | ||||||
|  |     if current.is_detached { | ||||||
|  |         let current_tid = current.get_tid(); | ||||||
|  |         drop(current); | ||||||
|  |         remove_zombie_child(&parent_ref, current_tid); | ||||||
|  |         return; | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|     // Notify the parent process if necessary
 |     // Notify the parent process if necessary
 | ||||||
|     let parent_ref = current.get_parent().clone(); |  | ||||||
|     let (mut parent, current) = { |     let (mut parent, current) = { | ||||||
|         // Always lock parent before its child
 |         // Always lock parent before its child
 | ||||||
|         drop(current); |         drop(current); | ||||||
| @ -103,34 +111,44 @@ pub fn do_wait4(child_filter: &ChildProcessFilter, exit_status: &mut i32) -> Res | |||||||
|         waiter |         waiter | ||||||
|     }; |     }; | ||||||
| 
 | 
 | ||||||
|  |     // Wait until a child has interesting events
 | ||||||
|     let child_pid = waiter.sleep_until_woken_with_result(); |     let child_pid = waiter.sleep_until_woken_with_result(); | ||||||
| 
 | 
 | ||||||
|     let mut current = current_ref.lock().unwrap(); |     // Remove the child from the parent
 | ||||||
|     let child_i = { |     *exit_status = remove_zombie_child(¤t_ref, child_pid); | ||||||
|         let mut child_i_opt = None; |  | ||||||
|         for (child_i, child_ref) in current.get_children_iter().enumerate() { |  | ||||||
|             let child = child_ref.lock().unwrap(); |  | ||||||
|             if child.get_pid() != child_pid { |  | ||||||
|                 continue; |  | ||||||
|             } |  | ||||||
| 
 | 
 | ||||||
|             if child.get_status() != Status::ZOMBIE { |     let mut current = current_ref.lock().unwrap(); | ||||||
|                 panic!("THIS SHOULD NEVER HAPPEN!"); |  | ||||||
|             } |  | ||||||
|             child_i_opt = Some(child_i); |  | ||||||
|             *exit_status = child.get_exit_status(); |  | ||||||
|         } |  | ||||||
|         child_i_opt.unwrap() |  | ||||||
|     }; |  | ||||||
|     current.children.swap_remove(child_i); |  | ||||||
|     current.waiting_children = None; |     current.waiting_children = None; | ||||||
| 
 | 
 | ||||||
|     // Release the last reference to the child process
 |  | ||||||
|     process_table::remove(child_pid); |  | ||||||
| 
 |  | ||||||
|     Ok(child_pid) |     Ok(child_pid) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | fn remove_zombie_child(parent_ref: &ProcessRef, child_tid: pid_t) -> i32 { | ||||||
|  |     // Find the zombie child process
 | ||||||
|  |     let mut parent = parent_ref.lock().unwrap(); | ||||||
|  |     let (child_i, child_ref) = parent | ||||||
|  |         .get_children_iter() | ||||||
|  |         .enumerate() | ||||||
|  |         .find(|(child_i, child_ref)| { | ||||||
|  |             let child = child_ref.lock().unwrap(); | ||||||
|  |             if child.get_tid() != child_tid { | ||||||
|  |                 return false; | ||||||
|  |             } | ||||||
|  |             assert!(child.get_status() == Status::ZOMBIE); | ||||||
|  |             true | ||||||
|  |         }) | ||||||
|  |         .expect("cannot find the zombie child"); | ||||||
|  | 
 | ||||||
|  |     // Remove the zombie child from parent
 | ||||||
|  |     parent.children.swap_remove(child_i); | ||||||
|  |     // Remove the zombie child from process table
 | ||||||
|  |     process_table::remove(child_tid); | ||||||
|  | 
 | ||||||
|  |     // Return the exit status
 | ||||||
|  |     let child = child_ref.lock().unwrap(); | ||||||
|  |     child.get_exit_status() | ||||||
|  | } | ||||||
|  | 
 | ||||||
| fn lock_two_in_order<'a>( | fn lock_two_in_order<'a>( | ||||||
|     first_ref: &'a ProcessRef, |     first_ref: &'a ProcessRef, | ||||||
|     second_ref: &'a ProcessRef, |     second_ref: &'a ProcessRef, | ||||||
|  | |||||||
| @ -23,6 +23,7 @@ pub struct Process { | |||||||
|     tgid: pid_t, |     tgid: pid_t, | ||||||
|     host_tid: pid_t, |     host_tid: pid_t, | ||||||
|     exit_status: i32, |     exit_status: i32, | ||||||
|  |     is_detached: bool, | ||||||
|     // TODO: move cwd, root_inode into a FileSystem structure
 |     // TODO: move cwd, root_inode into a FileSystem structure
 | ||||||
|     // TODO: should cwd be a String or INode?
 |     // TODO: should cwd be a String or INode?
 | ||||||
|     cwd: String, |     cwd: String, | ||||||
|  | |||||||
| @ -14,6 +14,7 @@ lazy_static! { | |||||||
|             tgid: 0, |             tgid: 0, | ||||||
|             host_tid: 0, |             host_tid: 0, | ||||||
|             exit_status: 0, |             exit_status: 0, | ||||||
|  |             is_detached: false, | ||||||
|             cwd: "/".to_owned(), |             cwd: "/".to_owned(), | ||||||
|             elf_path: "/".to_owned(), |             elf_path: "/".to_owned(), | ||||||
|             clear_child_tid: None, |             clear_child_tid: None, | ||||||
| @ -28,6 +29,7 @@ lazy_static! { | |||||||
| } | } | ||||||
| 
 | 
 | ||||||
| impl Process { | impl Process { | ||||||
|  |     // TODO: this constructor has become complicated enough to justify using builders
 | ||||||
|     pub fn new( |     pub fn new( | ||||||
|         cwd: &str, |         cwd: &str, | ||||||
|         elf_path: &str, |         elf_path: &str, | ||||||
| @ -35,6 +37,7 @@ impl Process { | |||||||
|         vm_ref: ProcessVMRef, |         vm_ref: ProcessVMRef, | ||||||
|         file_table_ref: FileTableRef, |         file_table_ref: FileTableRef, | ||||||
|         rlimits_ref: ResourceLimitsRef, |         rlimits_ref: ResourceLimitsRef, | ||||||
|  |         is_detached: bool, | ||||||
|     ) -> Result<(pid_t, ProcessRef)> { |     ) -> Result<(pid_t, ProcessRef)> { | ||||||
|         let new_pid = process_table::alloc_pid(); |         let new_pid = process_table::alloc_pid(); | ||||||
|         let new_process_ref = Arc::new(SgxMutex::new(Process { |         let new_process_ref = Arc::new(SgxMutex::new(Process { | ||||||
| @ -48,6 +51,7 @@ impl Process { | |||||||
|             elf_path: elf_path.to_owned(), |             elf_path: elf_path.to_owned(), | ||||||
|             clear_child_tid: None, |             clear_child_tid: None, | ||||||
|             exit_status: 0, |             exit_status: 0, | ||||||
|  |             is_detached: is_detached, | ||||||
|             parent: None, |             parent: None, | ||||||
|             children: Vec::new(), |             children: Vec::new(), | ||||||
|             waiting_children: None, |             waiting_children: None, | ||||||
|  | |||||||
| @ -111,7 +111,7 @@ fn new_process( | |||||||
|             Arc::new(SgxMutex::new(files)) |             Arc::new(SgxMutex::new(files)) | ||||||
|         }; |         }; | ||||||
|         let rlimits_ref = Default::default(); |         let rlimits_ref = Default::default(); | ||||||
|         Process::new(&cwd, elf_path, task, vm_ref, files_ref, rlimits_ref)? |         Process::new(&cwd, elf_path, task, vm_ref, files_ref, rlimits_ref, false)? | ||||||
|     }; |     }; | ||||||
|     parent_adopts_new_child(&parent_ref, &new_process_ref); |     parent_adopts_new_child(&parent_ref, &new_process_ref); | ||||||
|     process_table::put(new_pid, new_process_ref.clone()); |     process_table::put(new_pid, new_process_ref.clone()); | ||||||
|  | |||||||
| @ -79,7 +79,7 @@ pub fn do_clone( | |||||||
|         let rlimits_ref = current.get_rlimits().clone(); |         let rlimits_ref = current.get_rlimits().clone(); | ||||||
|         let elf_path = ¤t.elf_path; |         let elf_path = ¤t.elf_path; | ||||||
|         let cwd = ¤t.cwd; |         let cwd = ¤t.cwd; | ||||||
|         Process::new(cwd, elf_path, task, vm_ref, files_ref, rlimits_ref)? |         Process::new(cwd, elf_path, task, vm_ref, files_ref, rlimits_ref, true)? | ||||||
|     }; |     }; | ||||||
| 
 | 
 | ||||||
|     if let Some(ctid) = ctid { |     if let Some(ctid) = ctid { | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user