From 558dd6e53de0ee6c6807c67bb3efbdbf2706281c Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 1 Oct 2020 12:35:18 -0700 Subject: [PATCH] Add sigio-based universal notifier strategy Introduce a new strategy for notifying other fish processes of universal variable changes, as a planned replacement for the complex strategy_named_pipe. The new strategy still uses a named pipe, but instead of select() on it, it arranges for SIGIO to be delivered when data is available. If a SIGIO has been seen since the last check, it means the file needs to be re-read. --- src/env_universal_common.cpp | 120 +++++++++++++++++++++++++++++++++++ src/env_universal_common.h | 8 ++- src/fish_tests.cpp | 14 ++-- src/signal.cpp | 18 ++++++ src/signal.h | 6 ++ 5 files changed, 157 insertions(+), 9 deletions(-) diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 98c1df3e0..012151e46 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -25,6 +25,7 @@ #ifdef HAVE_SYS_SELECT_H #include // IWYU pragma: keep #endif +#include #include #include // IWYU pragma: keep #include // IWYU pragma: keep @@ -43,6 +44,7 @@ #include "fallback.h" // IWYU pragma: keep #include "flog.h" #include "path.h" +#include "signal.h" #include "utf8.h" #include "util.h" // IWYU pragma: keep #include "wcstringutil.h" @@ -1298,6 +1300,119 @@ static autoclose_fd_t make_fifo(const wchar_t *test_path, const wchar_t *suffix) return res; } +// A universal notifier which uses SIGIO with a named pipe. This relies on the following behavior +// which appears to work on Linux: if data becomes available on the pipe then all processes get +// SIGIO even if the data is read (i.e. there is no race between SIGIO and another proc reading). +// +// A key difference between Linux and Mac is that on Linux SIGIO is only delivered if the pipe was +// previously empty. That is, two successive writes with no reads will produce two SIGIOs on Mac but +// only one on Linux. +// +// So, to post a notification, write anything to the pipe; if that would block drain the pipe and +// try again. +// +// To receive a notification, watch for SIGIO on the read end, then read out the data to enable +// SIGIO to be delivered again. +class universal_notifier_sigio_t final : public universal_notifier_t { +#ifdef SIGIO + public: + // Note we use a different suffix from universal_notifier_named_pipe_t to produce different + // FIFOs. This is because universal_notifier_named_pipe_t is level triggered while we are edge + // triggered. If they shared the same FIFO, then the named_pipe variant would keep firing + // forever. + explicit universal_notifier_sigio_t(const wchar_t *test_path) + : pipe_(try_make_pipe(test_path, L".notify")) {} + + ~universal_notifier_sigio_t() = default; + + void post_notification() override { + if (!pipe_.valid()) return; + + // Write a byte to send SIGIO. If the pipe is full, read some and try again. + while (!write_1_byte()) { + if (errno == EINTR) { + continue; + } else if (errno == EAGAIN) { + // The pipe is full. Try once more. + drain_some(); + write_1_byte(); + break; + } else { + break; + } + } + } + + bool poll() override { + uint32_t count = signal_get_sigio_pollin_count(); + if (count != sigio_pollin_count_) { + // On Mac, SIGIO is generated on every write to the pipe. + // On Linux, it is generated only when the pipe goes from empty to non-empty. + // Read from the pipe so that SIGIO may be delivered again. + drain_some(); + sigio_pollin_count_ = count; + return true; + } + return false; + } + + private: + // Construct a pipe for a given fifo path, arranging for SIGIO to be delivered. + // \return an invalid fd on failure. + static autoclose_fd_t try_make_pipe(const wchar_t *test_path, const wchar_t *suffix) { + autoclose_fd_t pipe = make_fifo(test_path, suffix); + if (!pipe.valid()) { + return autoclose_fd_t{}; + } + // Note that O_ASYNC cannot be passed to open() (see Linux kernel bug #5993). + // We have to set it afterwards like so. + // Also Linux got support for O_ASYNC on fifos in 2.6 (released 2003). + // Do not be noisy if this fails. + if (fcntl(pipe.fd(), F_SETFL, O_NONBLOCK | O_ASYNC) == -1) { + FLOG(uvar_file, "fcntl(F_SETFL) failed, universal variable notifications disabled"); + return autoclose_fd_t{}; + } + if (fcntl(pipe.fd(), F_SETOWN, getpid()) == -1) { + wperror(L"fcntl(F_SETOWN)"); + return autoclose_fd_t{}; + } +#ifdef F_SETSIG + // Linux requires an (apparently redundant) setting of SIGIO, in order for the si_code to be + // set properly in the signal handler's context. + if (fcntl(pipe.fd(), F_SETSIG, SIGIO) == -1) { + wperror(L"fcntl(F_SETSIG)"); + return autoclose_fd_t{}; + } +#endif + return pipe; + } + + // Try writing a single byte to our pipe. + // \return true on success, false on error, in which case errno will be set. + bool write_1_byte() const { + assert(pipe_.valid() && "Invalid pipe"); + char c = 0x42; + ssize_t amt = write(pipe_.fd(), &c, sizeof c); + return amt > 0; + } + + // Read some data off of the pipe, discarding it. + void drain_some() const { + if (!pipe_.valid()) return; + char buff[256]; + ignore_result(read(pipe_.fd(), buff, sizeof buff)); + } + + autoclose_fd_t pipe_{}; + uint32_t sigio_pollin_count_{0}; +#else + public: + [[noreturn]] universal_notifier_sigio_t() { + DIE("universal_notifier_sigio_t cannot be used on this system"); + } +#endif +}; + // Named-pipe based notifier. All clients open the same named pipe for reading and writing. The // pipe's readability status is a trigger to enter polling mode. // @@ -1459,6 +1574,8 @@ class universal_notifier_named_pipe_t final : public universal_notifier_t { universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_strategy() { #ifdef FISH_NOTIFYD_AVAILABLE return strategy_notifyd; +#elif defined(SIGIO) + return strategy_sigio; #elif defined(__CYGWIN__) return strategy_shmem_polling; #else @@ -1481,6 +1598,9 @@ std::unique_ptr universal_notifier_t::new_notifier_for_str case strategy_shmem_polling: { return make_unique(); } + case strategy_sigio: { + return make_unique(test_path); + } case strategy_named_pipe: { return make_unique(test_path); } diff --git a/src/env_universal_common.h b/src/env_universal_common.h index cc2e00e2b..468ae43b0 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -159,13 +159,15 @@ class env_universal_t { class universal_notifier_t { public: enum notifier_strategy_t { - // Use a value in shared memory. Simple, but requires polling and therefore semi-frequent - // wakeups. + // Poll on shared memory. strategy_shmem_polling, - // Strategy that uses notify(3). Simple and efficient, but OS X/macOS only. + // Mac-specific notify(3) implementation. strategy_notifyd, + // Set up a fifo and then waits for SIGIO to be delivered on it. + strategy_sigio, + // Strategy that uses a named pipe. Somewhat complex, but portable and doesn't require // polling most of the time. strategy_named_pipe, diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index fab93be81..2bbc0fd79 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -3776,13 +3776,11 @@ static void test_universal_ok_to_save() { } bool poll_notifier(const std::unique_ptr ¬e) { - bool result = false; - if (note->usec_delay_between_polls() > 0) { - result = note->poll(); - } + if (note->poll()) return true; + bool result = false; int fd = note->notification_fd(); - if (!result && fd >= 0) { + if (fd >= 0) { fd_set fds; FD_ZERO(&fds); FD_SET(fd, &fds); @@ -3805,7 +3803,8 @@ static void trigger_or_wait_for_notification(universal_notifier_t::notifier_stra usleep(40000); break; } - case universal_notifier_t::strategy_named_pipe: { + case universal_notifier_t::strategy_named_pipe: + case universal_notifier_t::strategy_sigio: { break; // nothing required } } @@ -3816,6 +3815,9 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy std::unique_ptr notifiers[16]; size_t notifier_count = sizeof notifiers / sizeof *notifiers; + // Set up SIGIO handler as needed. + signal_set_handlers(false); + // Populate array of notifiers. for (size_t i = 0; i < notifier_count; i++) { notifiers[i] = universal_notifier_t::new_notifier_for_strategy(strategy, UVARS_TEST_PATH); diff --git a/src/signal.cpp b/src/signal.cpp index 6bc1f12a8..f3640fba3 100644 --- a/src/signal.cpp +++ b/src/signal.cpp @@ -207,6 +207,11 @@ void signal_clear_cancel() { s_cancellation_signal = 0; } int signal_check_cancel() { return s_cancellation_signal; } +/// Number of POLL_IN SIGIO events. +static volatile relaxed_atomic_t s_signal_pollin_count{0}; + +uint32_t signal_get_sigio_pollin_count() { return s_signal_pollin_count; } + /// The single signal handler. By centralizing signal handling we ensure that we can never install /// the "wrong" signal handler (see #5969). static void fish_signal_handler(int sig, siginfo_t *info, void *context) { @@ -270,6 +275,14 @@ static void fish_signal_handler(int sig, siginfo_t *info, void *context) { // We have a sigalarm handler that does nothing. This is used in the signal torture // test, to verify that we behave correctly when receiving lots of irrelevant signals. break; + + case SIGIO: + // An async FD became readable/writable/etc. + if (info->si_code == POLL_IN) { + // Don't use ++ to avoid a CAS. + s_signal_pollin_count = s_signal_pollin_count + 1; + } + break; } errno = saved_errno; } @@ -352,6 +365,11 @@ void signal_set_handlers(bool interactive) { act.sa_flags = SA_SIGINFO; sigaction(SIGINT, &act, nullptr); + // Apply our SIGIO handler. + act.sa_sigaction = &fish_signal_handler; + act.sa_flags = SA_SIGINFO; + sigaction(SIGIO, &act, nullptr); + // Whether or not we're interactive we want SIGCHLD to not interrupt restartable syscalls. act.sa_flags = SA_SIGINFO; act.sa_sigaction = &fish_signal_handler; diff --git a/src/signal.h b/src/signal.h index 99c318646..6c8d9cdc4 100644 --- a/src/signal.h +++ b/src/signal.h @@ -42,6 +42,12 @@ int signal_check_cancel(); /// In generaly this should only be done in interactive sessions. void signal_clear_cancel(); +/// \return a count of POLL_IN SIGIO events. +/// This corresponds to the number of times we have received SIGIO with POLL_IN set as the code. +/// This is used by the universal variable machinery. It is a simple unsigned counter which wraps to +/// 0. +uint32_t signal_get_sigio_pollin_count(); + enum class topic_t : uint8_t; /// A sigint_detector_t can be used to check if a SIGINT (or SIGHUP) has been delivered. class sigchecker_t {