continuing on thread safety

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 {
+pub fn task_delete_self() {
-    unsafe{global_threading_available_c() == 1}
+    unsafe {
-}
+        delete_task(0 as *const core::ffi::c_void);
-
-pub fn check_global_threading_available() {
-    if !global_threading_available() {
-        panic!("using queue outside of threading environment")
     }
 }
 
-pub fn enable_global_threading(){
+pub fn global_threading_available() -> bool {
-    unsafe{enable_global_threading_c()};
+    unsafe { global_threading_available_c() == 1 }
 }
 
-pub fn disable_global_threading(){
+pub fn enable_global_threading() {
-    unsafe{disable_global_threading_c()};
+    unsafe { enable_global_threading_c() };
+}
+
+pub fn disable_global_threading() {
+    unsafe { disable_global_threading_c() };
 }

mission_rust/src/fsrc/queues/mod.rs

@@ -1,4 +1,4 @@
-use crate::osal;
+use crate::{check_global_threading_available, osal};
 
 pub struct MessageQueue<const DEPTH: usize> {
     queue_data: [Message; DEPTH],
@@ -18,7 +18,7 @@ impl<const DEPTH: usize> MessageQueue<DEPTH> {
     }
 
     pub fn receive(&self) -> Option<Message> {
-        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<const DEPTH: usize> MessageQueue<DEPTH> {
         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::<Message> 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<const DEPTH: usize> MessageQueue<DEPTH> {
     }
 
     fn initialize(&mut self) {
+        check_global_threading_available!();
         if self.queue_id != None {
             return;
         }
@@ -53,6 +58,7 @@ impl<const DEPTH: usize> MessageQueue<DEPTH> {
     }
 
     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::<Message> 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);

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) {
+    /// Initializes all tasks (and the objects contained within them)
-        //TODO unlock global multitasking mutex
+    /// 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 =
+                handle = crate::fsrc::osal::create_task(
-                    crate::fsrc::osal::create_task(task_entry, task_pointer, u32::try_from(task.get_stack_size()).unwrap());
+                    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());

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 {