Kill the forked agent run if it outlives runtimeout

lib/puppet/agent.rb

@@ -146,8 +146,55 @@ def run_in_fork(forking = true)
       atForkHandler.parent
     end
 
-    exit_code = Process.waitpid2(child_pid)
-    exit_code[1].exitstatus
+    _, status = wait_for_child(child_pid)
+    status.exitstatus
+  end
+
+  # Wait for the forked child to exit. The child enforces `runtimeout` on its
+  # own run, but it may fail to exit afterwards (a stranded thread, a hung
+  # subprocess, etc.). Without a deadline here, the parent blocks forever and
+  # the agent stops checking in. Once the run timeout plus a grace period has
+  # elapsed, the child is killed so the daemon can carry on.
+  def wait_for_child(child_pid)
+    deadline = child_deadline
+    return Process.waitpid2(child_pid) unless deadline
+
+    loop do
+      result = Process.waitpid2(child_pid, Process::WNOHANG)
+      return result if result
+
+      if monotonic_now >= deadline
+        Puppet.err _("Agent run (pid %{pid}) did not exit within %{timeout} seconds of the run timeout, killing it") %
+                   { pid: child_pid, timeout: child_grace_period }
+        begin
+          Process.kill(:KILL, child_pid)
+        rescue Errno::ESRCH
+          # The child exited between the last check and the kill.
+        end
+        return Process.waitpid2(child_pid)
+      end
+
+      sleep 1
+    end
+  end
+
+  # After the run timeout fires inside the child, it may still need to send
+  # its report, which is bounded by the HTTP connect and read timeouts.
+  def child_grace_period
+    Puppet[:http_connect_timeout] + Puppet[:http_read_timeout]
+  end
+
+  # Returns the absolute monotonic deadline for the child, or nil when
+  # `runtimeout` is disabled.
+  def child_deadline
+    runtimeout = Puppet[:runtimeout]
+    return nil if runtimeout.nil? || runtimeout <= 0
+
+    monotonic_now + runtimeout + child_grace_period
+  end
+
+  def monotonic_now
+    Process.clock_gettime(Process::CLOCK_MONOTONIC)
   end
 
   private

spec/unit/agent_spec.rb

@@ -315,6 +315,60 @@ def controlled_run(&block)
       end
     end
 
+    describe "when waiting for the forked child", :if => Puppet.features.posix? && RUBY_PLATFORM != 'java' do
+      let(:child_pid) { 1234 }
+      let(:status) { instance_double(Process::Status, exitstatus: 3) }
+
+      before do
+        @agent = Puppet::Agent.new(AgentTestClient, true)
+        allow(Kernel).to receive(:fork).and_return(child_pid)
+        allow(@agent).to receive(:sleep)
+      end
+
+      it "returns the child's exit status when it exits before the deadline" do
+        expect(Process).to receive(:waitpid2).with(child_pid, Process::WNOHANG).and_return(nil, [child_pid, status])
+        expect(Process).not_to receive(:kill)
+
+        expect(@agent.run_in_fork { 0 }).to eq(3)
+      end
+
+      it "blocks without a deadline when runtimeout is disabled" do
+        Puppet[:runtimeout] = 0
+
+        expect(Process).to receive(:waitpid2).with(child_pid).and_return([child_pid, status])
+        expect(Process).not_to receive(:kill)
+
+        expect(@agent.run_in_fork { 0 }).to eq(3)
+      end
+
+      it "kills the child once runtimeout plus the grace period has elapsed" do
+        Puppet[:runtimeout] = 10
+        Puppet[:http_connect_timeout] = 5
+        Puppet[:http_read_timeout] = 5
+        # now, first deadline check, second deadline check
+        allow(Process).to receive(:clock_gettime).with(Process::CLOCK_MONOTONIC).and_return(100, 105, 120)
+
+        expect(Process).to receive(:waitpid2).with(child_pid, Process::WNOHANG).twice.and_return(nil)
+        expect(Process).to receive(:kill).with(:KILL, child_pid)
+        expect(Process).to receive(:waitpid2).with(child_pid).and_return([child_pid, instance_double(Process::Status, exitstatus: nil)])
+        expect(Puppet).to receive(:err).with(/did not exit within 10 seconds of the run timeout/)
+
+        expect(@agent.run_in_fork { 0 }).to be_nil
+      end
+
+      it "reaps the child if it exits between the deadline check and the kill" do
+        Puppet[:runtimeout] = 10
+        allow(Process).to receive(:clock_gettime).with(Process::CLOCK_MONOTONIC).and_return(100, 100_000)
+
+        expect(Process).to receive(:waitpid2).with(child_pid, Process::WNOHANG).and_return(nil)
+        expect(Process).to receive(:kill).with(:KILL, child_pid).and_raise(Errno::ESRCH)
+        expect(Process).to receive(:waitpid2).with(child_pid).and_return([child_pid, status])
+        allow(Puppet).to receive(:err)
+
+        expect(@agent.run_in_fork { 0 }).to eq(3)
+      end
+    end
+
     describe "on Windows", :if => Puppet::Util::Platform.windows? do
       it "should never fork" do
         agent = Puppet::Agent.new(AgentTestClient, true)