From af24e23b2fd4deb33a0f14d9811a35062afe3af2 Mon Sep 17 00:00:00 2001
From: ybai08 <271969568+ybai08@users.noreply.github.com>
Date: Sat, 17 Oct 2026 21:34:34 -0500
Subject: [PATCH] Delete subprocess output tempfiles once they are read

`Subprocess.spawn` captured the child's stdout and stderr in two
Tempfiles and never closed them, so they were deleted by the garbage
collector whenever it got round to it. On Windows that delete fails with
`Errno::EACCES` if a process left behind by the command still holds the
file open, and Ruby prints a backtrace from the finalizer on every hook
run.

Close and unlink both files in an `ensure` once the output has been
read, so they are removed when `spawn` returns, including when reading
fails.

Fixes #899
---
 lib/overcommit/subprocess.rb       |  5 ++++
 spec/overcommit/subprocess_spec.rb | 47 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/lib/overcommit/subprocess.rb b/lib/overcommit/subprocess.rb
index 88ef362e..8493efab 100644
--- a/lib/overcommit/subprocess.rb
+++ b/lib/overcommit/subprocess.rb
@@ -50,6 +50,11 @@ def spawn(args, options = {})
         out.rewind
 
         Result.new(process.exit_code, out.read, err.read)
+      ensure
+        # Delete the output files now rather than leaving it to the garbage
+        # collector, which on Windows fails (and prints a backtrace) if a process
+        # left behind by the command still has them open.
+        [out, err].each { |stream| stream&.close! }
       end
 
       # Spawns a new process in the background using the given array of
diff --git a/spec/overcommit/subprocess_spec.rb b/spec/overcommit/subprocess_spec.rb
index 07503d25..4a11f7b5 100644
--- a/spec/overcommit/subprocess_spec.rb
+++ b/spec/overcommit/subprocess_spec.rb
@@ -106,6 +106,53 @@ def received_argv(result)
         received_argv(subject).should == ['', 'after']
       end
     end
+
+    context 'once the process has finished' do
+      # Regression test for https://github.com/sds/overcommit/issues/899, where
+      # the temporary files capturing the output were left for the garbage
+      # collector to delete. On Windows that delete fails with `Errno::EACCES`
+      # when a process left behind by the command still holds the file open,
+      # and Ruby prints a backtrace from the finalizer on every hook run.
+      let(:tempfiles) { [] }
+
+      # Records every Tempfile created, passing each one to the block (if given)
+      # so a test can stub it.
+      def record_tempfiles(&block)
+        ::Tempfile.stub(:new).and_wrap_original do |original, *args|
+          original.call(*args).tap do |tempfile|
+            tempfiles << tempfile
+            block&.call(tempfile)
+          end
+        end
+      end
+
+      def expect_tempfiles_deleted
+        tempfiles.size.should == 2
+        tempfiles.each do |tempfile|
+          tempfile.should be_closed
+          tempfile.path.should be_nil # Tempfile#path is nil once unlinked
+        end
+      end
+
+      it 'closes and deletes the files that captured its output' do
+        record_tempfiles
+
+        result = described_class.spawn([ruby, '-e', 'STDOUT.print "out"; STDERR.print "err"'])
+
+        result.stdout.should == 'out'
+        result.stderr.should == 'err'
+        expect_tempfiles_deleted
+      end
+
+      it 'closes and deletes the files when reading the output fails' do
+        record_tempfiles { |tempfile| tempfile.stub(:read).and_raise(IOError, 'read failed') }
+
+        expect { described_class.spawn([ruby, '-e', 'STDOUT.print "out"']) }.
+          to raise_error(IOError, 'read failed')
+
+        expect_tempfiles_deleted
+      end
+    end
   end
 
   # Subprocess no longer branches on platform, but this guards against