From 2d8705917cc7fbfbf007b3c78d2f069b75930c66 Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Wed, 31 Jan 2024 15:38:13 +0100 Subject: [PATCH] continuing on thread safety --- mission_rust/src/fsrc/osal/mod.rs | 35 +++++++++++++++++++---------- mission_rust/src/fsrc/queues/mod.rs | 19 ++++++++++++---- mission_rust/src/fsrc/tasks/mod.rs | 31 +++++++++++++++++++------ mission_rust/src/lib.rs | 2 +- 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/mission_rust/src/fsrc/osal/mod.rs b/mission_rust/src/fsrc/osal/mod.rs index 7c62870..05191c4 100644 --- a/mission_rust/src/fsrc/osal/mod.rs +++ b/mission_rust/src/fsrc/osal/mod.rs @@ -3,6 +3,17 @@ type TaskFunction = unsafe extern "C" fn(*mut core::ffi::c_void); //TODO verify uXX == uintXX_t //TODO safe API + +// This is a macro so that the panic is at the right place +#[macro_export] +macro_rules! check_global_threading_available { + () => ( + if !crate::osal::global_threading_available() { + panic!("using threaded API outside of threading environment") + } + ); +} + extern "C" { pub fn outbyte(c: u8); //void *create_task(TaskFunction_t taskFunction, void *parameter, size_t stack_size) @@ -37,20 +48,20 @@ extern "C" { } -pub fn global_threading_available() -> bool { - unsafe{global_threading_available_c() == 1} -} - -pub fn check_global_threading_available() { - if !global_threading_available() { - panic!("using queue outside of threading environment") +pub fn task_delete_self() { + unsafe { + delete_task(0 as *const core::ffi::c_void); } } -pub fn enable_global_threading(){ - unsafe{enable_global_threading_c()}; +pub fn global_threading_available() -> bool { + unsafe { global_threading_available_c() == 1 } } -pub fn disable_global_threading(){ - unsafe{disable_global_threading_c()}; -} \ No newline at end of file +pub fn enable_global_threading() { + unsafe { enable_global_threading_c() }; +} + +pub fn disable_global_threading() { + unsafe { disable_global_threading_c() }; +} diff --git a/mission_rust/src/fsrc/queues/mod.rs b/mission_rust/src/fsrc/queues/mod.rs index 457843e..4f13dc3 100644 --- a/mission_rust/src/fsrc/queues/mod.rs +++ b/mission_rust/src/fsrc/queues/mod.rs @@ -1,4 +1,4 @@ -use crate::osal; +use crate::{check_global_threading_available, osal}; pub struct MessageQueue { queue_data: [Message; DEPTH], @@ -18,7 +18,7 @@ impl MessageQueue { } pub fn receive(&self) -> Option { - osal::check_global_threading_available(); + check_global_threading_available!(); let actual_id = match self.queue_id { None => return None, Some(id) => id, @@ -26,7 +26,11 @@ impl MessageQueue { let mut message: Message = Message::default(); let res: u8; unsafe { - //message = core::mem::MaybeUninit::zeroed().assume_init(); // We only return it if the queue received something + // safe beacuse: + // OS will write not more than length of message queue elements + // queue was created with size_of:: as length of message queue elements + // in MessageQueue::initialize + // making this pointer access safe as long as OS is holding the contract let message_pointer: *mut core::ffi::c_void = &mut message as *mut _ as *mut core::ffi::c_void; res = osal::queue_receive(actual_id, message_pointer); @@ -39,6 +43,7 @@ impl MessageQueue { } fn initialize(&mut self) { + check_global_threading_available!(); if self.queue_id != None { return; } @@ -53,6 +58,7 @@ impl MessageQueue { } pub fn get_sender(&self) -> MessageQueueSender { + check_global_threading_available!(); let mut_self = self as *const Self as *mut Self; //oh look, a C developer wrote this unsafe { (*mut_self).initialize() }; //TODO this might be safe (we are in init), but does not look very good let queue_id = self.queue_id.expect("Queue uninitialized"); @@ -68,13 +74,18 @@ impl MessageQueueSender { } pub fn send(&self, message: Message) -> Result<(), ()> { - osal::check_global_threading_available(); + check_global_threading_available!(); let queue_id = match self.queue_id { None => return Err(()), Some(id) => id, }; let res: u8; unsafe { + // safe beacuse: + // OS will read not more than length of message queue elements + // queue was created with size_of:: as length of message queue elements + // in MessageQueue::initialize + // making this pointer access safe as long as OS is holding the contract let message_pointer: *const core::ffi::c_void = &message as *const _ as *const core::ffi::c_void; res = osal::queue_send(queue_id, message_pointer); diff --git a/mission_rust/src/fsrc/tasks/mod.rs b/mission_rust/src/fsrc/tasks/mod.rs index 936a43c..3013ca3 100644 --- a/mission_rust/src/fsrc/tasks/mod.rs +++ b/mission_rust/src/fsrc/tasks/mod.rs @@ -89,8 +89,17 @@ pub struct TaskExecutor<'a> { } impl<'a> TaskExecutor<'a> { - pub fn init_and_run(&mut self) { - //TODO unlock global multitasking mutex + /// Initializes all tasks (and the objects contained within them) + /// and starts them. + /// + /// It also enables global threading, allowing threading APIs to be used (queues, mutexes etc) + /// See TODO about threading + /// + ///# Arguments + /// + /// * `delete_init_task` - If true, will delete the init task (the task this functions is called in) which means the function will not return + /// + pub fn init_and_run(&mut self, delete_init_task: bool) { let object_manager = TaskObjectManager { tasks: unsafe { slice::from_raw_parts(self.tasks.as_ptr(), self.tasks.len()) }, }; @@ -98,17 +107,22 @@ impl<'a> TaskExecutor<'a> { let _ = task.initialize(&object_manager).unwrap(); } drop(object_manager); + crate::fsrc::osal::enable_global_threading(); for task in self.tasks.iter_mut() { // we give away a raw pointer, to be called by an OS task // while this is generally very broken, we use a reference tied - // to our own lifetime and destroy the task when we get dropped + // to our own lifetime and destroy the task when we get dropped. // this way, the reference is guaranteed to be valid over our // lifetime while the task is deleted at the end of our lifetime - let task_pointer: *const core::ffi::c_void = *task as *mut _ as *const core::ffi::c_void; //TODO this does work without the "*" in front of the task -> Why?? + let task_pointer: *const core::ffi::c_void = + *task as *mut _ as *const core::ffi::c_void; //TODO this does work without the "*" in front of the task -> Why?? let handle; unsafe { - handle = - crate::fsrc::osal::create_task(task_entry, task_pointer, u32::try_from(task.get_stack_size()).unwrap()); + handle = crate::fsrc::osal::create_task( + task_entry, + task_pointer, + u32::try_from(task.get_stack_size()).unwrap(), + ); } if handle == 0 as *mut core::ffi::c_void { panic!("could not create Task"); @@ -116,6 +130,9 @@ impl<'a> TaskExecutor<'a> { task.set_handle(handle); } } + if delete_init_task { + crate::osal::task_delete_self(); + } } } @@ -141,7 +158,7 @@ impl<'a> crate::objectmanager::ObjectManager<'a> for TaskObjectManager<'a> { impl<'a> Drop for TaskExecutor<'a> { fn drop(&mut self) { - //TODO lock global multitasking mutex + crate::fsrc::osal::disable_global_threading(); for task in self.tasks.iter_mut() { unsafe { crate::fsrc::osal::delete_task(task.get_handle()); diff --git a/mission_rust/src/lib.rs b/mission_rust/src/lib.rs index 590aacc..75bbf24 100644 --- a/mission_rust/src/lib.rs +++ b/mission_rust/src/lib.rs @@ -199,7 +199,7 @@ fn mission() { tasks: &mut [&mut t1, &mut t2], }; - task_executor.init_and_run(); + task_executor.init_and_run(false); sifln!("Mission delay"); unsafe {