From 129d4e3ae5236b90fa8eadd91268d0b40e847f4c Mon Sep 17 00:00:00 2001 From: Noam Kleinburd Date: Mon, 1 Apr 2019 17:00:10 +0300 Subject: [PATCH] Lock resources before checking if we need to clean them. See the comments deleted by this commit for more details as to how the race condition could effect the system. --- src/syscall/process.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/syscall/process.rs b/src/syscall/process.rs index c5b0038..a88fdab 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -525,11 +525,8 @@ fn empty(context: &mut context::Context, reaping: bool) { drop(context.tls.take()); } - // FIXME: Looks like a race condition. - // Is it possible for Arc::strong_count to return 1 to two contexts that exit at the - // same time, or return 2 to both, thus either double freeing or leaking the grants? + let mut grants = context.grants.lock(); if Arc::strong_count(&context.grants) == 1 { - let mut grants = context.grants.lock(); for grant in grants.drain(..) { if reaping { println!("{}: {}: Grant should not exist: {:?}", context.id.into(), unsafe { ::core::str::from_utf8_unchecked(&context.name.lock()) }, grant); @@ -927,11 +924,11 @@ pub fn exit(status: usize) -> ! { let mut close_files = Vec::new(); let pid = { let mut context = context_lock.write(); - // FIXME: Looks like a race condition. - // Is it possible for Arc::strong_count to return 1 to two contexts that exit at the - // same time, or return 2 to both, thus either double closing or leaking the files? - if Arc::strong_count(&context.files) == 1 { - mem::swap(context.files.lock().deref_mut(), &mut close_files); + { + let mut lock = context.files.lock(); + if Arc::strong_count(&context.files) == 1 { + mem::swap(lock.deref_mut(), &mut close_files); + } } context.files = Arc::new(Mutex::new(Vec::new())); context.id