aruba

aruba copied to clipboard

Summary

Explictly call #unlink on Tempfiles used in SpawnProcess, instead of waiting for the finalizer.

Details

Adds a call to #unlink right after reading the Tempfile and storing the contents in a variable.

Motivation and Context

On Windows, the finalizer for Tempfile will raise an error if Errno::ENOACCES is raised when removing the file. However, if this happens when #unlink is called, the error is caught.

This change calls #unlink on the Tempfiles that are used to store the output of spawned processes. This means the finalizer will not need to unlink the file, so any Errno::ENOACCES errors during unlinking will be caught.

The raised errors did not make any specs fail, but they did appear in the rspec output.

How Has This Been Tested?

I ran the specs. In CI, we will see if the backtraces are gone.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Documentation update
• [ ] Internal change (refactoring, test improvements, developer experience or update of dependencies)

Checklist:

• [ ] I've added tests for my code
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.

Good theory but this doesn't help 😞

Here's the #unlink method, and it is now clear why this PR doesn't help:

  def unlink
    return if @unlinked
    begin
      File.unlink(@tmpfile.path)
    rescue Errno::ENOENT
    rescue Errno::EACCES
      # may not be able to unlink on Windows; just ignore
      return
    end
    ObjectSpace.undefine_finalizer(self)
    @unlinked = true
  end

If unlink fails due to Errno::EACCES, the finalizer is not removed, and that finalizer will try again to unlink the Tempfile. As a result, Errno::EACCES errors will still occur there and cause backtraces to be printed.

This method also has the following comment:

  # However, unlink-before-close may not be supported on non-POSIX operating
  # systems. Microsoft Windows is the most notable case: unlinking a non-closed
  # file will result in an error, which this method will silently ignore. If
  # you want to practice unlink-before-close whenever possible, then you should
  # write code like this:

So the Errno::EACCES error is expected to come from the Tempfile still being open.

(Good investigation!)