Don't sleep on IO.select once every pipe has closed

cspell.json

@@ -152,6 +152,7 @@
     "certstore",
     "CFPREFERENCES",
     "cfprefsd",
+    "cgroupv",
     "chaput",
     "chardev",
     "chatops",
@@ -333,6 +334,7 @@
     "dscacheutil",
     "dscresource",
     "dslocal",
+    "ducktype",
     "DUPEUX",
     "DWORDLONG",
     "DYNALINK",
@@ -358,6 +360,7 @@
     "encap",
     "Encryptor",
     "encryptor",
+    "endgrent",
     "endlocal",
     "entriesread",
     "envdata",
@@ -451,6 +454,7 @@
     "GETFD",
     "GETFL",
     "getgr",
+    "getgrent",
     "getgrgid",
     "getgrnam",
     "gethostbyname",
@@ -704,6 +708,7 @@
     "loginclass",
     "loginwindow",
     "LOGLOCATION",
+    "LOGNAME",
     "logopts",
     "logstring",
     "LONGLONG",
@@ -1017,6 +1022,7 @@
     "PFILETIME",
     "PFLOAT",
     "PGENERICMAPPING",
+    "pgid",
     "phabricator",
     "PHALF",
     "PHANDLE",
@@ -1245,6 +1251,8 @@
     "Scriptable",
     "SCROLLBAR",
     "SCROLLBARS",
+    "secondarygroups",
+    "seconderies",
     "secontext",
     "secoption",
     "secopts",
@@ -1291,6 +1299,7 @@
     "SETTINGCHANGE",
     "setuid",
     "SETX",
+    "sgids",
     "SHARENAME",
     "SHAs",
     "shas",
@@ -1626,6 +1635,7 @@
     "WINVER",
     "WKSTA",
     "WMIGUID",
+    "WNOHANG",
     "woot",
     "workdir",
     "WPARAM",

lib/mixlib/shellout.rb

@@ -26,6 +26,10 @@ module Mixlib
 
   class ShellOut
     READ_WAIT_TIME = 0.01
+    # Initial interval for polling the child's exit status once every pipe has
+    # hit EOF and there is nothing left to select on. Backs off toward
+    # READ_WAIT_TIME. See Unix#run_command.
+    REAP_WAIT_TIME = 0.0005
     READ_SIZE = 4096
     DEFAULT_READ_TIMEOUT = 600
 

lib/mixlib/shellout/unix.rb

@@ -107,12 +107,31 @@ def run_command
 
         write_to_child_stdin
 
+        reap_wait = REAP_WAIT_TIME
+
         until @status
-          ready_buffers = attempt_buffer_read
-          unless ready_buffers
-            @execution_time += READ_WAIT_TIME
+          if open_pipes.empty?
+            # Every pipe has hit EOF, so IO.select has nothing left to wait on
+            # and would just sleep away a whole READ_WAIT_TIME. The only thing
+            # left to do is reap the child, and a child that has closed all of
+            # its descriptors is normally already on its way out, so poll for
+            # it much more finely than that.
+            #
+            # Back off toward READ_WAIT_TIME as we go, so a child that closed
+            # its descriptors but kept running -- one that detaches itself, say --
+            # settles back to the old polling rate instead of spinning the CPU
+            # for the rest of the timeout.
+            sleep reap_wait
+            waited = reap_wait
+            reap_wait = [reap_wait * 2, READ_WAIT_TIME].min
+          else
+            waited = attempt_buffer_read ? nil : READ_WAIT_TIME
+          end
+
+          if waited
+            @execution_time += waited
             if @execution_time >= timeout && !@result
-              # kill the bad proccess
+              # kill the bad process
               reap_errant_child
               # read anything it wrote when we killed it
               attempt_buffer_read

spec/mixlib/shellout_spec.rb

@@ -1334,6 +1334,36 @@ def ruby_wo_shell(code)
 
           end
 
+          # Closing both descriptors leaves open_pipes empty, so run_command
+          # has nothing left to select on and is only waiting to reap. Note
+          # this has to be done by the shell -- Ruby's STDOUT.close does not
+          # close the underlying descriptor, so the pipe never sees EOF.
+          context "and the child closes stdout and stderr but keeps running" do
+            let(:cmd) { [ "sh", "-c", "exec 1>&- 2>&-; sleep 30" ] }
+
+            it "should still time out" do
+              # note: let blocks don't correctly memoize if an exception is raised,
+              # so can't use executed_cmd
+              expect { shell_cmd.run_command }.to raise_error(Mixlib::ShellOut::CommandTimeout)
+              expect(shell_cmd.execution_time).to be >= 1
+            end
+
+            it "should back off rather than spin while waiting to reap it" do
+              # With nothing to select on we poll for the child's exit status,
+              # and that poll has to back off -- otherwise a child which never
+              # exits spins at REAP_WAIT_TIME for the whole timeout. One reap
+              # attempt per loop, so cap them near the pre-existing rate.
+              reaps = 0
+              allow(shell_cmd).to receive(:attempt_reap).and_wrap_original do |original|
+                reaps += 1
+                original.call
+              end
+
+              expect { shell_cmd.run_command }.to raise_error(Mixlib::ShellOut::CommandTimeout)
+              expect(reaps).to be < 2 * (1 / Mixlib::ShellOut::READ_WAIT_TIME)
+            end
+          end
+
         end
       end