Skip to content

Commit 44161e8

Browse files
authored
Fix blocking windows syscalls (#825)
* fix * build * Update Cargo.lock * Delete build-windows-branch.yaml
1 parent 16d3c60 commit 44161e8

3 files changed

Lines changed: 59 additions & 48 deletions

File tree

src-tauri/Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src-tauri/src/enterprise/service_locations/windows.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use defguard_wireguard_rs::{
1515
};
1616
use known_folders::get_known_folder_path;
1717
use log::{debug, error, warn};
18-
use tokio::time::sleep;
1918
use windows::{
2019
core::PSTR,
2120
Win32::System::RemoteDesktop::{
@@ -55,7 +54,9 @@ const SERVICE_LOCATIONS_SUBDIR: &str = "service_locations";
5554
///
5655
/// Note: `NotifyAddrChange` also fires when WireGuard interfaces are created. This is
5756
/// harmless because `connect_to_service_locations` skips already-connected locations.
58-
pub(crate) async fn watch_for_network_change(
57+
///
58+
/// Runs on a dedicated OS thread because `NotifyAddrChange` is a blocking syscall.
59+
pub(crate) fn watch_for_network_change(
5960
service_location_manager: Arc<RwLock<ServiceLocationManager>>,
6061
) {
6162
loop {
@@ -65,22 +66,22 @@ pub(crate) async fn watch_for_network_change(
6566

6667
if result != 0 {
6768
error!("NotifyAddrChange failed with error code: {result}");
68-
sleep(NETWORK_CHANGE_MONITOR_RESTART_DELAY).await;
69+
std::thread::sleep(NETWORK_CHANGE_MONITOR_RESTART_DELAY);
6970
continue;
7071
}
7172

7273
debug!(
7374
"Network address change detected, waiting {NETWORK_STABILIZATION_DELAY:?}s for \
7475
network to stabilize before attempting service location connections..."
7576
);
76-
sleep(NETWORK_STABILIZATION_DELAY).await;
77+
std::thread::sleep(NETWORK_STABILIZATION_DELAY);
7778

7879
debug!("Attempting to connect to service locations after network change");
79-
match service_location_manager
80+
let connect_result = service_location_manager
8081
.write()
8182
.unwrap()
82-
.connect_to_service_locations()
83-
{
83+
.connect_to_service_locations();
84+
match connect_result {
8485
Ok(_) => {
8586
debug!("Service location connect attempt after network change completed");
8687
}
@@ -91,11 +92,15 @@ pub(crate) async fn watch_for_network_change(
9192
}
9293
}
9394

94-
pub(crate) async fn watch_for_login_logoff(
95+
/// Watches for user logon/logoff events and connects/disconnects pre-logon service locations
96+
/// accordingly.
97+
///
98+
/// Runs on a dedicated OS thread because `WTSWaitSystemEvent` is a blocking syscall.
99+
pub(crate) fn watch_for_login_logoff(
95100
service_location_manager: Arc<RwLock<ServiceLocationManager>>,
96101
) -> Result<(), ServiceLocationError> {
97102
loop {
98-
let mut event_flags = 0;
103+
let mut event_flags: u32 = 0;
99104
let success = unsafe {
100105
WTSWaitSystemEvent(
101106
Some(WTS_CURRENT_SERVER_HANDLE),
@@ -110,23 +115,21 @@ pub(crate) async fn watch_for_login_logoff(
110115
}
111116
Err(err) => {
112117
error!("Failed waiting for login/logoff event: {err:?}");
113-
sleep(Duration::from_secs(LOGIN_LOGOFF_EVENT_RETRY_DELAY_SECS)).await;
118+
std::thread::sleep(Duration::from_secs(LOGIN_LOGOFF_EVENT_RETRY_DELAY_SECS));
114119
continue;
115120
}
116121
};
117122

118123
if event_flags & WTS_EVENT_LOGON != 0 {
119124
debug!("Detected user logon, attempting to auto-disconnect from service locations.");
120125
service_location_manager
121-
.clone()
122126
.write()
123127
.unwrap()
124128
.disconnect_service_locations(Some(ServiceLocationMode::PreLogon))?;
125129
}
126130
if event_flags & WTS_EVENT_LOGOFF != 0 {
127131
debug!("Detected user logoff, attempting to auto-connect to service locations.");
128132
service_location_manager
129-
.clone()
130133
.write()
131134
.unwrap()
132135
.connect_to_service_locations()?;
@@ -281,7 +284,11 @@ pub(crate) fn is_user_logged_in() -> bool {
281284
buffer.0 as *mut _,
282285
);
283286

284-
// We found an active session with a username
287+
// We found an active session with a username.
288+
// Free the session list before returning to avoid a leak.
289+
windows::Win32::System::RemoteDesktop::WTSFreeMemory(
290+
pp_sessions as _,
291+
);
285292
return true;
286293
}
287294
}

src-tauri/src/service/windows.rs

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -115,17 +115,19 @@ fn run_service() -> Result<(), DaemonError> {
115115

116116
let service_location_manager = Arc::new(RwLock::new(service_location_manager));
117117

118-
// Spawn network change monitoring task first so NotifyAddrChange is registered as early
119-
// as possible, minimising the window in which a network event could be missed before
120-
// the watcher is listening. The retry task below is the backstop for any event that
121-
// still slips through that window.
118+
// Spawn network change monitoring on a dedicated OS thread so the blocking
119+
// NotifyAddrChange syscall does not stall Tokio's async worker threads.
120+
// Register it first so no network event can be missed before the watcher is listening;
121+
// the retry loop below is the backstop for any event that slips through the startup window.
122122
let service_location_manager_clone = service_location_manager.clone();
123-
runtime.spawn(async move {
124-
let manager = service_location_manager_clone;
125-
info!("Starting network change monitoring");
126-
watch_for_network_change(manager.clone()).await;
127-
error!("Network change monitoring ended unexpectedly.");
128-
});
123+
std::thread::Builder::new()
124+
.name("network-change-monitor".to_string())
125+
.spawn(move || {
126+
info!("Starting network change monitoring");
127+
watch_for_network_change(service_location_manager_clone);
128+
error!("Network change monitoring ended unexpectedly.");
129+
})
130+
.expect("Failed to spawn network change monitor thread");
129131

130132
// Spawn service location auto-connect task with retries.
131133
// Each attempt skips locations that are already connected, so it is safe to call
@@ -174,32 +176,34 @@ fn run_service() -> Result<(), DaemonError> {
174176
info!("Service location auto-connect task finished");
175177
});
176178

177-
// Spawn login/logoff monitoring task, runs concurrently with the tasks above.
179+
// Spawn login/logoff monitoring on a dedicated OS thread so the blocking
180+
// WTSWaitSystemEvent syscall does not stall Tokio's async worker threads.
178181
let service_location_manager_clone = service_location_manager.clone();
179-
runtime.spawn(async move {
180-
let manager = service_location_manager_clone;
181-
182-
info!("Starting login/logoff event monitoring");
183-
loop {
184-
match watch_for_login_logoff(manager.clone()).await {
185-
Ok(()) => {
186-
warn!(
187-
"Login/logoff event monitoring ended unexpectedly. Restarting in \
188-
{LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS:?}..."
189-
);
190-
sleep(LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS).await;
191-
}
192-
Err(e) => {
193-
error!(
194-
"Error in login/logoff event monitoring: {e}. Restarting in \
195-
{LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS:?}...",
196-
);
197-
sleep(LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS).await;
198-
info!("Restarting login/logoff event monitoring");
182+
std::thread::Builder::new()
183+
.name("login-logoff-monitor".to_string())
184+
.spawn(move || {
185+
info!("Starting login/logoff event monitoring");
186+
loop {
187+
match watch_for_login_logoff(service_location_manager_clone.clone()) {
188+
Ok(()) => {
189+
warn!(
190+
"Login/logoff event monitoring ended unexpectedly. Restarting in \
191+
{LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS:?}..."
192+
);
193+
std::thread::sleep(LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS);
194+
}
195+
Err(e) => {
196+
error!(
197+
"Error in login/logoff event monitoring: {e}. Restarting in \
198+
{LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS:?}...",
199+
);
200+
std::thread::sleep(LOGIN_LOGOFF_MONITORING_RESTART_DELAY_SECS);
201+
info!("Restarting login/logoff event monitoring");
202+
}
199203
}
200204
}
201-
}
202-
});
205+
})
206+
.expect("Failed to spawn login/logoff monitor thread");
203207

204208
// Spawn the main gRPC server task
205209
let service_location_manager_clone = service_location_manager.clone();

0 commit comments

Comments
 (0)