Correct bug causing early teardown of fd_monitor

fd_monitor is used when an external command pipes into a buffer, e.g. for
command substitutions. It monitors the read end of the external command's
pipe in the background, and fills the buffer as data arrives. fd_monitor is
multiplexed, so multiple buffers can be monitored at once by a single
thread.

It may happen that there's no active buffer fill; in this case fd_monitor
wants to keep its thread alive for a little bit in case a new one arrives.
This is useful for e.g. handling loops where you run the same command
multiple times.

However there was a bug due to a refactoring which caused fd_monitor to
exit too aggressively. This didn't affect correctness but it meant more
thread creation and teardown.

Fix this; this improves the aliases.fish benchmark by about 20 msec.

No need to changelog this IMO.

src/fd_monitor.cpp

@@ -117,7 +117,7 @@ void fd_monitor_t::run_in_background() {
     for (;;) {
         // Poke any items that need it.
         if (!pokelist.empty()) {
-            this->poke_in_background(std::move(pokelist));
+            this->poke_in_background(pokelist);
             pokelist.clear();
         }
 
@@ -136,16 +136,15 @@ void fd_monitor_t::run_in_background() {
             timeout_usec = std::min(timeout_usec, item.usec_remaining(now));
         }
 
-        // If we have only one item, it means that we are not actively monitoring any fds other than
-        // our self-pipe. In this case we wish to allow the thread to exit, but after a time, so we
+        // If we have no items, then we wish to allow the thread to exit, but after a time, so we
         // aren't spinning up and tearing down the thread repeatedly.
-        // Set a timeout of 16 msec; if nothing becomes readable by then we will exit.
+        // Set a timeout of 256 msec; if nothing becomes readable by then we will exit.
         // We refer to this as the wait-lap.
-        bool is_wait_lap = (items_.size() == 1);
+        bool is_wait_lap = (items_.size() == 0);
         if (is_wait_lap) {
             assert(timeout_usec == fd_monitor_item_t::kNoTimeout &&
                    "Should not have a timeout on wait-lap");
-            timeout_usec = 16 * kUsecPerMsec;
+            timeout_usec = 256 * kUsecPerMsec;
         }
 
         // Call select().
@@ -170,7 +169,8 @@ void fd_monitor_t::run_in_background() {
 
         // Handle any changes if the change signaller was set. Alternatively this may be the wait
         // lap, in which case we might want to commit to exiting.
-        if (fds.test(change_signal_fd) || is_wait_lap) {
+        bool change_signalled = fds.test(change_signal_fd);
+        if (change_signalled || is_wait_lap) {
             // Clear the change signaller before processing incoming changes.
             change_signaller_.try_consume();
             auto data = data_.acquire();
@@ -185,7 +185,8 @@ void fd_monitor_t::run_in_background() {
             pokelist = std::move(data->pokelist);
             data->pokelist.clear();
 
-            if (data->terminate || (is_wait_lap && items_.empty())) {
+            if (data->terminate ||
+                (is_wait_lap && items_.empty() && pokelist.empty() && !change_signalled)) {
                 // Maybe terminate is set.
                 // Alternatively, maybe we had no items, waited a bit, and still have no items.
                 // It's important to do this while holding the lock, otherwise we race with new
@@ -199,7 +200,7 @@ void fd_monitor_t::run_in_background() {
     }
 }
 
-void fd_monitor_t::poke_in_background(poke_list_t pokelist) {
+void fd_monitor_t::poke_in_background(const poke_list_t &pokelist) {
     ASSERT_IS_BACKGROUND_THREAD();
     auto poker = [&pokelist](fd_monitor_item_t &item) {
         int fd = item.fd.fd();

src/fd_monitor.h

@@ -110,7 +110,7 @@ class fd_monitor_t {
     // Poke items in the pokelist, removing any items that close their FD.
     // The pokelist is consumed after this.
     // This is only called in the background thread.
-    void poke_in_background(poke_list_t pokelist);
+    void poke_in_background(const poke_list_t &pokelist);
 
     // The list of items to monitor. This is only accessed on the background thread.
     item_list_t items_{};