Disable interrupts under Mutex

crates/smb2_practice_mod/Cargo.toml

@@ -5,7 +5,7 @@ edition = "2021"
 
 [dependencies]
 arrayvec = { version = "0.7.6", default-features = false }
-critical-section = "1.1.3"
+critical-section = { version = "1.1.3", features = ["restore-state-u32"] }
 num_enum = { version = "0.7.3", default-features = false }
 once_cell = { version = "1.20.2", default-features = false, features = [
     "critical-section",

crates/smb2_practice_mod/src/app.rs

@@ -21,7 +21,7 @@ use crate::{
         pad::Pad,
         pref::{BoolPref, Pref},
     },
-    utils::{libsavestate::LibSaveState, misc::with_mutex, relutil::ModuleId},
+    utils::{libsavestate::LibSaveState, relutil::ModuleId},
 };
 
 static APP_CONTEXT: Lazy<Mutex<RefCell<AppContext>>> =
@@ -31,22 +31,6 @@ pub fn with_app<T>(f: impl FnOnce(&mut AppContext) -> T) -> T {
     critical_section::with(|cs| f(&mut APP_CONTEXT.borrow_ref_mut(cs)))
 }
 
-struct Globals {
-    process_inputs_hook: ProcessInputsHook,
-    draw_debug_text_hook: DrawDebugTextHook,
-    oslink_hook: OSLinkHook,
-    game_ready_init_hook: GameReadyInitHook,
-    game_play_tick_hook: GamePlayTickHook,
-}
-
-static GLOBALS: Mutex<Globals> = Mutex::new(Globals {
-    process_inputs_hook: ProcessInputsHook::new(),
-    draw_debug_text_hook: DrawDebugTextHook::new(),
-    oslink_hook: OSLinkHook::new(),
-    game_ready_init_hook: GameReadyInitHook::new(),
-    game_play_tick_hook: GamePlayTickHook::new(),
-});
-
 #[derive(Default)]
 pub struct AppContext {
     pub pad: Pad,
@@ -72,32 +56,35 @@ pub struct AppContext {
     pub il_mark: IlMark,
     pub camera: Camera,
     pub stage_edits: StageEdits,
-    pub _hide: Hide,
+    pub hide: Hide,
     pub sfx: Sfx,
     pub scratch: Scratch,
     pub validate: Validate,
     pub draw: Draw,
     pub pref: Pref,
+
+    process_inputs_hook: ProcessInputsHook,
+    draw_debug_text_hook: DrawDebugTextHook,
+    oslink_hook: OSLinkHook,
+    game_ready_init_hook: GameReadyInitHook,
+    game_play_tick_hook: GamePlayTickHook,
 }
 
 impl AppContext {
     fn new() -> Self {
         crate::systems::heap::HEAP.init();
-        with_mutex(&GLOBALS, |cx| {
-            cx.process_inputs_hook.hook();
-            cx.draw_debug_text_hook.hook();
-            cx.oslink_hook.hook();
-        });
 
-        Self::default()
+        let app_context = Self::default();
+        app_context.process_inputs_hook.hook();
+        app_context.draw_debug_text_hook.hook();
+        app_context.oslink_hook.hook();
+        app_context
     }
 }
 
 // Tick functions hook
 hook!(ProcessInputsHook => (), mkb::process_inputs, || {
-    with_mutex(&GLOBALS, |cx| {
-        cx.process_inputs_hook.call();
-    });
+    with_app(|cx| cx.process_inputs_hook.clone()).call();
 
     with_app(|cx| {
         cx.pad.tick();
@@ -140,28 +127,25 @@ hook!(ProcessInputsHook => (), mkb::process_inputs, || {
 
 // Draw functions hook
 hook!(DrawDebugTextHook => (), mkb::draw_debugtext, || {
-    with_mutex(&GLOBALS, |cx| {
-        cx.draw_debug_text_hook.call();
-    });
-
-    // When the game is paused, screenshot the game's draw buffer before we draw our custom UI
-    // elements. The original screenshot call is nopped.
-    unsafe {
-        if mkb::g_pause_status == 1 {
-            mkb::take_pausemenu_screenshot(
-                &raw mut mkb::fullscreen_texture_buf as *mut _,
-                0,
-                0,
-                (*mkb::current_render_mode).fbWidth as i16,
-                (*mkb::current_render_mode).efbHeight as i16,
-                mkb::GX_TF_RGB5A3,
-            );
-        }
-    };
+    with_app(|cx| cx.draw_debug_text_hook.clone()).call();
 
     with_app(|cx| {
-        cx.draw.predraw();
+        // When the game is paused, screenshot the game's draw buffer before we draw our custom UI
+        // elements. The original screenshot call is nopped.
+        unsafe {
+            if mkb::g_pause_status == 1 {
+                mkb::take_pausemenu_screenshot(
+                    &raw mut mkb::fullscreen_texture_buf as *mut _,
+                    0,
+                    0,
+                    (*mkb::current_render_mode).fbWidth as i16,
+                    (*mkb::current_render_mode).efbHeight as i16,
+                    mkb::GX_TF_RGB5A3,
+                );
+            }
+        };
 
+        cx.draw.predraw();
         cx.timer.draw(&cx.pref, &cx.freecam, &cx.validate);
         cx.iw.draw(&cx.pref, &cx.freecam);
         cx.il_battle.draw(&cx.pref, &cx.freecam, &cx.binds, &cx.pad);
@@ -192,32 +176,30 @@ hook!(OSLinkHook,
         mkb::OSLink,
         |rel_buffer, bss_buffer| {
 
-    with_mutex(&GLOBALS, |cx| {
-        let ret = cx.oslink_hook.call(rel_buffer, bss_buffer);
+    let ret = with_app(|cx| cx.oslink_hook.clone()).call(rel_buffer, bss_buffer);
+
+    with_app(|cx| {
         let module_id = ModuleId::try_from(unsafe{*rel_buffer}.info.id);
         if let Ok(ModuleId::MainGame) = module_id {
             cx.game_ready_init_hook.hook();
             cx.game_play_tick_hook.hook();
         }
-        ret
-    })
+    });
+
+    ret
 });
 
 hook!(GameReadyInitHook => (), mkb::smd_game_ready_init, || {
     with_app(|cx| {
         cx.stage_edits.on_game_ready_init(&cx.pref);
         cx.ball_color.switch_monkey(&cx.pref);
-    });
 
-    with_mutex(&GLOBALS, |cx| {
-        cx.game_ready_init_hook.call();
-    });
+        cx.game_ready_init_hook.clone()
+    }).call();
 });
 
 hook!(GamePlayTickHook => (), mkb::smd_game_play_tick, || {
-    with_mutex(&GLOBALS, |cx| {
-        cx.game_play_tick_hook.call();
-    });
+    with_app(|cx| cx.game_play_tick_hook.clone()).call();
 
     with_app(|cx| {
         cx.validate.validate_run(&cx.pref, &cx.lib_save_state, &cx.menu_impl, &cx.physics, &cx.pad);

crates/smb2_practice_mod/src/lib.rs

@@ -22,6 +22,8 @@ use core::{
 
 use critical_section::RawRestoreState;
 
+use mkb::mkb;
+
 // Disable unless we're buliding with our custom target explicitly, so rust-analyzer doesn't trip up
 // on conflicting panic handler implementations with std
 #[cfg(target_arch = "powerpc")]
@@ -99,12 +101,18 @@ unsafe fn perform_assembly_patches() {
     );
 }
 
-// We're never running multiple "threads" or hooking interrupts, so establishing a critical section
-// is a no-op
+// We disable interrupts while basically any of our code runs, even though we never hook anything
+// called in interrupt context as far as I know. It still seems to help prevent random crashes
+// somehow.
+// I guess another possibility is for an interrupt to trigger at a point in our code with high
+// stack usage, using enough additional stack to cause a stack overflow.
 struct MyCriticalSection;
 critical_section::set_impl!(MyCriticalSection);
 unsafe impl critical_section::Impl for MyCriticalSection {
-    unsafe fn acquire() -> RawRestoreState {}
-
-    unsafe fn release(_token: RawRestoreState) {}
+    unsafe fn acquire() -> RawRestoreState {
+        mkb::OSDisableInterrupts() as u32
+    }
+    unsafe fn release(token: RawRestoreState) {
+        mkb::OSRestoreInterrupts(token as i32);
+    }
 }

crates/smb2_practice_mod/src/mods/ballcolor.rs

@@ -1,4 +1,3 @@
-use critical_section::Mutex;
 use num_enum::TryFromPrimitive;
 
 use mkb::mkb;
@@ -10,24 +9,13 @@ use crate::{
         draw,
         pref::{Pref, U8Pref},
     },
-    utils::misc::with_mutex,
 };
 
-struct Globals {
-    load_stagedef_hook: LoadStagedefHook,
-}
-
-static GLOBALS: Mutex<Globals> = Mutex::new(Globals {
-    load_stagedef_hook: LoadStagedefHook::new(),
-});
-
 pub const COLOR_MIN: u8 = 0;
 pub const COLOR_MAX: u8 = 0xff;
 
 hook!(LoadStagedefHook, stage_id: u32 => (), mkb::load_stagedef, |stage_id| {
-    with_mutex(&GLOBALS, |cx| {
-        cx.load_stagedef_hook.call(stage_id);
-    });
+    with_app(|cx| cx.ball_color.load_stagedef_hook.clone()).call(stage_id);
     with_app(|cx| {
         cx.ball_color.switch_monkey(&cx.pref);
     });
@@ -64,17 +52,19 @@ pub struct BallColor {
     rainbow: u32,
     default_color: mkb::GXColor,
     current_color: mkb::GXColor,
+
+    load_stagedef_hook: LoadStagedefHook,
 }
 
 impl Default for BallColor {
     fn default() -> Self {
-        with_mutex(&GLOBALS, |cx| {
-            cx.load_stagedef_hook.hook();
-        });
+        let load_stagedef_hook = LoadStagedefHook::new();
+        load_stagedef_hook.hook();
         Self {
             rainbow: 0,
             default_color: unsafe { *(0x80472a34 as *const mkb::GXColor) },
             current_color: Default::default(),
+            load_stagedef_hook,
         }
     }
 }

crates/smb2_practice_mod/src/mods/cmseg.rs

@@ -1,6 +1,5 @@
 use core::{ffi::c_long, ptr::null_mut};
 
-use critical_section::Mutex;
 use mkb::mkb;
 use num_enum::TryFromPrimitive;
 
@@ -11,23 +10,13 @@ use crate::{
         draw,
         pref::{BoolPref, Pref, U8Pref},
     },
-    utils::{misc::with_mutex, timerdisp},
+    utils::timerdisp,
 };
 
 use super::freecam::Freecam;
 
-struct Globals {
-    reset_cm_course_hook: ResetCmCourseHook,
-}
-
-static GLOBALS: Mutex<Globals> = Mutex::new(Globals {
-    reset_cm_course_hook: ResetCmCourseHook::new(),
-});
-
 hook!(ResetCmCourseHook => (), mkb::g_reset_cm_course, || {
-    with_mutex(&GLOBALS, |cx| {
-        cx.reset_cm_course_hook.call();
-    });
+    with_app(|cx| cx.cm_seg.reset_cm_course_hook.clone()).call();
     with_app(|cx| {
         cx.cm_seg.on_reset_cm_course();
     });
@@ -82,13 +71,14 @@ pub struct CmSeg {
     overwritten_opcode: mkb::CourseCommandOpcode,
     overwritten_starting_monkeys: i8,
     pbs: [u32; 14],
+
+    reset_cm_course_hook: ResetCmCourseHook,
 }
 
 impl Default for CmSeg {
     fn default() -> Self {
-        with_mutex(&GLOBALS, |cx| {
-            cx.reset_cm_course_hook.hook();
-        });
+        let reset_cm_course_hook = ResetCmCourseHook::new();
+        reset_cm_course_hook.hook();
         Self {
             state: Default::default(),
             seg_request: Default::default(),
@@ -99,6 +89,8 @@ impl Default for CmSeg {
             overwritten_opcode: 0,
             overwritten_starting_monkeys: 0,
             pbs: [u32::MAX; 14],
+
+            reset_cm_course_hook,
         }
     }
 }

crates/smb2_practice_mod/src/mods/dpad.rs

@@ -2,46 +2,36 @@ use crate::{
     app::with_app,
     hook,
     systems::pref::{BoolPref, Pref},
-    utils::misc::with_mutex,
 };
-use critical_section::Mutex;
 use mkb::mkb;
 
 hook!(PadReadHook, statuses: *mut mkb::PADStatus => u32, mkb::PADRead, |statuses| {
-    let ret = with_mutex(&GLOBALS, |cx| {
-        cx.pad_read_hook.call(statuses)
-    });
+    let ret = with_app(|cx| cx.dpad.pad_read_hook.clone()).call(statuses);
     with_app(|cx| {
         cx.dpad.on_pad_read(statuses, &cx.pref);
     });
     ret
 });
 
 hook!(CreateSpeedSpritesHook, x: f32, y: f32 => (), mkb::create_speed_sprites, |x, y| {
-    with_mutex(&GLOBALS, |cx| {
-        cx.create_speed_sprites_hook.call(x + 5.0, y);
-    });
+    with_app(|cx| cx.dpad.create_speed_sprites_hook.clone()).call(x + 5.0, y);
 });
 
-struct Globals {
+pub struct Dpad {
     pad_read_hook: PadReadHook,
     create_speed_sprites_hook: CreateSpeedSpritesHook,
 }
 
-static GLOBALS: Mutex<Globals> = Mutex::new(Globals {
-    pad_read_hook: PadReadHook::new(),
-    create_speed_sprites_hook: CreateSpeedSpritesHook::new(),
-});
-
-pub struct Dpad {}
-
 impl Default for Dpad {
     fn default() -> Self {
-        with_mutex(&GLOBALS, |cx| {
-            cx.pad_read_hook.hook();
-            cx.create_speed_sprites_hook.hook();
-        });
-        Self {}
+        let pad_read_hook = PadReadHook::new();
+        pad_read_hook.hook();
+        let create_speed_sprites_hook = CreateSpeedSpritesHook::new();
+        create_speed_sprites_hook.hook();
+        Self {
+            pad_read_hook,
+            create_speed_sprites_hook,
+        }
     }
 }