diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 2e2cbedb6..82791057e 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -1306,119 +1306,6 @@ 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 { - if (sigio_count_ != signal_get_sigio_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(); - // We may have gotten another SIGIO because the pipe just became writable again. - // In particular BSD sends SIGIO on read even if there is no data to be read. - // Re-fetch the sigio count. - sigio_count_ = signal_get_sigio_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. - // Linux got support for O_ASYNC on fifos in 2.6 (released 2003). Treat its absence as a - // failure, but don't be noisy if this fails. Non-Linux platforms without O_ASYNC should use - // a different notifier strategy to avoid running into this. -#ifdef O_ASYNC - if (fcntl(pipe.fd(), F_SETFL, O_NONBLOCK | O_ASYNC) == -1) -#endif - { - FLOGF(uvar_file, - _(L"fcntl(F_SETFL) failed, universal variable notifications disabled")); - return autoclose_fd_t{}; - } - if (fcntl(pipe.fd(), F_SETOWN, getpid()) == -1) { - FLOGF(uvar_file, - _(L"fcntl(F_SETOWN) failed, universal variable notifications disabled")); - return autoclose_fd_t{}; - } - 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_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. // @@ -1582,16 +1469,6 @@ universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_ return strategy_notifyd; #elif defined(__CYGWIN__) return strategy_shmem_polling; -#elif 0 && defined(SIGIO) && (defined(__APPLE__) || defined(__BSD__) || defined(__linux__)) - // FIXME: The SIGIO notifier relies on an extremely specific interaction between signal handling and - // O_ASYNC writes, and doesn't currently work particularly well, so it's disabled. - // See discussion in #6585 and #7774 for examples of breakage. - // - // The SIGIO notifier does not yet work on WSL. See #7429 - if (is_windows_subsystem_for_linux()) { - return strategy_named_pipe; - } - return strategy_sigio; #else return strategy_named_pipe; #endif @@ -1612,9 +1489,6 @@ 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 2d852a54d..45cb9bbd9 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -166,9 +166,6 @@ class universal_notifier_t { // 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 673430cfb..7b86ce338 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -3987,8 +3987,7 @@ 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_sigio: { + case universal_notifier_t::strategy_named_pipe: { break; // nothing required } } @@ -3999,9 +3998,6 @@ 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); @@ -4051,12 +4047,6 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy // Nobody should poll now. for (size_t i = 0; i < notifier_count; i++) { - // On BSD, SIGIO may be delivered by read() even if it returns EAGAIN; - // that is the polling itself may trigger a SIGIO. Therefore we poll twice. - if (strategy == universal_notifier_t::strategy_sigio) { - (void)poll_notifier(notifiers[i]); - } - if (poll_notifier(notifiers[i])) { err(L"Universal variable notifier polled true after all changes, with strategy %d", (int)strategy); diff --git a/src/signal.cpp b/src/signal.cpp index c763b861d..16fe044ea 100644 --- a/src/signal.cpp +++ b/src/signal.cpp @@ -207,11 +207,6 @@ void signal_clear_cancel() { s_cancellation_signal = 0; } int signal_check_cancel() { return s_cancellation_signal; } -/// Number of SIGIO events. -static volatile relaxed_atomic_t s_sigio_count{0}; - -uint32_t signal_get_sigio_count() { return s_sigio_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) { @@ -276,14 +271,6 @@ static void fish_signal_handler(int sig, siginfo_t *info, void *context) { // test, to verify that we behave correctly when receiving lots of irrelevant signals. break; -#if defined(SIGIO) - case SIGIO: - // An async FD became readable/writable/etc. - // Don't try to look at si_code, it is not set under BSD. - // Don't use ++ to avoid a CAS. - s_sigio_count = s_sigio_count + 1; - break; -#endif } errno = saved_errno; } @@ -366,11 +353,6 @@ 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_sigaction = &fish_signal_handler; act.sa_flags = SA_SIGINFO | SA_RESTART; diff --git a/src/signal.h b/src/signal.h index 3889ef806..99c318646 100644 --- a/src/signal.h +++ b/src/signal.h @@ -42,10 +42,6 @@ int signal_check_cancel(); /// In generaly this should only be done in interactive sessions. void signal_clear_cancel(); -/// \return a count of SIGIO signals. -/// This is used by universal variables, and is a simple unsigned counter which wraps to 0. -uint32_t signal_get_sigio_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 {