Early implementation of inotify-based universal variable notifier for

Linux

common.h

@@ -849,6 +849,9 @@ void assert_is_not_forked_child(const char *who);
 #define ASSERT_IS_NOT_FORKED_CHILD_TRAMPOLINE(x) assert_is_not_forked_child(x)
 #define ASSERT_IS_NOT_FORKED_CHILD() ASSERT_IS_NOT_FORKED_CHILD_TRAMPOLINE(__FUNCTION__)
 
+/** Macro to help suppress potentially unused variable warnings */
+#define USE(var) (void)(var)
+
 extern "C" {
     __attribute__((noinline)) void debug_thread_error(void);
 }

configure.ac

@@ -366,6 +366,7 @@ AC_DEFINE(
 # Check for os dependant libraries for all binaries.
 AC_SEARCH_LIBS( connect, socket, , [AC_MSG_ERROR([Cannot find the socket library, needed to build this package.] )] )
 AC_SEARCH_LIBS( nanosleep, rt, , [AC_MSG_ERROR([Cannot find the rt library, needed to build this package.] )] )
+AC_SEARCH_LIBS( shm_open, rt, , [AC_MSG_ERROR([Cannot find the rt library, needed to build this package.] )] )
 AC_SEARCH_LIBS( pthread_create, pthread, , [AC_MSG_ERROR([Cannot find the pthread library, needed to build this package.] )] )
 AC_SEARCH_LIBS( setupterm, [ncurses tinfo curses], , [AC_MSG_ERROR([Could not find a curses implementation, needed to build fish. If this is Linux, try running 'sudo apt-get install libncurses5-dev' or 'sudo yum install ncurses-devel'])] )
 AC_SEARCH_LIBS( [nan], [m], [AC_DEFINE( [HAVE_NAN], [1], [Define to 1 if you have the nan function])] )
@@ -520,6 +521,7 @@ AC_CHECK_FUNCS( wcsdup wcsndup wcslen wcscasecmp wcsncasecmp fwprintf )
 AC_CHECK_FUNCS( futimes wcwidth wcswidth wcstok fputwc fgetwc )
 AC_CHECK_FUNCS( wcstol wcslcat wcslcpy lrand48_r killpg )
 AC_CHECK_FUNCS( backtrace backtrace_symbols sysconf getifaddrs )
+AC_CHECK_FUNCS( inotify_init inotify_init1 )
 
 if test x$local_gettext != xno; then
   AC_CHECK_FUNCS( gettext dcgettext )
@@ -749,6 +751,7 @@ case $target_os in
     ;;
 esac
 
+
 # Tell the world what we know
 AC_CONFIG_FILES([Makefile])
 AC_OUTPUT

env_universal_common.cpp

@@ -16,6 +16,7 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <arpa/inet.h>
 #include <pwd.h>
 #include <errno.h>
 #include <sys/stat.h>
@@ -51,6 +52,11 @@
 #include <notify.h>
 #endif
 
+#if HAVE_INOTIFY_INIT || HAVE_INOTIFY_INIT1
+#define FISH_INOTIFY_AVAILABLE 1
+#include <sys/inotify.h>
+#endif
+
 /**
    Non-wide version of the set command
 */
@@ -1542,7 +1548,9 @@ public:
     {
         if (token != 0)
         {
+#if FISH_NOTIFYD_AVAILABLE
             notify_cancel(token);
+#endif
         }
     }
     
@@ -1551,17 +1559,19 @@ public:
         return notify_fd;
     }
     
-    void drain_notification_fd(int fd)
+    bool drain_notification_fd(int fd)
     {
         /* notifyd notifications come in as 32 bit values. We don't care about the value. We set ourselves as non-blocking, so just read until we can't read any more. */
         assert(fd == notify_fd);
+        bool read_something = false;
         unsigned char buff[64];
         ssize_t amt_read;
         do
         {
             amt_read = read(notify_fd, buff, sizeof buff);
-
+            read_something = (read_something || amt_read > 0);
         } while (amt_read == sizeof buff);
+        return read_something;
     }
     
     void post_notification()
@@ -1576,10 +1586,85 @@ public:
     }
 };
 
