From 28cd0a9f6ec2306f24825f0b928b89e3fa55353f Mon Sep 17 00:00:00 2001 From: Quentin Legot Date: Thu, 4 May 2023 22:58:13 +0200 Subject: [PATCH 1/2] Try to fix double free --- src/kernel/thread_manager.rs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/kernel/thread_manager.rs b/src/kernel/thread_manager.rs index c6cd396..c45dbff 100644 --- a/src/kernel/thread_manager.rs +++ b/src/kernel/thread_manager.rs @@ -355,29 +355,30 @@ impl ThreadManager { /// Wake up a waiter if necessary, or release it if no thread is waiting. pub fn lock_release(&mut self, id: i32, machine: &mut Machine) -> Result { + let old_status = machine.interrupt.set_status(InterruptStatus::InterruptOff); let current_thread = match self.get_g_current_thread() { Some(thread) => Rc::clone(thread), None => Err(String::from("lock_release error: current_thread should not be None."))? }; - let mut lock = match self.get_obj_addrs().search_lock(id).cloned() { + let mut lock = match self.get_obj_addrs().search_lock(id) { Some(lock) => lock, None => Err(String::from("lock_release error: cannot find lock."))? }; - let old_status = machine.interrupt.set_status(InterruptStatus::InterruptOff); if let Some(lock_owner) = &lock.owner { - if Rc::ptr_eq(¤t_thread, lock_owner) { - if let Some(thread) = lock.waiting_queue.pop() { - if !lock.waiting_queue.is_empty() { - let clone = Rc::clone(&thread); - lock.owner = Some(thread); - self.ready_to_run(clone); - } else { - lock.free = true; - lock.owner = None; - }} + if current_thread.eq(lock_owner) { // is_held_by_current_thread + match lock.waiting_queue.pop() { + Some(th) => { + lock.owner = Some(Rc::clone(&th)); + self.ready_to_run(Rc::clone(&th)); + }, + None => { + lock.free = true; + lock.owner = None; + } + } } }; - self.get_obj_addrs().update_lock(id, lock); + // self.get_obj_addrs().update_lock(id, lock); machine.interrupt.set_status(old_status); Ok(MachineOk::Ok) } From bec0143a40db2605164716021229bce8e7041deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Autin?= Date: Thu, 4 May 2023 23:44:10 +0200 Subject: [PATCH 2/2] Added missing current_thread assignment in test_lock_multiple --- src/kernel/thread_manager.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/kernel/thread_manager.rs b/src/kernel/thread_manager.rs index c45dbff..ec2db61 100644 --- a/src/kernel/thread_manager.rs +++ b/src/kernel/thread_manager.rs @@ -379,6 +379,7 @@ impl ThreadManager { } }; // self.get_obj_addrs().update_lock(id, lock); + machine.interrupt.set_status(old_status); Ok(MachineOk::Ok) } @@ -483,35 +484,36 @@ mod test { let lock_id = thread_manager.get_obj_addrs().add_lock(lock); let thread_1 = Rc::new(RefCell::new(Thread::new("test_lock_1"))); let thread_2 = Rc::new(RefCell::new(Thread::new("test_lock_2"))); - let thread_test_1 = thread_1.clone(); - let thread_test_2 = thread_2.clone(); - thread_manager.ready_to_run(Rc::clone(&thread_1)); - thread_manager.ready_to_run(Rc::clone(&thread_2)); - thread_manager.set_g_current_thread(Some(thread_1)); + thread_manager.ready_to_run(thread_1.clone()); + thread_manager.ready_to_run(thread_2.clone()); + thread_manager.set_g_current_thread(Some(thread_1.clone())); thread_manager.lock_acquire(lock_id, &mut machine).expect("lock acquire return an error at first iteration: "); { let lock = thread_manager.get_obj_addrs().search_lock(lock_id).unwrap(); - assert_eq!(lock.owner,Some(thread_test_1.clone())); + assert_eq!(lock.owner,Some(thread_1.clone())); assert!(!lock.free); assert!(lock.waiting_queue.is_empty()); } - thread_manager.set_g_current_thread(Some(thread_2)); + thread_manager.set_g_current_thread(Some(thread_2.clone())); thread_manager.lock_acquire(lock_id, &mut machine).expect("lock acquire return an error at second iteration: "); { let lock = thread_manager.get_obj_addrs().search_lock(lock_id).unwrap(); - assert_eq!(lock.owner,Some(thread_test_1)); + assert_eq!(lock.owner,Some(thread_1.clone())); assert!(!lock.free); assert_eq!(lock.waiting_queue.iter().count(),1); } + thread_manager.lock_release(lock_id, &mut machine).expect("lock release return an error at first iteration: "); { let lock = thread_manager.get_obj_addrs().search_lock(lock_id).unwrap(); - assert_eq!(lock.owner, Some(thread_test_2)); + assert_eq!(lock.owner, Some(thread_2.clone())); assert!(!lock.free); assert!(lock.waiting_queue.is_empty()); } + + thread_manager.set_g_current_thread(Some(thread_2.clone())); thread_manager.lock_release(lock_id, &mut machine).expect("lock release return an error at second iteration: "); { let lock = thread_manager.get_obj_addrs().search_lock(lock_id).unwrap();