Create an inotify based universal notifier for Linux

Recall that universal notifiers are used to report changes to universal
variables to other shell instances. This adds a new strategy based on using
inotify to directly monitor the universal variables
file.

We have tried this in the past and abandoned it because it doesn't properly
work on some CI systems - let's try again.

Cargo.toml

@@ -51,7 +51,7 @@ lazy_static = "1.4.0"
 libc = "0.2.137"
 lru = "0.10.0"
 moveit = "0.5.1"
-nix = { version = "0.25.0", default-features = false, features = [] }
+nix = { version = "0.25.0", default-features = false, features = ["inotify"] }
 num-traits = "0.2.15"
 # to make integer->enum conversion easier
 num-derive = "0.3.3"

fish-rust/src/env_universal_common.rs

@@ -832,7 +832,7 @@ impl EnvUniversal {
 }
 
 /// \return the default variable path, or an empty string on failure.
-fn default_vars_path() -> WString {
+pub fn default_vars_path() -> WString {
     if let Some(mut path) = default_vars_path_directory() {
         path.push_str("/fish_variables");
         return path;

fish-rust/src/universal_notifier/inotify.rs

@@ -0,0 +1,94 @@
+use crate::common::wcs2osstring;
+use crate::env_universal_common::default_vars_path;
+use crate::universal_notifier::UniversalNotifier;
+use crate::wchar::prelude::*;
+use crate::wutil::{wbasename, wdirname};
+use nix::sys::inotify::{AddWatchFlags, InitFlags, Inotify};
+use std::ffi::OsString;
+use std::os::fd::{AsRawFd, RawFd};
+
+/// A notifier based on inotify.
+pub struct InotifyNotifier {
+    // The inotify instance.
+    inotify: Inotify,
+    // The basename of the file to watch.
+    basename: OsString,
+}
+
+impl InotifyNotifier {
+    /// Create a notifier at the default fish_variables path.
+    pub fn new() -> Option<Self> {
+        Self::new_at(&default_vars_path())
+    }
+
+    /// Create a notifier at a given path.
+    /// The path should be the full path to the fish_variables file.
+    /// InotifyNotifier will watch the parent directory for changes to that file.
+    /// It should not watch for modifications to the file itself, because uvars are atomically
+    /// swapped into place.
+    pub fn new_at(path: &wstr) -> Option<Self> {
+        let dirname = wdirname(path);
+        let basename = wbasename(path);
+        let inotify = Inotify::init(InitFlags::IN_CLOEXEC | InitFlags::IN_NONBLOCK).ok()?;
+        inotify
+            .add_watch(
+                wcs2osstring(dirname).as_os_str(),
+                AddWatchFlags::IN_MODIFY | AddWatchFlags::IN_MOVED_TO,
+            )
+            .ok()?;
+        Some(InotifyNotifier {
+            inotify,
+            basename: wcs2osstring(basename),
+        })
+    }
+}
+
+impl UniversalNotifier for InotifyNotifier {
+    // Do nothing to trigger a notification.
+    // The notifications are generated from changes to the file itself.
+    fn post_notification(&self) {}
+
+    // Returns the fd from which to watch for events.
+    fn notification_fd(&self) -> Option<RawFd> {
+        Some(self.inotify.as_raw_fd())
+    }
+
+    // The notification_fd is readable; drain it. Returns true if a notification is considered to
+    // have been posted.
+    fn notification_fd_became_readable(&self, fd: RawFd) -> bool {
+        assert_eq!(fd, self.inotify.as_raw_fd(), "unexpected fd");
+        let Ok(evts) = self.inotify.read_events() else {
+            return false;
+        };
+        evts.iter()
+            .any(|evt| evt.name.as_ref() == Some(&self.basename))
+    }
+}
+
+#[test]
+fn test_inotify_notifiers() {
+    use crate::common::{charptr2wcstring, wcs2osstring};
+    use std::fs::remove_dir_all;
+    use std::path::PathBuf;
+
+    let template = std::ffi::CString::new("/tmp/fish_inotify_XXXXXX").unwrap();
+    let temp_dir_ptr = unsafe { libc::mkdtemp(template.into_raw() as *mut libc::c_char) };
+    if temp_dir_ptr.is_null() {
+        panic!("failed to create temp dir");
+    }
+    let fake_uvars_dir = charptr2wcstring(temp_dir_ptr);
+    let fake_uvars_path = fake_uvars_dir.clone() + "/fish_variables";
+
+    let mut notifiers = Vec::new();
+    for _ in 0..16 {
+        notifiers
+            .push(InotifyNotifier::new_at(&fake_uvars_path).expect("failed to create notifier"));
+    }
+    let notifiers = notifiers
+        .iter()
+        .map(|n| n as &dyn UniversalNotifier)
+        .collect::<Vec<_>>();
+    super::test_notifiers(&notifiers, Some(&fake_uvars_path));
+
+    let _ = remove_dir_all(PathBuf::from(wcs2osstring(&fake_uvars_dir)));
+}

fish-rust/src/universal_notifier/mod.rs

@@ -4,6 +4,9 @@ use std::os::fd::RawFd;
 #[cfg(target_os = "macos")]
 mod notifyd;
 
+#[cfg(any(target_os = "android", target_os = "linux"))]
+mod inotify;
+
 /// The "universal notifier" is an object responsible for broadcasting and receiving universal
 /// variable change notifications. These notifications do not contain the change, but merely
 /// indicate that the uvar file has changed. It is up to the uvar subsystem to re-read the file.
@@ -50,6 +53,12 @@ pub fn create_notifier() -> Box<dyn UniversalNotifier> {
             return Box::new(notifier);
         }
     }
+    #[cfg(any(target_os = "android", target_os = "linux"))]
+    {
+        if let Some(notifier) = inotify::InotifyNotifier::new() {
+            return Box::new(notifier);
+        }
+    }
     Box::new(NullNotifier)
 }
 
@@ -60,9 +69,13 @@ pub fn default_notifier() -> &'static dyn UniversalNotifier {
     DEFAULT_NOTIFIER.get_or_init(create_notifier).as_ref()
 }
 
