Merge branch 'wg-improve-connecting-state'

4 files changed, 34 insertions, 62 deletions

diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs
index ddfe0139c8..cc7bcdb325 100644
--- a/talpid-core/src/tunnel/mod.rs
+++ b/talpid-core/src/tunnel/mod.rs
@@ -137,7 +137,7 @@ impl TunnelMonitor {
         on_event: L,
     ) -> Result<Self>
     where
-        L: Fn(TunnelEvent) + Send + Sync + 'static,
+        L: Fn(TunnelEvent) + Send + Clone + Sync + 'static,
     {
         Self::ensure_ipv6_can_be_used_if_enabled(&tunnel_parameters.get_generic_options())?;
         let log_file = Self::prepare_tunnel_log_file(&tunnel_parameters, log_dir)?;
@@ -162,7 +162,7 @@ impl TunnelMonitor {
         on_event: L,
     ) -> Result<Self>
     where
-        L: Fn(TunnelEvent) + Send + Sync + 'static,
+        L: Fn(TunnelEvent) + Send + Sync + Clone + 'static,
     {
         let config = wireguard::config::Config::from_parameters(&params)
             .chain_err(|| ErrorKind::WireguardConfigError)?;
diff --git a/talpid-core/src/tunnel/wireguard/mod.rs b/talpid-core/src/tunnel/wireguard/mod.rs
index 5bcaff847f..f183cc36e0 100644
--- a/talpid-core/src/tunnel/wireguard/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/mod.rs
@@ -2,7 +2,7 @@
 use self::config::Config;
 use super::{TunnelEvent, TunnelMetadata};
 use crate::routing;
-use std::{net::IpAddr, path::Path, sync::mpsc};
+use std::{path::Path, sync::mpsc};
 
 pub mod config;
 mod ping_monitor;
@@ -59,14 +59,14 @@ pub struct WireguardMonitor {
 }
 
 impl WireguardMonitor {
-    pub fn start<F: Fn(TunnelEvent) + Send + Sync + 'static>(
+    pub fn start<F: Fn(TunnelEvent) + Send + Sync + Clone + 'static>(
         config: &Config,
         log_path: Option<&Path>,
         on_event: F,
     ) -> Result<WireguardMonitor> {
         let tunnel = Box::new(WgGoTunnel::start_tunnel(&config, log_path)?);
         let router = routing::RouteManager::new().chain_err(|| ErrorKind::SetupRoutingError)?;
-        let event_callback = Box::new(on_event);
+        let event_callback = Box::new(on_event.clone());
         let (close_msg_sender, close_msg_receiver) = mpsc::channel();
         let mut monitor = WireguardMonitor {
             tunnel,
@@ -76,16 +76,29 @@ impl WireguardMonitor {
             close_msg_receiver,
         };
         monitor.setup_routing(&config)?;
-        monitor.tunnel_up(&config);
 
-        ping_monitor::ping(
-            config.ipv4_gateway.into(),
-            PING_TIMEOUT,
-            &monitor.tunnel.get_interface_name().to_string(),
-            true,
-        )
-        .chain_err(|| ErrorKind::PingTimeoutError)?;
-        monitor.start_pinger(&config);
+        let metadata = monitor.tunnel_metadata(&config);
+        let iface_name = monitor.tunnel.get_interface_name().to_string();
+        let gateway = config.ipv4_gateway.into();
+        let close_sender = monitor.close_msg_sender.clone();
+
+        ::std::thread::spawn(move || {
+            match ping_monitor::ping(gateway, PING_TIMEOUT, &iface_name, true) {
+                Ok(()) => {
+                    (on_event)(TunnelEvent::Up(metadata));
+                }
+                Err(e) => {
+                    log::error!("First ping to gateway failed - {}", e);
+                    let _ = close_sender.send(CloseMsg::PingErr);
+                }
+            };
+
+            if let Err(e) = ping_monitor::monitor_ping(gateway, PING_TIMEOUT, &iface_name) {
+                log::trace!("Ping monitor failed - {}", e);
+            }
+            let _ = close_sender.send(CloseMsg::PingErr);
+        });
+
 
         Ok(monitor)
     }
@@ -153,28 +166,14 @@ impl WireguardMonitor {
             .chain_err(|| ErrorKind::SetupRoutingError)
     }
 
-    fn start_pinger(&self, config: &Config) {
-        let close_sender = self.close_msg_sender.clone();
-
-        ping_monitor::spawn_ping_monitor(
-            IpAddr::from(config.ipv4_gateway),
-            PING_TIMEOUT,
-            self.tunnel.get_interface_name().to_string(),
-            move || {
-                let _ = close_sender.send(CloseMsg::PingErr);
-            },
-        )
-    }
-
-    fn tunnel_up(&self, config: &Config) {
+    fn tunnel_metadata(&self, config: &Config) -> TunnelMetadata {
         let interface_name = self.tunnel.get_interface_name();
-        let metadata = TunnelMetadata {
+        TunnelMetadata {
             interface: interface_name.to_string(),
             ips: config.tunnel.addresses.clone(),
             ipv4_gateway: config.ipv4_gateway,
             ipv6_gateway: config.ipv6_gateway,
-        };
-        (self.event_callback)(TunnelEvent::Up(metadata));
+        }
     }
 }
 
diff --git a/talpid-core/src/tunnel/wireguard/ping_monitor.rs b/talpid-core/src/tunnel/wireguard/ping_monitor.rs
index 3c6f4525e8..d471971ca6 100644
--- a/talpid-core/src/tunnel/wireguard/ping_monitor.rs
+++ b/talpid-core/src/tunnel/wireguard/ping_monitor.rs
@@ -12,25 +12,16 @@ error_chain! {
     }
 }
 
-pub fn spawn_ping_monitor<F: FnOnce() + Send + 'static>(
-    ip: IpAddr,
-    timeout_secs: u16,
-    interface: String,
-    on_fail: F,
-) {
-    thread::spawn(move || loop {
+pub fn monitor_ping(ip: IpAddr, timeout_secs: u16, interface: &str) -> Result<()> {
+    loop {
         let start = time::Instant::now();
-        if let Err(e) = ping(ip, timeout_secs, &interface, false) {
-            log::debug!("ping failed - {}", e);
-            on_fail();
-            return;
-        }
+        ping(ip, timeout_secs, &interface, false)?;
         if let Some(remaining) =
             time::Duration::from_secs(timeout_secs.into()).checked_sub(start.elapsed())
         {
             thread::sleep(remaining);
         }
-    });
+    }
 }
 
 pub fn ping(
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 78c6deee6b..77c8e41f75 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -346,26 +346,8 @@ impl TunnelState for ConnectingState {
                             log::error!("Failed to start tunnel: {}", error);
                             let block_reason = match *error.kind() {
                                 tunnel::ErrorKind::EnableIpv6Error => BlockReason::Ipv6Unavailable,
-
-                                #[cfg(unix)]
-                                tunnel::ErrorKind::WirguardTunnelMonitoringError(ref err) => {
-                                    match &err {
-                                        tunnel::wireguard::ErrorKind::PingTimeoutError => {
-                                            if crate::offline::is_offline() {
-                                                BlockReason::IsOffline
-                                            } else {
-                                                BlockReason::StartTunnelError
-                                            }
-                                        }
-                                        _ => BlockReason::StartTunnelError,
-                                    }
-                                }
                                 _ => BlockReason::StartTunnelError,
                             };
-
-                            let chained_error = error.chain_err(|| "Failed to start tunnel");
-                            error!("{}", chained_error.display_chain());
-
                             BlockedState::enter(shared_values, block_reason)
                         }
                     }