Move RouteManager into SharedTunnelStateValues

5 files changed, 56 insertions, 15 deletions

diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs
index 6881b02311..8e19fe3813 100644
--- a/talpid-core/src/tunnel/mod.rs
+++ b/talpid-core/src/tunnel/mod.rs
@@ -1,5 +1,5 @@
 use self::tun_provider::TunProvider;
-use crate::logging;
+use crate::{logging, routing::RouteManager};
 #[cfg(not(target_os = "android"))]
 use std::collections::HashMap;
 use std::{
@@ -149,6 +149,7 @@ impl TunnelMonitor {
         resource_dir: &Path,
         on_event: L,
         tun_provider: &mut TunProvider,
+        route_manager: &mut RouteManager,
     ) -> Result<Self>
     where
         L: Fn(TunnelEvent) + Send + Clone + Sync + 'static,
@@ -164,9 +165,13 @@ impl TunnelMonitor {
             #[cfg(target_os = "android")]
             TunnelParameters::OpenVpn(_) => Err(Error::UnsupportedPlatform),
 
-            TunnelParameters::Wireguard(config) => {
-                Self::start_wireguard_tunnel(&config, log_file, on_event, tun_provider)
-            }
+            TunnelParameters::Wireguard(config) => Self::start_wireguard_tunnel(
+                &config,
+                log_file,
+                on_event,
+                tun_provider,
+                route_manager,
+            ),
         }
     }
 
@@ -175,6 +180,7 @@ impl TunnelMonitor {
         log: Option<PathBuf>,
         on_event: L,
         tun_provider: &mut TunProvider,
+        route_manager: &mut RouteManager,
     ) -> Result<Self>
     where
         L: Fn(TunnelEvent) + Send + Sync + Clone + 'static,