+class universal_notifier_inotify_t : public universal_notifier_t
+{
+    int watch_fd;
+    int watch_descriptor;
+    
+    void setup_inotify(const wchar_t *test_path)
+    {
+#if FISH_INOTIFY_AVAILABLE
+        
+        const wcstring path = test_path ? test_path : default_vars_path();
+        
+        // Construct the watchfd
+#if HAVE_INOTIFY_INIT1
+        this->watch_fd = inotify_init1(IN_NONBLOCK | IN_CLOEXEC);
+#else
+        this->watch_fd = inotify_init();
+        if (this->watch_fd >= 0)
+        {
+            int flags = fcntl(this->watch_fd, F_GETFL, 0);
+            fcntl(this->watch_fd, F_SETFL, flags | O_NONBLOCK | FD_CLOEXEC);
+        }
+#endif
+        if (this->watch_fd < 0)
+        {
+            wperror(L"inotify_init");
+        }
+        else
+        {
+            std::string narrow_path = wcs2string(path);
+            this->watch_descriptor = inotify_add_watch(this->watch_fd, narrow_path.c_str(), IN_MODIFY | IN_EXCL_UNLINK);
+            if (this->watch_descriptor < 0)
+            {
+                wperror(L"inotify_add_watch");
+            }
+        }
+#endif
+    }
+    
+public:
+    
+    universal_notifier_inotify_t(const wchar_t *test_path) : watch_fd(-1), watch_descriptor(-1)
+    {
+        setup_inotify(test_path);
+    }
+    
+    ~universal_notifier_inotify_t()
+    {
+        if (watch_fd >= 0)
+        {
+            close(watch_fd);
+        }
+        USE(watch_descriptor);
+    }
+    
+    int notification_fd()
+    {
+        return watch_fd;
+    }
+    
+    bool drain_notification_fd(int fd)
+    {
+        assert(fd == watch_fd);
+        bool result = false;
+        
+#if FISH_INOTIFY_AVAILABLE
+        struct inotify_event evt = {};
+        ssize_t read_amt = read(watch_fd, &evt, sizeof evt);
+        result = (read_amt > 0);
+#endif
+        return result;
+    }
+};
+
 universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_strategy()
 {
 #if FISH_NOTIFYD_AVAILABLE
     return strategy_notifyd;
+#elif FISH_INOTIFY_AVAILABLE
+    return strategy_inotify;
 #else
     return strategy_shmem_polling;
 #endif
@@ -1591,7 +1676,7 @@ universal_notifier_t &universal_notifier_t::default_notifier()
     return *result;
 }
 
-universal_notifier_t *universal_notifier_t::new_notifier_for_strategy(universal_notifier_t::notifier_strategy_t strat)
+universal_notifier_t *universal_notifier_t::new_notifier_for_strategy(universal_notifier_t::notifier_strategy_t strat, const wchar_t *test_path)
 {
     if (strat == strategy_default)
     {
@@ -1605,6 +1690,10 @@ universal_notifier_t *universal_notifier_t::new_notifier_for_strategy(universal_
         case strategy_notifyd:
             return new universal_notifier_notifyd_t();
             
+        case strategy_inotify:
+            return new universal_notifier_inotify_t(test_path);
+
+            
         default:
             fprintf(stderr, "Unsupported strategy %d\n", strat);
             return NULL;
@@ -1644,6 +1733,7 @@ unsigned long universal_notifier_t::usec_delay_between_polls() const
     return 0;
 }
 
-void universal_notifier_t::drain_notification_fd(int fd)
+bool universal_notifier_t::drain_notification_fd(int fd)
 {
+    return false;
 }

env_universal_common.h

@@ -280,6 +280,7 @@ public:
     {
         strategy_default,
         strategy_shmem_polling,
+        strategy_inotify,
         strategy_notifyd
     };
 
@@ -297,7 +298,7 @@ public:
     virtual ~universal_notifier_t();
     
     /* Factory constructor. Free with delete */
-    static universal_notifier_t *new_notifier_for_strategy(notifier_strategy_t strat);
+    static universal_notifier_t *new_notifier_for_strategy(notifier_strategy_t strat, const wchar_t *test_path = NULL);
     
     /* Default instance. Other instances are possible for testing. */
     static universal_notifier_t &default_notifier();
@@ -317,8 +318,8 @@ public:
     /* Recommended delay between polls. A value of 0 means no polling required (so no timeout) */
     virtual unsigned long usec_delay_between_polls() const;
     
-    /* The notification_fd is readable; drain it */
-    virtual void drain_notification_fd(int fd);
+    /* The notification_fd is readable; drain it. Returns true if a notification is considered to have been posted. */
+    virtual bool drain_notification_fd(int fd);
 };
 
 std::string get_machine_identifier();

fish_tests.cpp

@@ -2258,14 +2258,43 @@ bool poll_notifier(universal_notifier_t *note)
             struct timeval tv = {0, 0};
             if (select(fd + 1, &fds, NULL, NULL, &tv) > 0 && FD_ISSET(fd, &fds))
             {
-                note->drain_notification_fd(fd);
-                result = true;
+                result = note->drain_notification_fd(fd);
             }
         }
     }
     return result;
 }
 
