Remove notify dependency and add inotify instead

Switch from notify to inotify since we only used notify on Linux.
This helps cleanup our dependency tree and allows us to have more
control over how the thread responsible for monitoring `resolv.conf`
exits.

3 files changed, 70 insertions, 45 deletions

diff --git a/talpid-core/Cargo.toml b/talpid-core/Cargo.toml
index e6ed69fa6a..96d383325c 100644
--- a/talpid-core/Cargo.toml
+++ b/talpid-core/Cargo.toml
@@ -54,7 +54,7 @@ jnix = { version = "0.4", features = ["derive"] }
 
 
 [target.'cfg(target_os = "linux")'.dependencies]
-notify = "4.0"
+inotify = "0.10"
 resolv-conf = "0.7"
 rtnetlink = "0.8"
 netlink-packet-core = "0.2"
diff --git a/talpid-core/src/dns/linux/mod.rs b/talpid-core/src/dns/linux/mod.rs
index 5a4bfdb5da..3eb11df51a 100644
--- a/talpid-core/src/dns/linux/mod.rs
+++ b/talpid-core/src/dns/linux/mod.rs
@@ -10,8 +10,6 @@ use self::{
 use crate::routing::RouteManagerHandle;
 use std::{env, fmt, net::IpAddr};
 
-const RESOLV_CONF_PATH: &str = "/etc/resolv.conf";
-
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// Errors that can happen in the Linux DNS monitor
@@ -58,7 +56,7 @@ impl super::DnsMonitorT for DnsMonitor {
     fn set(&mut self, interface: &str, servers: &[IpAddr]) -> Result<()> {
         self.reset()?;
         // Creating a new DNS monitor for each set, in case the system changed how it manages DNS.
-        let mut inner = DnsMonitorHolder::new()?;
+        let mut inner = DnsMonitorHolder::new(&self.handle)?;
         if !servers.is_empty() {
             inner.set(&self.handle, &self.route_manager, interface, servers)?;
             self.inner = Some(inner);
@@ -95,21 +93,23 @@ impl fmt::Display for DnsMonitorHolder {
 }
 
 impl DnsMonitorHolder {
-    fn new() -> Result<Self> {
+    fn new(handle: &tokio::runtime::Handle) -> Result<Self> {
         let dns_module = env::var_os("TALPID_DNS_MODULE");
 
         let manager = match dns_module.as_ref().and_then(|value| value.to_str()) {
-            Some("static-file") => DnsMonitorHolder::StaticResolvConf(StaticResolvConf::new()?),
+            Some("static-file") => {
+                DnsMonitorHolder::StaticResolvConf(handle.block_on(StaticResolvConf::new())?)
+            }
             Some("resolvconf") => DnsMonitorHolder::Resolvconf(Resolvconf::new()?),
             Some("systemd") => DnsMonitorHolder::SystemdResolved(SystemdResolved::new()?),
             Some("network-manager") => DnsMonitorHolder::NetworkManager(NetworkManager::new()?),
-            Some(_) | None => Self::with_detected_dns_manager()?,
+            Some(_) | None => Self::with_detected_dns_manager(handle)?,
         };
         log::debug!("Managing DNS via {}", manager);
         Ok(manager)
     }
 
-    fn with_detected_dns_manager() -> Result<Self> {
+    fn with_detected_dns_manager(handle: &tokio::runtime::Handle) -> Result<Self> {
         SystemdResolved::new()
             .map(DnsMonitorHolder::SystemdResolved)
             .or_else(|err| {
@@ -124,7 +124,11 @@ impl DnsMonitorHolder {
                 NetworkManager::new().map(DnsMonitorHolder::NetworkManager)
             })
             .or_else(|_| Resolvconf::new().map(DnsMonitorHolder::Resolvconf))
-            .or_else(|_| StaticResolvConf::new().map(DnsMonitorHolder::StaticResolvConf))
+            .or_else(|_| {
+                handle
+                    .block_on(StaticResolvConf::new())
+                    .map(DnsMonitorHolder::StaticResolvConf)
+            })
             .map_err(|_| Error::NoDnsMonitor)
     }
 
diff --git a/talpid-core/src/dns/linux/static_resolv_conf.rs b/talpid-core/src/dns/linux/static_resolv_conf.rs
index 691d7b468b..613a995db2 100644
--- a/talpid-core/src/dns/linux/static_resolv_conf.rs
+++ b/talpid-core/src/dns/linux/static_resolv_conf.rs
@@ -1,25 +1,20 @@
-use super::RESOLV_CONF_PATH;
-use notify::{RecommendedWatcher, RecursiveMode, Watcher};
+use futures::StreamExt;
+use inotify::{Inotify, WatchMask};
 use parking_lot::Mutex;
 use resolv_conf::{Config, ScopedIp};
-use std::{
-    fs, io,
-    net::IpAddr,
-    path::Path,
-    sync::{mpsc, Arc},
-    thread,
-};
+use std::{fs, io, net::IpAddr, sync::Arc};
 use talpid_types::ErrorExt;
+use triggered::{trigger, Listener, Trigger};
 
 const RESOLV_CONF_BACKUP_PATH: &str = "/etc/resolv.conf.mullvadbackup";
-const RESOLV_CONF_DIR: &str = "/etc/";
+const RESOLV_CONF_PATH: &str = "/etc/resolv.conf";
 
 pub type Result<T> = std::result::Result<T, Error>;
 
 #[derive(err_derive::Error, Debug)]
 pub enum Error {
     #[error(display = "Failed to watch /etc/resolv.conf for changes")]
-    WatchResolvConf(#[error(source)] notify::Error),
+    WatchResolvConf(#[error(source)] std::io::Error),
 
     #[error(display = "Failed to write to {}", _0)]
     WriteResolvConf(&'static str, #[error(source)] io::Error),
@@ -40,11 +35,11 @@ pub struct StaticResolvConf {
 }
 
 impl StaticResolvConf {
-    pub fn new() -> Result<Self> {
+    pub async fn new() -> Result<Self> {
         restore_from_backup()?;
 
         let state = Arc::new(Mutex::new(None));
-        let watcher = DnsWatcher::start(state.clone())?;
+        let watcher = DnsWatcher::start(state.clone()).await?;
 
         Ok(StaticResolvConf {
             state,
@@ -107,39 +102,65 @@ impl State {
 }
 
 struct DnsWatcher {
-    _watcher: RecommendedWatcher,
+    cancel_trigger: Trigger,
+}
+
+impl Drop for DnsWatcher {
+    fn drop(&mut self) {
+        self.cancel_trigger.trigger();
+    }
 }
 
 impl DnsWatcher {
-    fn start(state: Arc<Mutex<Option<State>>>) -> Result<Self> {
-        let (event_tx, event_rx) = mpsc::channel();
-        let mut watcher = notify::raw_watcher(event_tx).map_err(Error::WatchResolvConf)?;
+    async fn start(state: Arc<Mutex<Option<State>>>) -> Result<Self> {
+        let mut watcher = Inotify::init().map_err(Error::WatchResolvConf)?;
+        let mut mask = WatchMask::empty();
+        // Documentation for the meaning of these masks can be found in `man inotify`
+        //
+        // We do not watch for writes but instead for when a file opened for writing is closed.
+        // This way we don't have collisions.
+        mask.insert(WatchMask::CLOSE_WRITE);
+        // DELETE_SELF is generated if the file watched is itself deleted
+        mask.insert(WatchMask::DELETE_SELF);
+        mask.insert(WatchMask::MOVE_SELF);
 
         watcher
-            .watch(&RESOLV_CONF_DIR, RecursiveMode::NonRecursive)
+            .add_watch(&RESOLV_CONF_PATH, mask)
             .map_err(Error::WatchResolvConf)?;
 
-        thread::spawn(move || Self::event_loop(event_rx, &state));
+        let (cancel_trigger, cancel_listener) = trigger();
 
-        Ok(DnsWatcher { _watcher: watcher })
+        tokio::spawn(async move { Self::event_loop(watcher, cancel_listener, &state).await });
+
+        Ok(DnsWatcher { cancel_trigger })
     }
 
-    fn event_loop(events: mpsc::Receiver<notify::RawEvent>, state: &Arc<Mutex<Option<State>>>) {
-        for event in events {
-            if event
-                .path
-                .as_ref()
-                .map(|p| p.as_path() == AsRef::<Path>::as_ref(RESOLV_CONF_PATH))
-                .unwrap_or(false)
-            {
-                let mut locked_state = state.lock();
-                if let Err(error) = Self::update(locked_state.as_mut()) {
-                    log::error!(
-                        "{}",
-                        error.display_chain_with_msg(
-                            "Failed to update DNS state after DNS settings changed"
-                        )
-                    );
+    async fn event_loop(
+        mut watcher: Inotify,
+        mut cancel_listener: Listener,
+        state: &Arc<Mutex<Option<State>>>,
+    ) {
+        const EVENT_BUFFER_SIZE: usize = 1024;
+        let mut buffer = [0; EVENT_BUFFER_SIZE];
+        let mut events = watcher
+            .event_stream(&mut buffer)
+            .expect("Could not read events for resolv.conf");
+
+        loop {
+            tokio::select! {
+                _ = &mut cancel_listener => {
+                    break;
+                },
+                Some(_) = events.next() => {
+                    let mut locked_state = state.lock();
+                    if let Err(error) = Self::update(locked_state.as_mut()) {
+                        log::error!(
+                            "{}",
+                            error.display_chain_with_msg(
+                                "Failed to update DNS state after DNS settings changed"
+                            )
+                        );
+                    }
                 }
             }
         }