Remove DeviceHandle mutex by not reusing event objects

2 files changed, 31 insertions, 41 deletions

diff --git a/talpid-core/src/split_tunnel/windows/driver.rs b/talpid-core/src/split_tunnel/windows/driver.rs
index 7103ae9ee0..e33f1abc13 100644
--- a/talpid-core/src/split_tunnel/windows/driver.rs
+++ b/talpid-core/src/split_tunnel/windows/driver.rs
@@ -124,20 +124,13 @@ pub enum SplittingChangeReason {
 
 pub struct DeviceHandle {
     handle: fs::File,
-    overlapped: OVERLAPPED,
 }
 
+unsafe impl Sync for DeviceHandle {}
 unsafe impl Send for DeviceHandle {}
 
 impl DeviceHandle {
     pub fn new() -> io::Result<Self> {
-        let mut overlapped: OVERLAPPED = unsafe { mem::zeroed() };
-        overlapped.hEvent = unsafe { CreateEventW(ptr::null_mut(), TRUE, FALSE, ptr::null()) };
-
-        if overlapped.hEvent == ptr::null_mut() {
-            return Err(io::Error::last_os_error());
-        }
-
         // Connect to the driver
         log::trace!("Connecting to the driver");
         let handle = OpenOptions::new()
@@ -148,7 +141,7 @@ impl DeviceHandle {
             .attributes(0)
             .open(DRIVER_SYMBOLIC_NAME)?;
 
-        let device = Self { handle, overlapped };
+        let device = Self { handle };
 
         // Initialize the driver
         let state = device.get_driver_state()?;
@@ -173,7 +166,6 @@ impl DeviceHandle {
             DriverIoctlCode::Initialize as u32,
             None,
             0,
-            &self.overlapped,
             Some(DRIVER_IO_TIMEOUT),
         )?;
         Ok(())
@@ -186,7 +178,6 @@ impl DeviceHandle {
             DriverIoctlCode::RegisterProcesses as u32,
             Some(&process_tree_buffer),
             0,
-            &self.overlapped,
             Some(DRIVER_IO_TIMEOUT),
         )?;
         Ok(())
@@ -248,7 +239,6 @@ impl DeviceHandle {
             DriverIoctlCode::RegisterIpAddresses as u32,
             Some(buffer),
             0,
-            &self.overlapped,
             Some(DRIVER_IO_TIMEOUT),
         )?;
 
@@ -261,7 +251,6 @@ impl DeviceHandle {
             DriverIoctlCode::GetState as u32,
             None,
             size_of::<u64>() as u32,
-            &self.overlapped,
             Some(DRIVER_IO_TIMEOUT),
         )?
         .unwrap();
@@ -287,7 +276,6 @@ impl DeviceHandle {
             DriverIoctlCode::SetConfiguration as u32,
             Some(&config),
             0,
-            &self.overlapped,
             Some(DRIVER_IO_TIMEOUT),
         )?;
 
@@ -300,7 +288,6 @@ impl DeviceHandle {
             DriverIoctlCode::ClearConfiguration as u32,
             None,
             0,
-            &self.overlapped,
             Some(DRIVER_IO_TIMEOUT),
         )?;
 
@@ -308,12 +295,6 @@ impl DeviceHandle {
     }
 }
 
-impl Drop for DeviceHandle {
-    fn drop(&mut self) {
-        unsafe { CloseHandle(self.overlapped.hEvent) };
-    }
-}
-
 impl AsRawHandle for DeviceHandle {
     fn as_raw_handle(&self) -> RawHandle {
         self.handle.as_raw_handle()
@@ -709,9 +690,28 @@ pub fn device_io_control(
     ioctl_code: u32,
     input: Option<&[u8]>,
     output_size: u32,
-    overlapped: &OVERLAPPED,
     timeout: Option<Duration>,
 ) -> Result<Option<Vec<u8>>, io::Error> {
+    struct HandleOwner {
+        handle: RawHandle,
+    }
+    impl Drop for HandleOwner {
+        fn drop(&mut self) {
+            unsafe { CloseHandle(self.handle) };
+        }
+    }
+
+    let mut overlapped: OVERLAPPED = unsafe { mem::zeroed() };
+    overlapped.hEvent = unsafe { CreateEventW(ptr::null_mut(), TRUE, FALSE, ptr::null()) };
+
+    if overlapped.hEvent == ptr::null_mut() {
+        return Err(io::Error::last_os_error());
+    }
+
+    let _handle_owner = HandleOwner {
+        handle: overlapped.hEvent,
+    };
+
     let mut out_buffer = if output_size > 0 {
         Some(Vec::with_capacity(output_size as usize))
     } else {
@@ -723,7 +723,7 @@ pub fn device_io_control(
         ioctl_code,
         input,
         out_buffer.as_mut(),
-        overlapped,
+        &overlapped,
         timeout,
     )
     .map(|()| out_buffer)
diff --git a/talpid-core/src/split_tunnel/windows/mod.rs b/talpid-core/src/split_tunnel/windows/mod.rs
index 87c9d7cba9..1cba3bfa9f 100644
--- a/talpid-core/src/split_tunnel/windows/mod.rs
+++ b/talpid-core/src/split_tunnel/windows/mod.rs
@@ -17,7 +17,7 @@ use std::{
     net::{IpAddr, Ipv4Addr, Ipv6Addr},
     os::windows::io::{AsRawHandle, RawHandle},
     ptr,
-    sync::{mpsc as sync_mpsc, Arc, Mutex, Weak},
+    sync::{mpsc as sync_mpsc, Arc, Weak},
     time::Duration,
 };
 use talpid_types::{tunnel::ErrorStateCause, ErrorExt};
@@ -111,7 +111,7 @@ type RequestTx = sync_mpsc::SyncSender<(Request, RequestResponseTx)>;
 const REQUEST_TIMEOUT: Duration = Duration::from_secs(5);
 
 struct EventThreadContext {
-    handle: Arc<Mutex<driver::DeviceHandle>>,
+    handle: Arc<driver::DeviceHandle>,
     event_overlapped: OVERLAPPED,
     quit_event: RawHandle,
 }
@@ -121,7 +121,7 @@ impl SplitTunnel {
     /// Initialize the driver.
     pub fn new(daemon_tx: Weak<mpsc::UnboundedSender<TunnelCommand>>) -> Result<Self, Error> {
         let handle = driver::DeviceHandle::new().map_err(Error::InitializationFailed)?;
-        let handle = Arc::new(Mutex::new(handle));
+        let handle = Arc::new(handle);
 
         let mut event_overlapped: OVERLAPPED = unsafe { mem::zeroed() };
         event_overlapped.hEvent =
@@ -157,7 +157,7 @@ impl SplitTunnel {
 
                 if let Err(error) = unsafe {
                     driver::device_io_control_buffer_async(
-                        event_context.handle.lock().unwrap().as_raw_handle(),
+                        event_context.handle.as_raw_handle(),
                         driver::DriverIoctlCode::DequeEvent as u32,
                         Some(&mut data_buffer),
                         None,
@@ -205,7 +205,7 @@ impl SplitTunnel {
 
                 let result = unsafe {
                     GetOverlappedResult(
-                        event_context.handle.lock().unwrap().as_raw_handle(),
+                        event_context.handle.as_raw_handle(),
                         &event_context.event_overlapped as *const _ as *mut _,
                         &mut returned_bytes,
                         TRUE,
@@ -285,7 +285,7 @@ impl SplitTunnel {
         })
     }
 
-    fn spawn_command_thread(handle: Arc<Mutex<driver::DeviceHandle>>) -> RequestTx {
+    fn spawn_command_thread(handle: Arc<driver::DeviceHandle>) -> RequestTx {
         let (tx, rx): (RequestTx, _) = sync_mpsc::sync_channel(3);
 
         std::thread::spawn(move || {
@@ -293,17 +293,9 @@ impl SplitTunnel {
                 let response = match request {
                     Request::SetPaths(paths) => {
                         if paths.len() > 0 {
-                            handle
-                                .lock()
-                                .expect("ST driver mutex poisoned")
-                                .set_config(&paths)
-                                .map_err(Error::SetConfiguration)
+                            handle.set_config(&paths).map_err(Error::SetConfiguration)
                         } else {
-                            handle
-                                .lock()
-                                .expect("ST driver mutex poisoned")
-                                .clear_config()
-                                .map_err(Error::SetConfiguration)
+                            handle.clear_config().map_err(Error::SetConfiguration)
                         }
                     }
                     Request::RegisterIps(
@@ -317,8 +309,6 @@ impl SplitTunnel {
                             tunnel_ipv6 = None;
                         }
                         handle
-                            .lock()
-                            .expect("ST driver mutex poisoned")
                             .register_ips(tunnel_ipv4, tunnel_ipv6, internet_ipv4, internet_ipv6)
                             .map_err(Error::RegisterIps)
                     }