From 74d0fddfbb570514118b4ddc228ce19ccd8bb969 Mon Sep 17 00:00:00 2001 From: Thomas Karpiniec Date: Sat, 8 Jun 2024 17:19:14 +1000 Subject: [PATCH] Clean up race condition in initial watch on Windows --- src/watch_win.rs | 53 ++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/watch_win.rs b/src/watch_win.rs index c88cd7c..67e0d4c 100644 --- a/src/watch_win.rs +++ b/src/watch_win.rs @@ -40,18 +40,10 @@ impl Drop for WatchHandle { } pub(crate) fn watch_interfaces( - mut callback: F, + callback: F, ) -> Result { - let null_list = List::default(); - let prev_list = crate::list::list_interfaces()?; - callback(Update { - interfaces: prev_list.0.clone(), - diff: prev_list.diff_from(&null_list), - }); - - // TODO: Can wo do something about the race condition? let state = Box::pin(Mutex::new(WatchState { - prev_list, + prev_list: List::default(), cb: Box::new(callback), })); let state_ctx = &*state.as_ref() as *const _ as *const c_void; @@ -67,7 +59,14 @@ pub(crate) fn watch_interfaces( ) }; match res { - NO_ERROR => Ok(WatchHandle { hnd, _state: state }), + NO_ERROR => { + // Trigger an initial update. + // This is allowed to race with true updates because it + // will always calculate a diff and discard no-ops. + handle_notif(&mut *state.lock().unwrap()); + // Then return the handle + Ok(WatchHandle { hnd, _state: state }) + } ERROR_INVALID_HANDLE => Err(Error::Internal), ERROR_INVALID_PARAMETER => Err(Error::Internal), ERROR_NOT_ENOUGH_MEMORY => Err(Error::Internal), @@ -80,24 +79,24 @@ unsafe extern "system" fn notif( _row: *const MIB_UNICASTIPADDRESS_ROW, _notification_type: MIB_NOTIFICATION_TYPE, ) { - println!("There was a change!"); - let Ok(new_list) = crate::list::list_interfaces() else { - println!("Failed to get list of interfaces on change"); - return; - }; let state_ptr = ctx as *const Mutex; unsafe { let state_guard = &mut *state_ptr.as_ref().unwrap().lock().unwrap(); - if new_list == state_guard.prev_list { - // TODO: Hitting this a lot, is it true? - println!("Interfaces seem to be the same, ignoring"); - return; - } - let update = Update { - interfaces: new_list.0.clone(), - diff: new_list.diff_from(&state_guard.prev_list), - }; - (state_guard.cb)(update); - state_guard.prev_list = new_list; + handle_notif(state_guard); + } +} + +fn handle_notif(state: &mut WatchState) { + let Ok(new_list) = crate::list::list_interfaces() else { + return; + }; + if new_list == state.prev_list { + return; } + let update = Update { + interfaces: new_list.0.clone(), + diff: new_list.diff_from(&state.prev_list), + }; + (state.cb)(update); + state.prev_list = new_list; } -- 2.39.5