diff options
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | mullvad-daemon/src/lib.rs | 38 | ||||
| -rw-r--r-- | talpid-core/src/tunnel_state_machine/mod.rs | 25 |
3 files changed, 57 insertions, 10 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e89e1b8b..90f82e056c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ Line wrap the file at 100 chars. Th ### Changed - Prefer WireGuard when tunnel protocol is set to _auto_ on Linux and MacOS. +- Wait for tunnel state machine to properly shut down, cleaning up the firewall properly on Windows + during the daemon shutdown. #### Android - Allow other apps to request the VPN tunnel to connect or disconnect. @@ -46,7 +48,7 @@ Line wrap the file at 100 chars. Th - Show when the app failed to block all connections after an error in the android and desktop apps. #### macOS -- Fix firewall rules to properly handle DNS requests over TCP when "Local network sharing" is +- Fix firewall rules to properly handle DNS requests over TCP when "Local network sharing" is disabled. Previously DNS requests over TCP would timeout. #### Android diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 3f74e82df7..74383b43a4 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -73,6 +73,8 @@ mod wireguard; /// Remove this and use the one in jsonrpc_core when that is released. type BoxFuture<T, E> = Box<dyn Future<Item = T, Error = E> + Send>; +const TUNNEL_STATE_MACHINE_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); + #[derive(err_derive::Error, Debug)] #[error(no_from)] pub enum Error { @@ -434,6 +436,8 @@ pub struct Daemon<L: EventListener> { last_generated_bridge_relay: Option<Relay>, app_version_info: AppVersionInfo, shutdown_callbacks: Vec<Box<dyn FnOnce()>>, + /// oneshot channel that completes once the tunnel state machine has been shut down + tunnel_state_machine_shutdown_signal: oneshot::Receiver<()>, } impl<L> Daemon<L> @@ -449,6 +453,8 @@ where #[cfg(target_os = "android")] android_context: AndroidContext, ) -> Result<Self, Error> { let ca_path = resource_dir.join(mullvad_paths::resources::API_CA_FILENAME); + let (tunnel_state_machine_shutdown_tx, tunnel_state_machine_shutdown_signal) = + oneshot::channel(); let mut rpc_manager = mullvad_rpc::MullvadRpcFactory::with_cache_dir(&cache_dir, &ca_path); @@ -511,6 +517,7 @@ where resource_dir, cache_dir, internal_event_tx.to_specialized_sender(), + tunnel_state_machine_shutdown_tx, #[cfg(target_os = "android")] android_context, ) @@ -546,6 +553,7 @@ where last_generated_bridge_relay: None, app_version_info, shutdown_callbacks: vec![], + tunnel_state_machine_shutdown_signal, }; daemon.ensure_wireguard_keys_for_current_account(); @@ -586,22 +594,46 @@ where } fn finalize(self) { - let (event_listener, shutdown_callbacks) = self.shutdown(); + let (event_listener, shutdown_callbacks, tunnel_state_machine_shutdown_signal) = + self.shutdown(); for cb in shutdown_callbacks { cb(); } + + let state_machine_shutdown = tokio_timer::Timer::default().timeout( + // the oneshot::Canceled error type does not play well with the timer error, as such, + // it has to be cast away. + tunnel_state_machine_shutdown_signal.map_err(|_| { + log::error!("Tunnel state machine already shut down"); + }), + TUNNEL_STATE_MACHINE_SHUTDOWN_TIMEOUT, + ); + + match state_machine_shutdown.wait() { + Ok(_) => { + log::info!("Tunnel state machine shut down"); + } + Err(_) => { + log::error!("Tunnel state machine did not shut down in time, shutting down anyway"); + } + } mem::drop(event_listener); } /// Shuts down the daemon without shutting down the underlying event listener and the shutdown /// callbacks - fn shutdown(self) -> (L, Vec<Box<dyn FnOnce()>>) { + fn shutdown(self) -> (L, Vec<Box<dyn FnOnce()>>, oneshot::Receiver<()>) { let Daemon { event_listener, shutdown_callbacks, + tunnel_state_machine_shutdown_signal, .. } = self; - (event_listener, shutdown_callbacks) + ( + event_listener, + shutdown_callbacks, + tunnel_state_machine_shutdown_signal, + ) } diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs index 773a14e8da..5379a76740 100644 --- a/talpid-core/src/tunnel_state_machine/mod.rs +++ b/talpid-core/src/tunnel_state_machine/mod.rs @@ -21,7 +21,10 @@ use crate::{ offline, tunnel::tun_provider::TunProvider, }; -use futures::{sync::mpsc, Async, Future, Poll, Stream}; +use futures::{ + sync::{mpsc, oneshot}, + Async, Future, Poll, Stream, +}; use std::{ io, path::{Path, PathBuf}, @@ -70,6 +73,7 @@ pub fn spawn( resource_dir: PathBuf, cache_dir: impl AsRef<Path> + Send + 'static, state_change_listener: impl Sender<TunnelStateTransition> + Send + 'static, + shutdown_tx: oneshot::Sender<()>, #[cfg(target_os = "android")] android_context: AndroidContext, ) -> Result<Arc<mpsc::UnboundedSender<TunnelCommand>>, Error> { let (command_tx, command_rx) = mpsc::unbounded(); @@ -102,6 +106,7 @@ pub fn spawn( cache_dir, command_rx, state_change_listener, + shutdown_tx, ) { Ok((mut reactor, event_loop)) => { startup_result_tx.send(Ok(())).expect( @@ -141,6 +146,7 @@ fn create_event_loop( cache_dir: impl AsRef<Path>, commands: mpsc::UnboundedReceiver<TunnelCommand>, state_change_listener: impl Sender<TunnelStateTransition>, + shutdown_tx: oneshot::Sender<()>, ) -> Result<(Core, impl Future<Item = (), Error = Error>), Error> { let reactor = Core::new().map_err(Error::ReactorError)?; let state_machine = TunnelStateMachine::new( @@ -155,11 +161,18 @@ fn create_event_loop( commands, )?; - let future = state_machine.for_each(move |state_change_event| { - state_change_listener - .send(state_change_event) - .map_err(|_| Error::SendStateChange) - }); + let future = state_machine + .for_each(move |state_change_event| { + state_change_listener + .send(state_change_event) + .map_err(|_| Error::SendStateChange) + }) + .then(move |_| { + if shutdown_tx.send(()).is_err() { + log::error!("Can't send shutdown completion to daemon"); + } + Ok(()) + }); Ok((reactor, future)) } |