Wrap LOADED_MODS in a std Mutex

Native/global_state/src/global_state.rs

@@ -13,6 +13,7 @@ use std::cell::UnsafeCell;
 use std::collections::VecDeque;
 use std::marker::PhantomData;
 use std::ptr::addr_of_mut;
+use std::sync::Mutex;
 use std::sync::atomic::AtomicI64;
 use std::time::SystemTime;
 use std::fmt;
@@ -98,6 +99,14 @@ pub struct LoadedModState {
     pub selected_variant: SelectedVariantMap,
 }
 
+// `LoadedModState` holds raw d3d resource pointers (textures, buffers) which
+// are not `Send` by default. We need `Send` so that the global `Mutex` can be
+// `Sync`, which lets multiple threads (render thread, deferred load thread)
+// access it through the lock. The same `unsafe impl Send` pattern is already
+// used for `LoadMsg` in `mod_load::load_thread`, where a cloned `NativeModData`
+// is shipped across threads.
+unsafe impl Send for LoadedModState {}
+
 pub struct ClrState {
     pub runtime_pointer: Option<u64>,
     pub run_context: String,
@@ -257,8 +266,13 @@ pub static mut GLOBAL_STATE: HookState = HookState {
 };
 pub static mut ANIM_SNAP_STATE:UnsafeCell<Option<AnimSnapState>> = UnsafeCell::new(None);
 
-/// Loaded mod database
-pub static mut LOADED_MODS: Option<LoadedModState> = None;
+/// Loaded mod database.
+///
+/// Wrapped in a `Mutex` because the render thread reads/mutates this on every
+/// draw call while the deferred load thread writes back into it when a mod
+/// finishes loading. Callers should keep the lock for as short a span as
+/// possible — particularly on the DIP path.
+pub static LOADED_MODS: Mutex<Option<LoadedModState>> = Mutex::new(None);
 
 const TRACK_GS_PTR:bool = true;
 

Native/hook_core/src/hook_render.rs

@@ -802,7 +802,14 @@ where
         ptr: GLOBAL_STATE.bound_vertex_buffer,
         checksums: GLOBAL_STATE.vb_checksums.as_ref(),
     };