-// Test a slice of notifiers.
 #[cfg(test)]
-pub fn test_notifiers(notifiers: &[&dyn UniversalNotifier]) {
+use crate::wchar::prelude::*;
+
+// Test a slice of notifiers.
+// fish_variables_path is the path to the (simulated) fish_variables file.
+#[cfg(test)]
+pub fn test_notifiers(notifiers: &[&dyn UniversalNotifier], fish_variables_path: Option<&wstr>) {
     let poll_notifier = |n: &dyn UniversalNotifier| -> bool {
         let Some(fd) = n.notification_fd() else {
             return false;
@@ -74,6 +87,22 @@ pub fn test_notifiers(notifiers: &[&dyn UniversalNotifier]) {
         }
     };
 
+    // Helper to simulate modifying a file, using the atomic rename() approach.
+    let modify_path = |path: &wstr| -> Result<(), std::io::Error> {
+        use crate::common::wcs2osstring;
+        use std::fs;
+        use std::io::Write;
+        let path = wcs2osstring(path);
+        let mut new_path = std::path::PathBuf::from(path.clone());
+        new_path.set_extension("new");
+
+        let mut file = fs::File::create(&new_path)?;
+        file.write_all(b"Random text")?;
+        std::mem::drop(file);
+        fs::rename(new_path, path)?;
+        Ok(())
+    };
+
     // Nobody should poll yet.
     for (idx, &n) in notifiers.iter().enumerate() {
         assert!(
@@ -87,6 +116,11 @@ pub fn test_notifiers(notifiers: &[&dyn UniversalNotifier]) {
     for (idx1, &n1) in notifiers.iter().enumerate() {
         n1.post_notification();
 
+        // If we're using inotify, simulate modifying the file.
+        if let Some(path) = fish_variables_path {
+            modify_path(path).expect("failed to modify file");
+        }
+
         // notifyd requires a round trip to the notifyd server, which means we have to wait a
         // little bit to receive it. In practice 40 ms seems to be enough.
         unsafe { libc::usleep(40000) };

fish-rust/src/universal_notifier/notifyd.rs

@@ -145,5 +145,5 @@ fn test_notifyd_notifiers() {
         .iter()
         .map(|n| n as &dyn UniversalNotifier)
         .collect::<Vec<_>>();
-    super::test_notifiers(&notifiers);
+    super::test_notifiers(&notifiers, None);
 }