diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 290284f8f..cdc6735d1 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -1323,6 +1323,7 @@ static autoclose_fd_t make_fifo(const wchar_t *test_path, const wchar_t *suffix) class universal_notifier_named_pipe_t final : public universal_notifier_t { #if !defined(__CYGWIN__) autoclose_fd_t pipe_fd; + file_id_t pipe_fd_id{}; long long readback_time_usec{0}; size_t readback_amount{0}; @@ -1344,6 +1345,19 @@ class universal_notifier_named_pipe_t final : public universal_notifier_t { ignore_result(read(pipe_fd.fd(), buff, sizeof buff)); } + /// Check if the pipe's file ID (aka struct stat) is different from what we have stored. + /// If it has changed, it indicates that someone has written to the pipe; update our stored id. + /// \return true if changed, false if not. + bool check_pipe_id_changed() { + if (!pipe_fd.valid()) return false; + file_id_t file_id = file_id_for_fd(pipe_fd.fd()); + if (file_id == this->pipe_fd_id) { + return false; + } + this->pipe_fd_id = file_id; + return true; + } + public: explicit universal_notifier_named_pipe_t(const wchar_t *test_path) : pipe_fd(make_fifo(test_path, L".notifier")) {} @@ -1373,6 +1387,8 @@ class universal_notifier_named_pipe_t final : public universal_notifier_t { polling_due_to_readable_fd = true; drain_if_still_readable_time_usec = get_time() + k_readable_too_long_duration_usec; should_sync = true; + // Update our pipe's timestamps. + check_pipe_id_changed(); } return should_sync; } @@ -1392,6 +1408,9 @@ class universal_notifier_named_pipe_t final : public universal_notifier_t { // Now schedule a read for some time in the future. this->readback_time_usec = get_time() + k_flash_duration_usec; this->readback_amount += sizeof c; + + // No need to react to our own changes. + check_pipe_id_changed(); } unsigned long usec_delay_between_polls() const override { @@ -1458,7 +1477,7 @@ class universal_notifier_named_pipe_t final : public universal_notifier_t { drain_excessive_data(); } } - return true; + return check_pipe_id_changed(); } #else // this class isn't valid on this system public: diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 033870e11..b1e41be58 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -4002,27 +4002,10 @@ bool poll_notifier(const std::unique_ptr ¬e) { return result; } -static void trigger_or_wait_for_notification(universal_notifier_t::notifier_strategy_t strategy) { - switch (strategy) { - case universal_notifier_t::strategy_shmem_polling: { - break; // nothing required - } - 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 40 ms seems to be enough. - usleep(40000); - break; - } - case universal_notifier_t::strategy_named_pipe: { - break; // nothing required - } - } -} - static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy_t strategy) { say(L"Testing universal notifiers with strategy %d", (int)strategy); - std::unique_ptr notifiers[16]; - size_t notifier_count = sizeof notifiers / sizeof *notifiers; + constexpr size_t notifier_count = 16; + std::unique_ptr notifiers[notifier_count]; // Populate array of notifiers. for (size_t i = 0; i < notifier_count; i++) { @@ -4041,21 +4024,31 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy for (size_t post_idx = 0; post_idx < notifier_count; post_idx++) { notifiers[post_idx]->post_notification(); - // Do special stuff to "trigger" a notification for testing. - trigger_or_wait_for_notification(strategy); + if (strategy == 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 40 ms seems to be enough. + usleep(40000); + } for (size_t i = 0; i < notifier_count; i++) { + bool polled = poll_notifier(notifiers[i]); + // We aren't concerned with the one who posted. Poll from it (to drain it), and then // skip it. if (i == post_idx) { - poll_notifier(notifiers[i]); continue; } - if (!poll_notifier(notifiers[i])) { + if (!polled) { err(L"Universal variable notifier (%lu) %p polled failed to notice changes, with " L"strategy %d", i, notifiers[i].get(), (int)strategy); + continue; + } + // It should not poll again immediately. + if (poll_notifier(notifiers[i])) { + err(L"Universal variable notifier (%lu) %p polled twice in a row with strategy %d", + i, notifiers[i].get(), (int)strategy); } } diff --git a/src/input_common.cpp b/src/input_common.cpp index 6a025d7a9..bef35b36f 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -96,8 +96,8 @@ static readb_result_t readb(int in_fd, bool queue_is_empty) { // The priority order is: uvars, stdin, ioport. // Check to see if we want a universal variable barrier. // This may come about through readability, or through a call to poll(). - if (notifier.poll() || - (fdset.test(notifier_fd) && notifier.notification_fd_became_readable(notifier_fd))) { + if ((fdset.test(notifier_fd) && notifier.notification_fd_became_readable(notifier_fd)) || + notifier.poll()) { if (env_universal_barrier() && !queue_is_empty) { return readb_uvar_notified; }