Minimal interrupt safety

crates/smb2_practice_mod/src/app.rs

@@ -28,7 +28,21 @@ static APP_CONTEXT: Lazy<Mutex<RefCell<AppContext>>> =
     Lazy::new(|| Mutex::new(RefCell::new(AppContext::new())));
 
 pub fn with_app<T>(f: impl FnOnce(&mut AppContext) -> T) -> T {
-    critical_section::with(|cs| f(&mut APP_CONTEXT.borrow_ref_mut(cs)))
+    // Disabling interrupts while our code apparently prevents the random crashes I've been seeing.
+    // Although I'm pretty sure we don't hook any functions which may be called in interrupt
+    // context, maybe we do. I guess another possibility is that an interrupt is triggered during a
+    // function of ours with high stack usage, pushing it over the edge and causing a stack
+    // overflow.
+    //
+    // It would be better to disable interrupts inside critical_section::Mutex, that's basically
+    // what it's designed for. However, my PR which does so (#58) increases the binary size by 10KB
+    // and I really don't understand why.
+    let interrupts = unsafe { mkb::OSDisableInterrupts() };
+    let ret = critical_section::with(|cs| f(&mut APP_CONTEXT.borrow_ref_mut(cs)));
+    unsafe {
+        mkb::OSRestoreInterrupts(interrupts);
+    }
+    ret
 }
 
 struct Globals {

crates/smb2_practice_mod/src/mods/freecam.rs

@@ -30,6 +30,7 @@ struct Globals {
     event_camera_tick_hook: EventCameraTickHook,
 }
 
+// Removing Lazy and const initializing adds 5KB to the binary. WTF.
 static GLOBALS: Lazy<Mutex<Globals>> = Lazy::new(|| Mutex::new(Globals::default()));
 
 struct Context<'a> {

crates/smb2_practice_mod/src/systems/cardio.rs

@@ -90,81 +90,91 @@ impl CardIo {
         }
     }
 
-    // Synchronous at the moment. Also, do not call while write_file() is running!
-    pub fn read_file(&mut self, file_name: &str) -> Result<Vec<u8>, CARDResult> {
-        unsafe {
-            let _fake_gamecode = FakeGamecode::new();
-            let mut res;
-
-            // Probe card
-            loop {
-                res = to_card_result(mkb::CARDProbeEx(0, null_mut(), null_mut()));
-                if res != CARDResult::Busy {
-                    break;
-                }
-            }
-            if res != CARDResult::Ready {
-                return Err(res);
-            }
-
-            // Mount card
-            mkb::CARDMountAsync(
-                0,
-                self.card_work_area.as_mut_ptr() as *mut c_void,
-                null_mut(),
-                null_mut(),
-            );
-            loop {
-                res = to_card_result(mkb::CARDGetResultCode(0));
-                if res != CARDResult::Busy {
-                    break;
-                }
-            }
-            if res != CARDResult::Ready {
-                return Err(res);
-            }
-            // Open file
-            res = to_card_result(mkb::CARDOpen(
-                0,
-                cstr!(16, file_name),
-                &mut self.card_file_info,
-            ));
-            if res != CARDResult::Ready {
-                mkb::CARDUnmount(0);
-                return Err(res);
+    unsafe fn read_file_internal(&mut self, file_name: &str) -> Result<Vec<u8>, CARDResult> {
+        let _fake_gamecode = FakeGamecode::new();
+        let mut res;
+
+        // Probe card
+        loop {
+            res = to_card_result(mkb::CARDProbeEx(0, null_mut(), null_mut()));
+            if res != CARDResult::Busy {
+                break;
             }
+        }
+        if res != CARDResult::Ready {
+            return Err(res);
+        }
 
-            // Get file size
-            let mut stat = mkb::CARDStat::default();
-            res = to_card_result(mkb::CARDGetStatus(0, self.card_file_info.fileNo, &mut stat));
-            if res != CARDResult::Ready {
-                mkb::CARDUnmount(0);
-                return Err(res);
+        // Mount card
+        mkb::CARDMountAsync(
+            0,
+            self.card_work_area.as_mut_ptr() as *mut c_void,
+            null_mut(),
+            null_mut(),
+        );
+        loop {
+            res = to_card_result(mkb::CARDGetResultCode(0));
+            if res != CARDResult::Busy {
+                break;
             }
+        }
+        if res != CARDResult::Ready {
+            return Err(res);
+        }
+        // Open file
+        res = to_card_result(mkb::CARDOpen(
+            0,
+            cstr!(16, file_name),
+            &mut self.card_file_info,
+        ));
+        if res != CARDResult::Ready {
+            mkb::CARDUnmount(0);
+            return Err(res);
+        }
 
-            let buf_size = math::round_up_pow2(stat.length as usize, CARD_READ_SIZE as usize);
-            let mut buf = vec![0u8; buf_size];
+        // Get file size
+        let mut stat = mkb::CARDStat::default();
+        res = to_card_result(mkb::CARDGetStatus(0, self.card_file_info.fileNo, &mut stat));
+        if res != CARDResult::Ready {
+            mkb::CARDUnmount(0);
+            return Err(res);
+        }
 
-            mkb::CARDReadAsync(
-                &mut self.card_file_info as *mut _,
-                buf.as_mut_ptr() as *mut _,
-                buf_size as c_long,
-                0,
-                null_mut(),
-            );
-            loop {
-                res = to_card_result(mkb::CARDGetResultCode(0));
-                if res != CARDResult::Busy {
-                    break;
-                }
-            }
-            if res != CARDResult::Ready {
-                mkb::CARDUnmount(0);
-                return Err(res);
+        let buf_size = math::round_up_pow2(stat.length as usize, CARD_READ_SIZE as usize);
+        let mut buf = vec![0u8; buf_size];
+
+        mkb::CARDReadAsync(
+            &mut self.card_file_info as *mut _,
+            buf.as_mut_ptr() as *mut _,
+            buf_size as c_long,
+            0,
+            null_mut(),
+        );
+        loop {
+            res = to_card_result(mkb::CARDGetResultCode(0));
+            if res != CARDResult::Busy {
+                break;
             }
-
+        }
+        if res != CARDResult::Ready {
             mkb::CARDUnmount(0);
-            Ok(buf)
+            return Err(res);
+        }
+
+        mkb::CARDUnmount(0);
+        Ok(buf)
+    }
+
+    // Synchronous at the moment. Also, do not call while write_file() is running!
+    pub fn read_file(&mut self, file_name: &str) -> Result<Vec<u8>, CARDResult> {
+        unsafe {
+            // HACK: re-enable interrupts during file reading. We could also make reading async like
+            // writing, but then if SMB2WorkshopMod did the same we'd have to coordinate
+            // non-overlapping reads on startup.
+            let interrupts = mkb::OSEnableInterrupts();
+            let result = self.read_file_internal(file_name);
+            mkb::OSRestoreInterrupts(interrupts);
+            result
         }
     }