+static void trigger_or_wait_for_notification(universal_notifier_t *notifier, universal_notifier_t::notifier_strategy_t strategy)
+{
+    switch (strategy)
+    {
+        case universal_notifier_t::strategy_default:
+            assert(0 && "strategy_default should be passed");
+            break;
+            
+        case universal_notifier_t::strategy_shmem_polling:
+            // nothing required
+            break;
+            
+        case universal_notifier_t::strategy_notifyd:
+            // notifyd requires a round trip to the notifyd server, which means we have to wait a little bit to receive it
+            // In practice, this seems to be enough
+            usleep(1000000 / 25);
+            break;
+            
+        case universal_notifier_t::strategy_inotify:
+        {
+            // Hacktastic. Replace the file, then wait
+            char cmd[512];
+            sprintf(cmd, "cp %ls %ls ; mv %ls %ls", UVARS_TEST_PATH, UVARS_TEST_PATH L".tmp", UVARS_TEST_PATH ".tmp", UVARS_TEST_PATH);
+            if (system(cmd)) err(L"Command failed: %s", cmd);
+            usleep(1000000 / 25);
+            break;
+        }
+    }
+}
+
 static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy_t strategy)
 {
     assert(strategy != universal_notifier_t::strategy_default);
@@ -2276,7 +2305,7 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy
     // Populate array of notifiers
     for (size_t i=0; i < notifier_count; i++)
     {
-        notifiers[i] = universal_notifier_t::new_notifier_for_strategy(strategy);
+        notifiers[i] = universal_notifier_t::new_notifier_for_strategy(strategy, UVARS_TEST_PATH);
     }
     
     // Nobody should poll yet
@@ -2293,12 +2322,9 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy
     {
         notifiers[post_idx]->post_notification();
         
-        // notifyd requires a round trip to the notifyd server, which means we have to wait a little bit to receive it
-        // In practice, this seems to be enough
-        if (strategy != universal_notifier_t::strategy_shmem_polling)
-        {
-            usleep(1000000 / 25);
-        }
+        // Do special stuff to "trigger" a notification for testing
+        trigger_or_wait_for_notification(notifiers[post_idx], strategy);
+        
         for (size_t i=0; i < notifier_count; i++)
         {
             // We aren't concerned with the one who posted
@@ -2339,6 +2365,11 @@ static void test_universal_notifiers()
 #if __APPLE__
     test_notifiers_with_strategy(universal_notifier_t::strategy_notifyd);
 #endif
+#if __linux || linux
+    if (system("mkdir -p /tmp/fish_uvars_test/ && touch /tmp/fish_uvars_test/varsfile.txt")) err(L"mkdir failed");
+    
+    test_notifiers_with_strategy(universal_notifier_t::strategy_inotify);
+#endif
 }
 
 class history_tests_t

input_common.cpp

@@ -135,10 +135,6 @@ static wint_t readb()
         }
         
         res = select(fd_max + 1, &fdset, 0, 0, usecs_delay > 0 ? &tv : NULL);
-        if (res == 0)
-        {
-            fprintf(stderr, "ping\n");
-        }
         if (res==-1)
         {
             switch (errno)
@@ -196,8 +192,11 @@ static wint_t readb()
             
             if (notifier_fd > 0 && FD_ISSET(notifier_fd, &fdset))
             {
-                notifier.drain_notification_fd(notifier_fd);
-                env_universal_barrier();
+                bool notified = notifier.drain_notification_fd(notifier_fd);
+                if (notified)
+                {
+                    env_universal_barrier();
+                }
             }
 
             if (ioport > 0 && FD_ISSET(ioport, &fdset))