-    let res = LOADED_MODS.as_mut()
+    let mut loaded_mods_guard = match LOADED_MODS.lock() {
+        Ok(g) => g,
+        Err(e) => {
+            write_log_file(&format!("check_and_render_mod: LOADED_MODS lock poisoned: {}", e));
+            return CheckRenderModResult::NotRendered;
+        }
+    };
+    let res = loaded_mods_guard.as_mut()
         .and_then(|mods| {
             profile_start!(hdip, mod_select);
 

Native/hook_core/src/hook_render_d3d11.rs

@@ -642,9 +642,15 @@ pub unsafe extern "system" fn hook_draw_indexed(
 
                 // if there is a matching mod, render it
                 profile_start!(hdi, mod_precheck);
-                let quickcheck = LOADED_MODS.as_mut().map(
-                    |mods| mod_render::preselect(mods, prim_count, vert_count))
-                    .unwrap_or(false);
+                let quickcheck = match LOADED_MODS.lock() {
+                    Ok(mut g) => g.as_mut().map(
+                        |mods| mod_render::preselect(mods, prim_count, vert_count))
+                        .unwrap_or(false),
+                    Err(e) => {
+                        write_log_file(&format!("preselect: LOADED_MODS lock poisoned: {}", e));
+                        false
+                    }
+                };
                 let mod_status = if !quickcheck {
                     profile_end!(hdi, mod_precheck);
                     // this write_log_file can be used to get information about misses.
@@ -679,21 +685,28 @@ pub unsafe extern "system" fn hook_draw_indexed(
                     CheckRenderModResult::Deleted => false,
                     CheckRenderModResult::NotRenderedButLoadRequested(ref name) => {
                         // setup data to begin mod load
-                        let nmod = mod_load::get_mod_by_name(name, &mut LOADED_MODS);
-                        if let Some(nmod) = nmod {
-                            // need to store current input layout in the d3d data
-                            if let ModD3DState::Unloaded =  nmod.d3d_data {
-                                let il = state.rs.current_input_layout;
-                                if !il.is_null() {
-                                    // we're officially keeping an extra reference to the input layout now
-                                    // so note that.
-                                    (*il).AddRef();
-                                    nmod.d3d_data = ModD3DState::Partial(
-                                        ModD3DData::D3D11(ModD3DData11::with_layout(il)));
-                                    write_log_file(&format!("created partial mod load state for mod {}", nmod.name));
-                                    //write_log_file(&format!("current in layout is: {}", il as usize));
+                        match LOADED_MODS.lock() {
+                            Ok(mut loaded_mods_guard) => {
+                                let nmod = mod_load::get_mod_by_name(name, &mut *loaded_mods_guard);
+                                if let Some(nmod) = nmod {
+                                    // need to store current input layout in the d3d data
+                                    if let ModD3DState::Unloaded =  nmod.d3d_data {
+                                        let il = state.rs.current_input_layout;
+                                        if !il.is_null() {
+                                            // we're officially keeping an extra reference to the input layout now
+                                            // so note that.
+                                            (*il).AddRef();
+                                            nmod.d3d_data = ModD3DState::Partial(
+                                                ModD3DData::D3D11(ModD3DData11::with_layout(il)));
+                                            write_log_file(&format!("created partial mod load state for mod {}", nmod.name));
+                                            //write_log_file(&format!("current in layout is: {}", il as usize));
+                                        }
+                                    }
                                 }
                             }
+                            Err(e) => {
+                                write_log_file(&format!("NotRenderedButLoadRequested: LOADED_MODS lock poisoned: {}", e));
+                            }
                         }
                         true
                     },

Native/hook_core/src/input_commands.rs

@@ -331,19 +331,33 @@ fn select_next_variant() {
     let hookstate = unsafe { &mut GLOBAL_STATE };
     let lastframe = hookstate.metrics.total_frames;
 
-    unsafe { LOADED_MODS.as_mut() }.map(|mstate| {
-        mod_render::select_next_variant(mstate, lastframe);
-        mod_prefs::save_variant_selections(&mstate.mods, &mstate.selected_variant);
-    });
+    match LOADED_MODS.lock() {
+        Ok(mut g) => {
+            g.as_mut().map(|mstate| {
+                mod_render::select_next_variant(mstate, lastframe);
+                mod_prefs::save_variant_selections(&mstate.mods, &mstate.selected_variant);
+            });
+        }
+        Err(e) => {
+            write_log_file(&format!("select_next_variant: LOADED_MODS lock poisoned: {}", e));
+        }
+    }
 }
 fn select_prev_variant() {
     let hookstate = unsafe { &mut GLOBAL_STATE };
     let lastframe = hookstate.metrics.total_frames;
 
-    unsafe { LOADED_MODS.as_mut() }.map(|mstate| {
-        mod_render::select_prev_variant(mstate, lastframe);
-        mod_prefs::save_variant_selections(&mstate.mods, &mstate.selected_variant);
-    });
+    match LOADED_MODS.lock() {
+        Ok(mut g) => {
+            g.as_mut().map(|mstate| {
+                mod_render::select_prev_variant(mstate, lastframe);
+                mod_prefs::save_variant_selections(&mstate.mods, &mstate.selected_variant);
+            });
+        }
+        Err(e) => {
+            write_log_file(&format!("select_prev_variant: LOADED_MODS lock poisoned: {}", e));
+        }
+    }
 }
 
 fn setup_fkey_input(device: DevicePointer, inp: &mut input::Input) {

Native/mod_load/src/load_thread.rs

@@ -187,15 +187,18 @@ fn load_resource(mut msg: LoadMsg) -> Result<(), String> {
         }
         else if loaded {
             let (diff,new_rc) = update_ref_count(msg.device, pre_rc);
-            // i don't think its necessary to lock here because this is just a read of the hash table, though 
-            // we'll overwrite the value if we find it
-            if let Some(ref mut oldnmod) = get_mod_by_index(msg.nmod.midx, &mut LOADED_MODS) {
-                // TODO(safety): I haven't seen a problem with this, but it may be a possible "torn write" where the render thread could observe a partially loaded struct,
-                // if d3d_data with Loaded is copied over before the remaining stuff (copy ordering not guaranteed).
-                // not clear on fix but maybe use atomics or else don't reuse the old hash slot at all (complicated because the mod is actually
-                // stored in a sorted vector in the hashtable entry - replace the whole vector?)
-                // other possibility is to get the global lock but that is ugh because I don't want to slow down draw primitive.
-                **oldnmod = msg.nmod.clone();
+            // The Mutex around LOADED_MODS now also covers the render thread's reads,
+            // so the prior "torn write" concern (render thread observing a half-copied
+            // NativeModData) is resolved as long as everyone goes through the lock.
+            match LOADED_MODS.lock() {
+                Ok(mut guard) => {
+                    if let Some(ref mut oldnmod) = get_mod_by_index(msg.nmod.midx, &mut *guard) {
+                        **oldnmod = msg.nmod.clone();
+                    }
+                }
+                Err(e) => {
+                    write_log_file(&format!("load thread: LOADED_MODS lock poisoned, skipping writeback for {}: {}", id, e));
+                }
             }
             write_log_file(&format!("load thread: load_resource added {} to device rc, new rc: {}", diff, new_rc));
         }

Native/mod_load/src/mod_load.rs

@@ -67,7 +67,13 @@ pub unsafe fn clear_loaded_mods(device: DevicePointer) {
     // get device ref count prior to adding everything
     let pre_rc = device.get_ref_count();
 
-    let mods = LOADED_MODS.take();
+    let mods = match LOADED_MODS.lock() {
+        Ok(mut g) => g.take(),
+        Err(e) => {
+            write_log_file(&format!("clear_loaded_mods: LOADED_MODS lock poisoned: {}", e));
+            return;
+        }
+    };
     let mut cnt = 0;
     mods.map(|mods| {
         for (_key, modvec) in mods.mods.into_iter() {
@@ -77,7 +83,6 @@ pub unsafe fn clear_loaded_mods(device: DevicePointer) {
             }
         }
     });
-    LOADED_MODS = None;
     GLOBAL_STATE.load_on_next_frame.as_mut().map(|hs| hs.clear());
 
     let post_rc = (device).get_ref_count();
@@ -719,11 +724,19 @@ pub unsafe fn setup_mod_data(device: DevicePointer, callbacks: interop::ManagedC
     let mut selected_variant = global_state::new_fnv_map(16);
     mod_prefs::load_and_apply_variants(&loaded_mods, &mut selected_variant);
 
-    LOADED_MODS = Some(LoadedModState {
-        mods: loaded_mods,
-        mods_by_name: mods_by_name,
-        selected_variant,
-    } );
+    match LOADED_MODS.lock() {
+        Ok(mut g) => {
+            *g = Some(LoadedModState {
+                mods: loaded_mods,
+                mods_by_name: mods_by_name,
+                selected_variant,
+            });
+        }
+        Err(e) => {
+            write_log_file(&format!("setup_mod_data: LOADED_MODS lock poisoned, mods will not be available: {}", e));
+            return;
+        }
+    }
 
     global_state::set_vb_checksum_targets(vb_checksum_targets);
 }
@@ -829,9 +842,16 @@ pub unsafe fn load_deferred_mods(device: DevicePointer, callbacks: interop::Mana
         let mut cnt = 0;
         let can_load_in_thread = (*DEVICE_STATE).multithreaded();
 
+        let mut loaded_mods_guard = match LOADED_MODS.lock() {
+            Ok(g) => g,
+            Err(e) => {
+                write_log_file(&format!("load_deferred_mods: LOADED_MODS lock poisoned: {}", e));
+                return;
+            }
+        };
         for nmd in to_load.iter() {
             let mut nmod =
-                get_mod_by_name(&nmd, &mut LOADED_MODS);
+                get_mod_by_name(&nmd, &mut *loaded_mods_guard);
             if let Some(ref mut nmod) = nmod {
                 if nmod.fill_attempts > MAX_FILL_ATTEMPTS {
                     if nmod.fill_attempts == MAX_FILL_ATTEMPTS + 1 {
@@ -919,6 +939,7 @@ pub unsafe fn load_deferred_mods(device: DevicePointer, callbacks: interop::Mana
                 }
             }
         }
+        drop(loaded_mods_guard);
 
         to_load.clear();
 

Native/mod_stats/src/mod_stats.rs

@@ -376,7 +376,10 @@ pub fn update(now:&SystemTime) -> Option<(u32,u32)> {
         let mut new_active = 0_u32;
         let mut total_active = 0_u32;
         let total_frames = unsafe { GLOBAL_STATE.metrics.total_frames };
-        unsafe { LOADED_MODS.as_ref() }
+        let loaded_mods_guard = LOADED_MODS.lock().map_err(|e| {
+            write_log_file(&format!("mod_stats update: LOADED_MODS lock poisoned: {}", e));
+        }).ok();
+        loaded_mods_guard.as_ref().and_then(|g| g.as_ref())
             .map(|m|  m.mods.values() )
             .map(|mlist| mlist.map(|m|
                 m.iter().filter(|nmd| {
@@ -727,7 +730,8 @@ mod tests {
 
         let set_mod_rendered = |name:&str, idx:usize| {
             let frame = unsafe { GLOBAL_STATE.metrics.total_frames };
-            let lms = unsafe { LOADED_MODS.as_mut().unwrap() };
+            let mut guard = LOADED_MODS.lock().unwrap();
+            let lms = guard.as_mut().unwrap();
             let mod_key = lms.mods_by_name.get(name).expect("mod not found");
             let nmd = lms.mods.get_mut(mod_key).expect("mod not found").get_mut(idx).expect("mod not found");
             nmd.d3d_data = ModD3DState::Loaded(ModD3DData::D3D11(ModD3DData11::new()));
@@ -750,7 +754,7 @@ mod tests {
             mods_by_name: mods_by_name,
             selected_variant: global_state::new_fnv_map(16),
         };
-        unsafe { LOADED_MODS = Some(lms); };
+        *LOADED_MODS.lock().unwrap() = Some(lms);
         set_update_interval_ms(0);
         assert_eq!(update(&SystemTime::now()), Some((0,0)));
         set_mod_rendered("mod_100_200_2", 1);
@@ -776,7 +780,7 @@ mod tests {
 
         std::thread::sleep(Duration::from_secs(1));
 
-        unsafe { LOADED_MODS = None };
+        *LOADED_MODS.lock().unwrap() = None;
         super::reset();
     }
 }