Skip to content

Commit

Permalink
Add FinalizerManager to manage finalizers
Browse files Browse the repository at this point in the history
As @jeremyevans pointed out for commit eb2d8b1:

> Each Tempfile instance has a separate File instance and file descriptor:
>
>   t = Tempfile.new
>   t.to_i # => 6
>   t.dup.to_i => 7

FinalizerManager will keep track of the open File objects for the
particular file and will only unlink the file when all of the File objects
have been closed.
  • Loading branch information
peterzhu2118 committed Aug 20, 2024
1 parent 37bee10 commit fb4da22
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 35 deletions.
58 changes: 24 additions & 34 deletions lib/tempfile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ def initialize(basename="", tmpdir=nil, mode: 0, **options)

@unlinked = false
@mode = mode|File::RDWR|File::CREAT|File::EXCL
@finalizer_obj = Object.new
tmpfile = nil
::Dir::Tmpname.create(basename, tmpdir, **options) do |tmpname, n, opts|
opts[:perm] = 0600
Expand All @@ -231,29 +230,29 @@ def initialize(basename="", tmpdir=nil, mode: 0, **options)

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))
@finalizer_obj = Object.new
@finalizer_manager = FinalizerManager.new(__getobj__.path)
@finalizer_manager.register(@finalizer_obj, __getobj__)
end

def initialize_dup(other) # :nodoc:
initialize_copy_iv(other)
super(other)
@finalizer_manager.register(@finalizer_obj, __getobj__)
end

def initialize_clone(other) # :nodoc:
initialize_copy_iv(other)
super(other)
@finalizer_manager.register(@finalizer_obj, __getobj__)
end

private def initialize_copy_iv(other) # :nodoc:
@unlinked = other.unlinked
@mode = other.mode
@opts = other.opts
@finalizer_obj = other.finalizer_obj
@finalizer_obj = Object.new
@finalizer_manager = other.finalizer_manager
end

# Opens or reopens the file with mode "r+".
Expand All @@ -263,8 +262,7 @@ def open
mode = @mode & ~(File::CREAT|File::EXCL)
__setobj__(File.open(__getobj__.path, mode, **@opts))

ObjectSpace.undefine_finalizer(@finalizer_obj)
define_finalizers
@finalizer_manager.register(@finalizer_obj, __getobj__)

__getobj__
end
Expand Down Expand Up @@ -334,9 +332,6 @@ def unlink
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 @@ -370,35 +365,30 @@ def inspect

protected

attr_reader :unlinked, :mode, :opts, :finalizer_obj

class Closer # :nodoc:
def initialize(tmpfile)
@tmpfile = tmpfile
end

def call(*args)
@tmpfile.close
end
end
attr_reader :unlinked, :mode, :opts, :finalizer_manager

class Remover # :nodoc:
class FinalizerManager # :nodoc:
def initialize(path)
@pid = Process.pid
@open_files = {}
@path = path
@pid = Process.pid
end

def call(*args)
return if @pid != Process.pid
def register(obj, file)
ObjectSpace.undefine_finalizer(obj)
ObjectSpace.define_finalizer(obj, self)
@open_files[obj.object_id] = file
end

$stderr.puts "removing #{@path}..." if $DEBUG
def call(object_id)
@open_files.delete(object_id).close

begin
File.unlink(@path)
rescue Errno::ENOENT
if @open_files.empty? && Process.pid == @pid
begin
File.unlink(@path)
rescue Errno::ENOENT
end
end

$stderr.puts "done" if $DEBUG
end
end

Expand Down
35 changes: 34 additions & 1 deletion test/test_tempfile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require 'tempfile'

class TestTempfile < Test::Unit::TestCase
LIB_TEMPFILE_RB_PATH = File.expand_path(__dir__ + "/../lib/tempfile.rb")

def initialize(*)
super
@tempfile = nil
Expand Down Expand Up @@ -172,6 +174,38 @@ def test_close_bang_does_not_unlink_if_already_unlinked
end
end unless /mswin|mingw/ =~ RUBY_PLATFORM

def test_finalizer_removes_file
assert_in_out_err(["-r#{LIB_TEMPFILE_RB_PATH}"], <<~RUBY) do |(filename,*), (error,*)|
file = Tempfile.new("foo")
puts file.path
RUBY
assert_file.not_exist?(filename)
assert_nil error
end
end

def test_finalizer_removes_file_when_dup
assert_in_out_err(["-r#{LIB_TEMPFILE_RB_PATH}"], <<~RUBY) do |(filename,*), (error,*)|
file = Tempfile.new("foo")
file.dup
puts file.path
RUBY
assert_file.not_exist?(filename)
assert_nil error
end
end

def test_finalizer_removes_file_when_clone
assert_in_out_err(["-r#{LIB_TEMPFILE_RB_PATH}"], <<~RUBY) do |(filename,*), (error,*)|
file = Tempfile.new("foo")
file.clone
puts file.path
RUBY
assert_file.not_exist?(filename)
assert_nil error
end
end

def test_finalizer_does_not_unlink_if_already_unlinked
assert_in_out_err('-rtempfile', <<-'EOS') do |(filename,*), (error,*)|
file = Tempfile.new('foo')
Expand Down Expand Up @@ -474,5 +508,4 @@ def test_create_anonymous_autoclose
assert_equal(true, t.autoclose?)
}
end

end

0 comments on commit fb4da22

Please sign in to comment.