@@ -185,6 +191,7 @@ impl TunnelMonitor {
             log.as_ref().map(|p| p.as_path()),
             on_event,
             tun_provider,
+            route_manager,
         )?;
         Ok(TunnelMonitor {
             monitor: InternalTunnelMonitor::Wireguard(monitor),
diff --git a/talpid-core/src/tunnel/wireguard/mod.rs b/talpid-core/src/tunnel/wireguard/mod.rs
index d19eb54550..6324d13e80 100644
--- a/talpid-core/src/tunnel/wireguard/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/mod.rs
@@ -46,8 +46,6 @@ pub enum Error {
 pub struct WireguardMonitor {
     /// Tunnel implementation
     tunnel: Arc<Mutex<Option<Box<dyn Tunnel>>>>,
-    /// Route manager
-    route_handle: routing::RouteManager,
     /// Callback to signal tunnel events
     event_callback: Box<dyn Fn(TunnelEvent) + Send + Sync + 'static>,
     close_msg_sender: mpsc::Sender<CloseMsg>,
@@ -62,6 +60,7 @@ impl WireguardMonitor {
         log_path: Option<&Path>,
         on_event: F,
         tun_provider: &mut TunProvider,
+        route_manager: &mut routing::RouteManager,
     ) -> Result<WireguardMonitor> {
         let tunnel = Box::new(WgGoTunnel::start_tunnel(
             &config,
@@ -70,12 +69,12 @@ impl WireguardMonitor {
             Self::get_tunnel_routes(config),
         )?);
         let iface_name = tunnel.get_interface_name().to_string();
-        #[cfg_attr(not(windows), allow(unused_mut))]
-        let mut route_handle = routing::RouteManager::new(Self::get_routes(&iface_name, &config))
+        route_manager
+            .add_routes(Self::get_routes(&iface_name, &config))
             .map_err(Error::SetupRoutingError)?;
 
         #[cfg(target_os = "windows")]
-        route_handle
+        route_manager
             .add_default_route_callback(Some(WgGoTunnel::default_route_changed_callback), ());
 
         let event_callback = Box::new(on_event.clone());
@@ -83,7 +82,6 @@ impl WireguardMonitor {
         let (pinger_tx, pinger_rx) = mpsc::channel();
         let monitor = WireguardMonitor {
             tunnel: Arc::new(Mutex::new(Some(tunnel))),
-            route_handle,
             event_callback,
             close_msg_sender,
             close_msg_receiver,
@@ -144,11 +142,6 @@ impl WireguardMonitor {
 
         let _ = self.pinger_stop_sender.send(());
 
-        // Clear routes manually - otherwise there will be some log spam since the tunnel device
-        // can be removed before the routes are cleared, which automatically clears some of the
-        // routes that were set.
-        self.route_handle.stop();
-
         self.stop_tunnel();
 
         (self.event_callback)(TunnelEvent::Down);
diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs
index 47bc8d0f36..dab6e37619 100644
--- a/talpid-core/src/tunnel_state_machine/connected_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connected_state.rs
@@ -89,12 +89,23 @@ impl ConnectedState {
         }
     }
 
+    fn reset_routes(shared_values: &mut SharedTunnelStateValues) {
+        if let Err(error) = shared_values.route_manager.clear_routes() {
+            log::error!(
+                "Failed to clear routes: {:?}",
+                error.display_chain_with_msg("Failed to clear routes")
+            );
+        }
+    }
+
     fn disconnect(
         self,
         shared_values: &mut SharedTunnelStateValues,
         after_disconnect: AfterDisconnect,
     ) -> EventConsequence<Self> {
         Self::reset_dns(shared_values);
+        Self::reset_routes(shared_values);
+
         EventConsequence::NewState(DisconnectingState::enter(
             shared_values,
             (self.close_handle, self.tunnel_close_event, after_disconnect),
@@ -185,6 +196,7 @@ impl ConnectedState {
         match poll_result {
             Ok(Async::Ready(block_reason)) => {
                 if let Some(reason) = block_reason {
+                    Self::reset_routes(shared_values);
                     return NewState(ErrorState::enter(shared_values, reason));
                 }
             }
@@ -194,6 +206,7 @@ impl ConnectedState {
 
         log::info!("Tunnel closed. Reconnecting.");
         Self::reset_dns(shared_values);
+        Self::reset_routes(shared_values);
         NewState(ConnectingState::enter(shared_values, 0))
     }
 }
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 14cd2cc51d..4bd1345339 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -5,6 +5,7 @@ use super::{
 };
 use crate::{
     firewall::FirewallPolicy,
+    routing::RouteManager,
     tunnel::{
         self, tun_provider::TunProvider, CloseHandle, TunnelEvent, TunnelMetadata, TunnelMonitor,
     },
@@ -68,18 +69,21 @@ impl ConnectingState {
         log_dir: &Option<PathBuf>,
         resource_dir: &Path,
         tun_provider: &mut TunProvider,
+        route_manager: &mut RouteManager,
         retry_attempt: u32,
     ) -> crate::tunnel::Result<Self> {
         let (event_tx, event_rx) = mpsc::unbounded();
         let on_tunnel_event = move |event| {
             let _ = event_tx.unbounded_send(event);
         };
+
         let monitor = TunnelMonitor::start(
             &parameters,
             log_dir,
             resource_dir,
             on_tunnel_event,
             tun_provider,
+            route_manager,
         )?;
         let close_handle = Some(monitor.close_handle());
         let tunnel_close_event = Self::spawn_tunnel_monitor_wait_thread(monitor);
@@ -165,11 +169,22 @@ impl ConnectingState {
         }
     }
 
+    fn reset_routes(shared_values: &mut SharedTunnelStateValues) {
+        if let Err(error) = shared_values.route_manager.clear_routes() {
+            log::error!(
+                "Failed to clear routes: {:?}",
+                error.display_chain_with_msg("Failed to clear routes")
+            );
+        }
+    }
+
     fn disconnect(
         self,
         shared_values: &mut SharedTunnelStateValues,
         after_disconnect: AfterDisconnect,
     ) -> EventConsequence<Self> {
+        Self::reset_routes(shared_values);
+
         EventConsequence::NewState(DisconnectingState::enter(
             shared_values,
             (self.close_handle, self.tunnel_close_event, after_disconnect),
@@ -270,6 +285,7 @@ impl ConnectingState {
         match poll_result {
             Ok(Async::Ready(block_reason)) => {
                 if let Some(reason) = block_reason {
+                    Self::reset_routes(shared_values);
                     return EventConsequence::NewState(ErrorState::enter(shared_values, reason));
                 }
             }
@@ -281,6 +297,7 @@ impl ConnectingState {
             "Tunnel closed. Reconnecting, attempt {}.",
             self.retry_attempt + 1
         );
+        Self::reset_routes(shared_values);
         EventConsequence::NewState(ConnectingState::enter(
             shared_values,
             self.retry_attempt + 1,
@@ -359,6 +376,7 @@ impl TunnelState for ConnectingState {
                         &shared_values.log_dir,
                         &shared_values.resource_dir,
                         &mut shared_values.tun_provider,
+                        &mut shared_values.route_manager,
                         retry_attempt,
                     ) {
                         Ok(connecting_state) => {
diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs
index baf52c4b2b..2ffce2bec9 100644
--- a/talpid-core/src/tunnel_state_machine/mod.rs
+++ b/talpid-core/src/tunnel_state_machine/mod.rs
@@ -19,6 +19,7 @@ use crate::{
     firewall::{Firewall, FirewallArguments},
     mpsc::Sender,
     offline,
+    routing::RouteManager,
     tunnel::tun_provider::TunProvider,
 };
 
@@ -27,6 +28,7 @@ use futures01::{
     Async, Future, Poll, Stream,
 };
 use std::{
+    collections::HashSet,
     io,
     path::{Path, PathBuf},
     sync::{mpsc as sync_mpsc, Arc},
@@ -56,6 +58,10 @@ pub enum Error {
     #[error(display = "Failed to initialize the system DNS manager and monitor")]
     InitDnsMonitorError(#[error(source)] crate::dns::Error),
 
+    /// Failed to initialize the route manager.
+    #[error(display = "Failed to initialize the route manager")]
+    InitRouteManagerError(#[error(source)] crate::routing::Error),
+
     /// Failed to initialize tunnel state machine event loop executor
     #[error(display = "Failed to initialize tunnel state machine event loop executor")]
     ReactorError(#[error(source)] io::Error),
@@ -231,9 +237,12 @@ impl TunnelStateMachine {
         };
         let firewall = Firewall::new(args).map_err(Error::InitFirewallError)?;
         let dns_monitor = DnsMonitor::new(cache_dir).map_err(Error::InitDnsMonitorError)?;
+        let route_manager =
+            RouteManager::new(HashSet::new()).map_err(Error::InitRouteManagerError)?;
         let mut shared_values = SharedTunnelStateValues {
             firewall,
             dns_monitor,
+            route_manager,
             allow_lan,
             block_when_disconnected,
             is_offline,
@@ -317,6 +326,7 @@ pub trait TunnelParametersGenerator: Send + 'static {
 struct SharedTunnelStateValues {
     firewall: Firewall,
     dns_monitor: DnsMonitor,
+    route_manager: RouteManager,
     /// Should LAN access be allowed outside the tunnel.
     allow_lan: bool,
     /// Should network access be allowed when in the disconnected state.