From 7c153a8307c662fa853280fd64657dff0cd9a625 Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Sat, 10 Apr 2021 20:41:33 -0700
Subject: [PATCH] Introduce prepare_to_select, fixing job_summary.py

This concerns printing status messages for background jobs which have
stopped or finished. Previously fish would do this from two places:

1. Before running a command (including empty string)
2. If a signal is received during select()

So if the job finishes while fish is doing something else (like running an
event handler) then we would not print status messages until the user hit
return. This caused the job_summary.py test to be flaky.

Fix this by splitting the interrupt handler into two parts: a part that
handles signals (e.g. triggering exit from the reader), and a part that
always runs just before blocking in select(). This second part always
reaps jobs and prints their status messages. This narrows the window for a
job exit to be "missed" before fish blocks in select, and should make the
job_summary.py test more reliable.
---
 src/input.cpp        | 15 ++++++++++-----
 src/input.h          |  3 +++
 src/input_common.cpp |  7 +++++++
 src/input_common.h   |  4 ++++
 4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/input.cpp b/src/input.cpp
index 13c6a6172..e1ae17af9 100644
--- a/src/input.cpp
+++ b/src/input.cpp
@@ -318,14 +318,19 @@ void init_input() {
 inputter_t::inputter_t(parser_t &parser, int in)
     : input_event_queue_t(in), parser_(parser.shared()) {}
 
-/// Handle interruptions to key reading by reaping finished jobs and propagating the interrupt to
-/// the reader.
-void inputter_t::select_interrupted() /* override */ {
-    // Fire any pending events.
+void inputter_t::prepare_to_select() /* override */ {
+    // Fire any pending events and reap stray processes, including printing exit status messages.
     auto &parser = *this->parser_;
     event_fire_delayed(parser);
-    // Reap stray processes, including printing exit status messages.
     if (job_reap(parser, true)) reader_schedule_prompt_repaint();
+}
+
+void inputter_t::select_interrupted() /* override */ {
+    // Fire any pending events and reap stray processes, including printing exit status messages.
+    auto &parser = *this->parser_;
+    event_fire_delayed(parser);
+    if (job_reap(parser, true)) reader_schedule_prompt_repaint();
+
     // Tell the reader an event occurred.
     if (reader_reading_interrupted()) {
         auto vintr = shell_modes.c_cc[VINTR];
diff --git a/src/input.h b/src/input.h
index 44593d677..726de50d9 100644
--- a/src/input.h
+++ b/src/input.h
@@ -54,6 +54,9 @@ class inputter_t final : private input_event_queue_t {
     wchar_t function_pop_arg();
 
    private:
+    // Called right before potentially blocking in select().
+    void prepare_to_select() override;
+
     // Called when select() is interrupted by a signal.
     void select_interrupted() override;
 
diff --git a/src/input_common.cpp b/src/input_common.cpp
index 5a1f636f2..f473720d2 100644
--- a/src/input_common.cpp
+++ b/src/input_common.cpp
@@ -161,6 +161,12 @@ char_event_t input_event_queue_t::readch() {
             return mevt.acquire();
         }
 
+        // We are going to block; but first allow any override to inject events.
+        this->prepare_to_select();
+        if (auto mevt = try_pop()) {
+            return mevt.acquire();
+        }
+
         readb_result_t rr = readb(in_);
         switch (rr) {
             case readb_eof:
@@ -229,5 +235,6 @@ void input_event_queue_t::push_back(const char_event_t& ch) { queue_.push_back(c
 
 void input_event_queue_t::push_front(const char_event_t& ch) { queue_.push_front(ch); }
 
+void input_event_queue_t::prepare_to_select() {}
 void input_event_queue_t::select_interrupted() {}
 input_event_queue_t::~input_event_queue_t() = default;
diff --git a/src/input_common.h b/src/input_common.h
index c696df60c..07b1539fd 100644
--- a/src/input_common.h
+++ b/src/input_common.h
@@ -213,6 +213,10 @@ class input_event_queue_t {
         queue_.insert(queue_.begin(), begin, end);
     }
 
+    /// Override point for when we are about to (potentially) block in select(). The default does
+    /// nothing.
+    virtual void prepare_to_select();
+
     /// Override point for when when select() is interrupted by a signal. The default does nothing.
     virtual void select_interrupted();