Merge branch 'win-routing-without-futures'

7 files changed, 146 insertions, 161 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d8bde10194..ab55f92589 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,7 @@ Line wrap the file at 100 chars.                                              Th
 #### Windows
 - Register 'NSI' service as a dependency of the daemon service.
 - Set daemon service SID type as 'unrestricted'.
+- Properly tear down routes after disconnecting from WireGuard relays
 
 #### Android
 - Fix notification message to update to `null` version when version check cache is stale right
diff --git a/talpid-core/src/routing/linux/change_listener.rs b/talpid-core/src/routing/linux/change_listener.rs
index 1895a28761..82608a59a7 100644
--- a/talpid-core/src/routing/linux/change_listener.rs
+++ b/talpid-core/src/routing/linux/change_listener.rs
@@ -1,7 +1,6 @@
-use super::{
-    super::{Node, Route},
-    RouteChange,
-};
+use crate::routing::{Node, Route};
+
+use super::RouteChange;
 use futures::{future::Either, sync::mpsc, Async, Future, Stream};
 use std::{collections::BTreeMap, io, net::IpAddr};
 
diff --git a/talpid-core/src/routing/linux/mod.rs b/talpid-core/src/routing/linux/mod.rs
index 00020eccbc..9c2f688b84 100644
--- a/talpid-core/src/routing/linux/mod.rs
+++ b/talpid-core/src/routing/linux/mod.rs
@@ -1,4 +1,4 @@
-use super::{NetNode, Node, Route};
+use crate::routing::{NetNode, Node, Route};
 
 use ipnetwork::IpNetwork;
 use std::{
@@ -401,12 +401,12 @@ fn parse_ip_route_show_line(line: &str, ip_version: IpVersion) -> Option<Route>
     if node_ip.is_none() && device.is_none() {
         None
     } else {
-        let node = super::Node {
+        let node = Node {
             ip: node_ip,
             device,
         };
 
-        Some(super::Route {
+        Some(Route {
             node,
             prefix,
             metric,
diff --git a/talpid-core/src/routing/macos.rs b/talpid-core/src/routing/macos.rs
index d87f71e159..1d02d87de1 100644
--- a/talpid-core/src/routing/macos.rs
+++ b/talpid-core/src/routing/macos.rs
@@ -1,4 +1,4 @@
-use super::{NetNode, Node, Route};
+use crate::routing::{NetNode, Node, Route};
 
 use ipnetwork::IpNetwork;
 use std::{
diff --git a/talpid-core/src/routing/mod.rs b/talpid-core/src/routing/mod.rs
index 517e9e7d44..2baf625f2f 100644
--- a/talpid-core/src/routing/mod.rs
+++ b/talpid-core/src/routing/mod.rs
@@ -1,130 +1,18 @@
 #![cfg_attr(target_os = "android", allow(dead_code))]
 #![cfg_attr(target_os = "windows", allow(dead_code))]
-// TODO: remove the allow(dead_code) for android once it's up to scratch.
-use futures::{sync::oneshot, Future};
-use ipnetwork::IpNetwork;
-use std::{collections::HashMap, fmt, net::IpAddr};
-
-#[cfg(target_os = "macos")]
-#[path = "macos.rs"]
-mod imp;
-
-#[cfg(target_os = "linux")]
-#[path = "linux/mod.rs"]
-mod imp;
 
-#[cfg(target_os = "android")]
-#[path = "android.rs"]
-mod imp;
+use ipnetwork::IpNetwork;
+use std::{fmt, net::IpAddr};
 
 #[cfg(target_os = "windows")]
 #[path = "windows.rs"]
 mod imp;
-#[cfg(target_os = "windows")]
-use crate::winnet;
 
-pub use imp::Error as PlatformError;
-
-/// Errors that can be encountered whilst initializing RouteManager
-#[derive(err_derive::Error, Debug)]
-pub enum Error {
-    /// Routing manager thread panicked before starting routing manager
-    #[error(display = "Routing manager thread panicked before starting routing manager")]
-    RoutingManagerThreadPanic,
-    /// Platform sepcific error occured
-    #[error(display = "Failed to create route manager")]
-    FailedToInitializeManager(#[error(source)] imp::Error),
-    /// Failed to spawn route manager future
-    #[error(display = "Failed to spawn route manager on the provided executor")]
-    FailedToSpawnManager,
-}
-
-/// RouteManager applies a set of routes to the route table.
-/// If a destination has to be routed through the default node,
-/// the route will be adjusted dynamically when the default route changes.
-pub struct RouteManager {
-    tx: Option<oneshot::Sender<oneshot::Sender<()>>>,
-    #[cfg(target_os = "windows")]
-    callback_handles: Vec<winnet::WinNetCallbackHandle>,
-}
-
-impl RouteManager {
-    /// Constructs a RouteManager and applies the required routes.
-    /// Takes a map of network destinations and network nodes as an argument, and applies said
-    /// routes.
-    pub fn new(required_routes: HashMap<IpNetwork, NetNode>) -> Result<Self, Error> {
-        let (tx, rx) = oneshot::channel();
-        let (start_tx, start_rx) = oneshot::channel();
-
-        std::thread::spawn(
-            move || match imp::RouteManagerImpl::new(required_routes, rx) {
-                Ok(route_manager) => {
-                    let _ = start_tx.send(Ok(()));
-                    if let Err(e) = route_manager.wait() {
-                        log::error!("Route manager failed - {}", e);
-                    }
-                }
-                Err(e) => {
-                    let _ = start_tx.send(Err(Error::FailedToInitializeManager(e)));
-                }
-            },
-        );
-        match start_rx.wait() {
-            Ok(Ok(())) => Ok(Self {
-                tx: Some(tx),
-                #[cfg(target_os = "windows")]
-                callback_handles: vec![],
-            }),
-            Ok(Err(e)) => Err(e),
-            Err(_) => Err(Error::RoutingManagerThreadPanic),
-        }
-    }
-
-    /// Sets a callback that is called whenever the default route changes.
-    #[cfg(target_os = "windows")]
-    pub fn add_default_route_callback<T: 'static>(
-        &mut self,
-        callback: Option<winnet::DefaultRouteChangedCallback>,
-        context: T,
-    ) {
-        match winnet::add_default_route_change_callback(callback, context) {
-            Err(_e) => {
-                // not sure if this should panic
-                log::error!("Failed to add callback!");
-            }
-            Ok(handle) => {
-                self.callback_handles.push(handle);
-            }
-        }
-    }
-
-    /// Stops RouteManager and removes all of the applied routes.
-    pub fn stop(&mut self) {
-        if let Some(tx) = self.tx.take() {
-            let (wait_tx, wait_rx) = oneshot::channel();
-            if tx.send(wait_tx).is_err() {
-                log::error!("RouteManager already down!");
-                return;
-            }
-
-            if wait_rx.wait().is_err() {
-                log::error!("RouteManager paniced while shutting down");
-            }
-        }
-    }
-}
-
-impl Drop for RouteManager {
-    fn drop(&mut self) {
-        // Ensuring callbacks are removed before the route manager is stopped
-        #[cfg(target_os = "windows")]
-        {
-            self.callback_handles.clear();
-        }
-        self.stop();
-    }
-}
+#[cfg(not(target_os = "windows"))]
+#[path = "unix.rs"]
+mod imp;
 
+pub use imp::{Error, RouteManager};
 
 /// A netowrk route with a specific network node, destinaiton and an optional metric.
 #[derive(Debug, Hash, Eq, PartialEq, Clone)]
diff --git a/talpid-core/src/routing/unix.rs b/talpid-core/src/routing/unix.rs
new file mode 100644
index 0000000000..146c371ceb
--- /dev/null
+++ b/talpid-core/src/routing/unix.rs
@@ -0,0 +1,92 @@
+#![cfg_attr(target_os = "android", allow(dead_code))]
+#![cfg_attr(target_os = "windows", allow(dead_code))]
+// TODO: remove the allow(dead_code) for android once it's up to scratch.
+use super::NetNode;
+use futures::{sync::oneshot, Future};
+use ipnetwork::IpNetwork;
+use std::collections::HashMap;
+
+#[cfg(target_os = "macos")]
+#[path = "macos.rs"]
+mod imp;
+
+#[cfg(target_os = "linux")]
+#[path = "linux/mod.rs"]
+mod imp;
+
+#[cfg(target_os = "android")]
+#[path = "android.rs"]
+mod imp;
+
+pub use imp::Error as PlatformError;
+
+/// Errors that can be encountered whilst initializing RouteManager
+#[derive(err_derive::Error, Debug)]
+pub enum Error {
+    /// Routing manager thread panicked before starting routing manager
+    #[error(display = "Routing manager thread panicked before starting routing manager")]
+    RoutingManagerThreadPanic,
+    /// Platform sepcific error occured
+    #[error(display = "Failed to create route manager")]
+    FailedToInitializeManager(#[error(source)] imp::Error),
+    /// Failed to spawn route manager future
+    #[error(display = "Failed to spawn route manager on the provided executor")]
+    FailedToSpawnManager,
+}
+
+/// RouteManager applies a set of routes to the route table.
+/// If a destination has to be routed through the default node,
+/// the route will be adjusted dynamically when the default route changes.
+pub struct RouteManager {
+    tx: Option<oneshot::Sender<oneshot::Sender<()>>>,
+}
+
+impl RouteManager {
+    /// Constructs a RouteManager and applies the required routes.
+    /// Takes a map of network destinations and network nodes as an argument, and applies said
+    /// routes.
+    pub fn new(required_routes: HashMap<IpNetwork, NetNode>) -> Result<Self, Error> {
+        let (tx, rx) = oneshot::channel();
+        let (start_tx, start_rx) = oneshot::channel();
+
+        std::thread::spawn(
+            move || match imp::RouteManagerImpl::new(required_routes, rx) {
+                Ok(route_manager) => {
+                    let _ = start_tx.send(Ok(()));
+                    if let Err(e) = route_manager.wait() {
+                        log::error!("Route manager failed - {}", e);
+                    }
+                }
+                Err(e) => {
+                    let _ = start_tx.send(Err(Error::FailedToInitializeManager(e)));
+                }
+            },
+        );
+        match start_rx.wait() {
+            Ok(Ok(())) => Ok(Self { tx: Some(tx) }),
+            Ok(Err(e)) => Err(e),
+            Err(_) => Err(Error::RoutingManagerThreadPanic),
+        }
+    }
+
+    /// Stops RouteManager and removes all of the applied routes.
+    pub fn stop(&mut self) {
+        if let Some(tx) = self.tx.take() {
+            let (wait_tx, wait_rx) = oneshot::channel();
+            if tx.send(wait_tx).is_err() {
+                log::error!("RouteManager already down!");
+                return;
+            }
+
+            if wait_rx.wait().is_err() {
+                log::error!("RouteManager paniced while shutting down");
+            }
+        }
+    }
+}
+
+impl Drop for RouteManager {
+    fn drop(&mut self) {
+        self.stop();
+    }
+}
diff --git a/talpid-core/src/routing/windows.rs b/talpid-core/src/routing/windows.rs
index 171442c7fe..e9a031531a 100644
--- a/talpid-core/src/routing/windows.rs
+++ b/talpid-core/src/routing/windows.rs
@@ -1,6 +1,5 @@
 use super::NetNode;
 use crate::winnet;
-use futures::{sync::oneshot, Async, Future};
 use ipnetwork::IpNetwork;
 use std::collections::HashMap;
 
@@ -14,16 +13,16 @@ pub enum Error {
 
 pub type Result<T> = std::result::Result<T, Error>;
 
-pub struct RouteManagerImpl {
-    shutdown_rx: oneshot::Receiver<oneshot::Sender<()>>,
-    is_manager_shut_down: bool,
+/// Manages routes by calling into WinNet
+pub struct RouteManager {
+    callback_handles: Vec<winnet::WinNetCallbackHandle>,
+    is_stopped: bool,
 }
 
-impl RouteManagerImpl {
-    pub fn new(
-        required_routes: HashMap<IpNetwork, NetNode>,
-        shutdown_rx: oneshot::Receiver<oneshot::Sender<()>>,
-    ) -> Result<Self> {
+impl RouteManager {
+    /// Creates a new route manager that will apply the provided routes and ensure they exist until
+    /// it's stopped.
+    pub fn new(required_routes: HashMap<IpNetwork, NetNode>) -> Result<Self> {
         let routes: Vec<_> = required_routes
             .iter()
             .map(|(destination, node)| {
@@ -41,41 +40,47 @@ impl RouteManagerImpl {
             return Err(Error::FailedToStartManager);
         }
 
-
         Ok(Self {
-            shutdown_rx,
-            is_manager_shut_down: false,
+            callback_handles: vec![],
+            is_stopped: false,
         })
     }
 
-    fn shutdown(&mut self) {
-        if !self.is_manager_shut_down {
-            winnet::deactivate_routing_manager();
-            self.is_manager_shut_down = true;
+    /// Sets a callback that is called whenever the default route changes.
+    #[cfg(target_os = "windows")]
+    pub fn add_default_route_callback<T: 'static>(
+        &mut self,
+        callback: Option<winnet::DefaultRouteChangedCallback>,
+        context: T,
+    ) {
+        if self.is_stopped {
+            return;
+        }
+
+        match winnet::add_default_route_change_callback(callback, context) {
+            Err(_e) => {
+                // not sure if this should panic
+                log::error!("Failed to add callback!");
+            }
+            Ok(handle) => {
+                self.callback_handles.push(handle);
+            }
         }
     }
-}
 
-impl Drop for RouteManagerImpl {
-    fn drop(&mut self) {
-        self.shutdown();
+    /// Stops the routing manager and invalidates the route manager - no new default route callbacks
+    /// can be added
+    pub fn stop(&mut self) {
+        if !self.is_stopped {
+            self.callback_handles.clear();
+            winnet::deactivate_routing_manager();
+            self.is_stopped = true;
+        }
     }
 }
 
-impl Future for RouteManagerImpl {
-    type Item = ();
-    type Error = Error;
-    fn poll(&mut self) -> Result<Async<()>> {
-        match self.shutdown_rx.poll() {
-            Ok(Async::Ready(result_tx)) => {
-                self.shutdown();
-                if let Err(_e) = result_tx.send(()) {
-                    log::error!("Receiver already down");
-                }
-                Ok(Async::Ready(()))
-            }
-            Ok(Async::NotReady) => Ok(Async::NotReady),
-            Err(_) => Ok(Async::Ready(())),
-        }
+impl Drop for RouteManager {
+    fn drop(&mut self) {
+        self.stop();
     }
 }