Skip to content

Commit

Permalink
Ensure finalizer order in Tempfile
Browse files Browse the repository at this point in the history
The Closer and Remover finalizers are defined on different objects in
Tempfile. The Closer is defined on the Tempfile object while the Remover
is defined on the finalizer_obj. This means that there is no guarantee
of the finalizer order.

On Windows, we must close the file before removing it because we cannot
remove an open file. But since the order is not guaranteed, the GC may
run the Remover finalizer first, which will fail with an Errno::EACCES
(Permission denied @ apply2files).

This commit changes it so that both the Closer and Remover finalizers
are defined on the finalizer_obj, which guarantees the order that it is
ran.
  • Loading branch information
peterzhu2118 committed Aug 15, 2024
1 parent ae896a5 commit 3f67d0a
Showing 1 changed file with 24 additions and 15 deletions.
39 changes: 24 additions & 15 deletions lib/tempfile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class Tempfile < DelegateClass(File)
# - Directory is the system temporary directory (system-dependent).
# - Generated filename is unique in that directory.
# - Permissions are <tt>0600</tt>;
# see {File Permissions}[https://docs.ruby-lang.org/en/master/File.html#label-File+Permissions].
# see {File Permissions}[rdoc-ref:File@File+Permissions].
# - Mode is <tt>'w+'</tt> (read/write mode, positioned at the end).
#
# The underlying file is removed when the \Tempfile object dies
Expand Down Expand Up @@ -207,12 +207,12 @@ class Tempfile < DelegateClass(File)
# Tempfile.new('foo', '.') # => #<Tempfile:./foo20220505-17839-xfstr8>
#
# Keyword arguments +mode+ and +options+ are passed directly to method
# {File.open}[https://docs.ruby-lang.org/en/master/File.html#method-c-open]:
# {File.open}[rdoc-ref:File.open]:
#
# - The value given with +mode+ must be an integer,
# and may be expressed as the logical OR of constants defined in
# {File::Constants}[https://docs.ruby-lang.org/en/master/File/Constants.html].
# - For +options+, see {Open Options}[https://docs.ruby-lang.org/en/master/IO.html#class-IO-label-Open+Options].
# {File::Constants}[rdoc-ref:File::Constants].
# - For +options+, see {Open Options}[rdoc-ref:IO@Open+Options].
#
# Related: Tempfile.create.
#
Expand All @@ -228,22 +228,25 @@ def initialize(basename="", tmpdir=nil, mode: 0, **options)
tmpfile = File.open(tmpname, @mode, **opts)
@opts = opts.freeze
end
ObjectSpace.define_finalizer(@finalizer_obj, Remover.new(tmpfile.path))
ObjectSpace.define_finalizer(self, Closer.new(tmpfile))

super(tmpfile)

define_finalizers
end

private def define_finalizers
ObjectSpace.define_finalizer(@finalizer_obj, Closer.new(__getobj__))
ObjectSpace.define_finalizer(@finalizer_obj, Remover.new(__getobj__.path))
end

def initialize_dup(other) # :nodoc:
initialize_copy_iv(other)
super(other)
ObjectSpace.define_finalizer(self, Closer.new(__getobj__))
end

def initialize_clone(other) # :nodoc:
initialize_copy_iv(other)
super(other)
ObjectSpace.define_finalizer(self, Closer.new(__getobj__))
end

private def initialize_copy_iv(other) # :nodoc:
Expand All @@ -256,10 +259,13 @@ def initialize_clone(other) # :nodoc:
# Opens or reopens the file with mode "r+".
def open
_close
ObjectSpace.undefine_finalizer(self)

mode = @mode & ~(File::CREAT|File::EXCL)
__setobj__(File.open(__getobj__.path, mode, **@opts))
ObjectSpace.define_finalizer(self, Closer.new(__getobj__))

ObjectSpace.undefine_finalizer(@finalizer_obj)
define_finalizers

__getobj__
end

Expand Down Expand Up @@ -327,7 +333,10 @@ def unlink
# may not be able to unlink on Windows; just ignore
return
end

ObjectSpace.undefine_finalizer(@finalizer_obj)
ObjectSpace.define_finalizer(@finalizer_obj, Closer.new(__getobj__))

@unlinked = true
end
alias delete unlink
Expand Down Expand Up @@ -453,11 +462,11 @@ def open(*args, **kw)
#
# With no block given and no arguments, creates and returns file whose:
#
# - Class is {File}[https://docs.ruby-lang.org/en/master/File.html] (not \Tempfile).
# - Class is {File}[rdoc-ref:File] (not \Tempfile).
# - Directory is the system temporary directory (system-dependent).
# - Generated filename is unique in that directory.
# - Permissions are <tt>0600</tt>;
# see {File Permissions}[https://docs.ruby-lang.org/en/master/File.html#label-File+Permissions].
# see {File Permissions}[rdoc-ref:File@File+Permissions].
# - Mode is <tt>'w+'</tt> (read/write mode, positioned at the end).
#
# The temporary file removal depends on the keyword argument +anonymous+ and
Expand Down Expand Up @@ -515,12 +524,12 @@ def open(*args, **kw)
# Tempfile.create('foo', '.') # => #<File:./foo20220505-9795-1emu6g8>
#
# Keyword arguments +mode+ and +options+ are passed directly to the method
# {File.open}[https://docs.ruby-lang.org/en/master/File.html#method-c-open]:
# {File.open}[rdoc-ref:File.open]:
#
# - The value given for +mode+ must be an integer
# and may be expressed as the logical OR of constants defined in
# {File::Constants}[https://docs.ruby-lang.org/en/master/File/Constants.html].
# - For +options+, see {Open Options}[https://docs.ruby-lang.org/en/master/IO.html#class-IO-label-Open+Options].
# {File::Constants}[rdoc-ref:File::Constants].
# - For +options+, see {Open Options}[rdoc-ref:IO@Open+Options].
#
# The keyword argument +anonymous+ specifies when the file is removed.
#
Expand Down

0 comments on commit 3f67d0a

Please sign in to comment.