From 4faebf74e6320a331ef6fe0adc4a7b7ad2f0b80f Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Thu, 7 Jan 2021 12:06:21 -0800
Subject: [PATCH] Remove 100 msec timeout from io_buffer_t

This removes the 100 msec timeout from io_buffer_t. We no longer need to
periodically wake up to check if a command substitution is finished,
because we get explicitly poked when that happens.
---
 src/io.cpp | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/src/io.cpp b/src/io.cpp
index 9a8bcc0b0..815e1fe1a 100644
--- a/src/io.cpp
+++ b/src/io.cpp
@@ -94,7 +94,7 @@ void io_buffer_t::begin_filling(autoclose_fd_t fd) {
     // We want to fill buffer_ by reading from fd. fd is the read end of a pipe; the write end is
     // owned by another process, or something else writing in fish.
     // Pass fd to an fd_monitor. It will add fd to its select() loop, and give us a callback when
-    // the fd is readable, or when our timeout is hit. The usual path is that we will get called
+    // the fd is readable, or when our item is poked. The usual path is that we will get called
     // back, read a bit from the fd, and append it to the buffer. Eventually the write end of the
     // pipe will be closed - probably the other process exited - and fd will be widowed; read() will
     // then return 0 and we will stop reading.
@@ -102,9 +102,10 @@ void io_buffer_t::begin_filling(autoclose_fd_t fd) {
     // e.g.:
     //   cmd ( background & ; echo hi )
     // Here the background process will inherit the write end of the pipe and hold onto it forever.
-    // In this case, we will hit the timeout on waiting for more data and notice that the shutdown
-    // flag is set (this indicates that the command substitution is done); in this case we will read
-    // until we get EAGAIN and then give up.
+    // In this case, when complete_background_fillthread() is called, the callback will be invoked
+    // with item_wake_reason_t::poke, and we will notice that the shutdown flag is set (this
+    // indicates that the command substitution is done); in this case we will read until we get
+    // EAGAIN and then give up.
 
     // Construct a promise that can go into our background thread.
     auto promise = std::make_shared<std::promise<void>>();
@@ -113,17 +114,10 @@ void io_buffer_t::begin_filling(autoclose_fd_t fd) {
     // Note this should only ever be called once.
     fillthread_waiter_ = promise->get_future();
 
-    // 100 msec poll rate. Note that in most cases, the write end of the pipe will be closed so
-    // select() will return; the polling is important only for weird cases like a background process
-    // launched in a command substitution.
-    constexpr uint64_t usec_per_msec = 1000;
-    uint64_t poll_usec = 100 * usec_per_msec;
-
     // Run our function to read until the receiver is closed.
     // It's OK to capture 'this' by value because 'this' waits for the promise in its dtor.
     fd_monitor_item_t item;
     item.fd = std::move(fd);
-    item.timeout_usec = poll_usec;
     item.callback = [this, promise](autoclose_fd_t &fd, item_wake_reason_t reason) {
         ASSERT_IS_BACKGROUND_THREAD();
         // Only check the shutdown flag if we timed out or were poked.