diff --git a/Native/global_state/src/global_state.rs b/Native/global_state/src/global_state.rs index f2d5875..c579b53 100644 --- a/Native/global_state/src/global_state.rs +++ b/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, pub run_context: String, @@ -257,8 +266,13 @@ pub static mut GLOBAL_STATE: HookState = HookState { }; pub static mut ANIM_SNAP_STATE:UnsafeCell> = UnsafeCell::new(None); -/// Loaded mod database -pub static mut LOADED_MODS: Option = 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> = Mutex::new(None); const TRACK_GS_PTR:bool = true; diff --git a/Native/hook_core/src/hook_render.rs b/Native/hook_core/src/hook_render.rs index 51c3544..6e1df4a 100644 --- a/Native/hook_core/src/hook_render.rs +++ b/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); diff --git a/Native/hook_core/src/hook_render_d3d11.rs b/Native/hook_core/src/hook_render_d3d11.rs index 4292f54..da6cddc 100644 --- a/Native/hook_core/src/hook_render_d3d11.rs +++ b/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 }, diff --git a/Native/hook_core/src/input_commands.rs b/Native/hook_core/src/input_commands.rs index baec754..f82af82 100644 --- a/Native/hook_core/src/input_commands.rs +++ b/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) { diff --git a/Native/mod_load/src/load_thread.rs b/Native/mod_load/src/load_thread.rs index 9c9b9eb..3880b3f 100644 --- a/Native/mod_load/src/load_thread.rs +++ b/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)); } diff --git a/Native/mod_load/src/mod_load.rs b/Native/mod_load/src/mod_load.rs index 94e329f..efb120f 100644 --- a/Native/mod_load/src/mod_load.rs +++ b/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(); diff --git a/Native/mod_stats/src/mod_stats.rs b/Native/mod_stats/src/mod_stats.rs index d96e785..fd36656 100644 --- a/Native/mod_stats/src/mod_stats.rs +++ b/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(); } } \ No newline at end of file