Remove use of POLL_IN in SIGIO uvar notifier

This fixes up the SIGIO notifier in preparation for using it on BSD. It
removes the reliance on the signal's si_code, which is not available in
BSD, and it properly handles the BSD behavior where SIGIO is delivered on
a read even if the read returns EAGAIN.
This commit is contained in:
ridiculousfish
2020-10-24 20:19:35 -07:00
parent 3652bcf731
commit 8bb20a8d91
3 changed files with 22 additions and 30 deletions

View File

@@ -1344,13 +1344,15 @@ class universal_notifier_sigio_t final : public universal_notifier_t {
}
bool poll() override {
uint32_t count = signal_get_sigio_pollin_count();
if (count != sigio_pollin_count_) {
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();
sigio_pollin_count_ = count;
// 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;
@@ -1369,21 +1371,15 @@ class universal_notifier_sigio_t final : public universal_notifier_t {
// 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");
FLOGF(uvar_file,
_(L"fcntl(F_SETFL) failed, universal variable notifications disabled"));
return autoclose_fd_t{};
}
if (fcntl(pipe.fd(), F_SETOWN, getpid()) == -1) {
FLOG(uvar_file, "fcntl(F_SETOWN) failed, universal variable notifications disabled");
FLOGF(uvar_file,
_(L"fcntl(F_SETOWN) failed, universal variable notifications disabled"));
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;
}
@@ -1404,7 +1400,7 @@ class universal_notifier_sigio_t final : public universal_notifier_t {
}
autoclose_fd_t pipe_{};
uint32_t sigio_pollin_count_{0};
uint32_t sigio_count_{0};
#else
public:
[[noreturn]] universal_notifier_sigio_t() {
@@ -1574,11 +1570,10 @@ 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;
// Note: We use POLL_IN to query SIGIO information, without it it is useless.
#elif defined(SIGIO) && defined(POLL_IN) && !defined(__FreeBSD__)
return strategy_sigio;
#elif defined(__CYGWIN__)
return strategy_shmem_polling;
#elif defined(SIGIO) && !defined(__FreeBSD__)
return strategy_sigio;
#else
return strategy_named_pipe;
#endif

View File

@@ -207,10 +207,10 @@ 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<uint32_t> s_signal_pollin_count{0};
/// Number of SIGIO events.
static volatile relaxed_atomic_t<uint32_t> s_sigio_count{0};
uint32_t signal_get_sigio_pollin_count() { return s_signal_pollin_count; }
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).
@@ -276,13 +276,12 @@ 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) && defined(POLL_IN)
#if defined(SIGIO)
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;
}
// 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
}

View File

@@ -42,11 +42,9 @@ 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();
/// \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.