[ruby/tempfile] Fix Tempfile#{dup,clone}

Instead of storing the delegate in @tmpfile, use __getobj__, since
delegate library already handles dup/clone for that.  Copy the
unlinked, mode, and opts instance variables to the returned object
when using dup/clone.

Split the close/unlink finalizer into two finalizers. The close
finalizer always closes when any Tempfile instance is GCed, since
each Tempfile instance uses a separate file descriptor. The unlink
finalizer unlinks only when the original and all duped/cloned
Tempfiles are GCed, since all share the same path.

For Tempfile#open, undefine the close finalizer after closing the
current file, the redefine the close finalizer with the new file.

Fixes [Bug #19441]

https://github.com/ruby/tempfile/commit/dafabf9c7b

lib/tempfile.rb

@@ -152,26 +152,49 @@ class Tempfile < DelegateClass(File)
 
     @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
-      @tmpfile = File.open(tmpname, @mode, **opts)
+      tmpfile = File.open(tmpname, @mode, **opts)
       @opts = opts.freeze
     end
-    ObjectSpace.define_finalizer(self, Remover.new(@tmpfile))
+    ObjectSpace.define_finalizer(@finalizer_obj, Remover.new(tmpfile.path))
+    ObjectSpace.define_finalizer(self, Closer.new(tmpfile))
 
-    super(@tmpfile)
+    super(tmpfile)
+  end
+
+  def initialize_dup(other)
+    initialize_copy_iv(other)
+    super(other)
+    ObjectSpace.define_finalizer(self, Closer.new(__getobj__))
+  end
+
+  def initialize_clone(other)
+    initialize_copy_iv(other)
+    super(other)
+    ObjectSpace.define_finalizer(self, Closer.new(__getobj__))
+  end
+
+  private def initialize_copy_iv(other)
+    @unlinked = other.unlinked
+    @mode = other.mode
+    @opts = other.opts
+    @finalizer_obj = other.finalizer_obj
   end
 
   # Opens or reopens the file with mode "r+".
   def open
     _close
+    ObjectSpace.undefine_finalizer(self)
     mode = @mode & ~(File::CREAT|File::EXCL)
-    @tmpfile = File.open(@tmpfile.path, mode, **@opts)
+    __setobj__(File.open(__getobj__.path, mode, **@opts))
-    __setobj__(@tmpfile)
+    ObjectSpace.define_finalizer(self, Closer.new(__getobj__))
   end
 
   def _close    # :nodoc:
-    @tmpfile.close
+    __getobj__.close
   end
   protected :_close
 
@@ -228,13 +251,13 @@ class Tempfile < DelegateClass(File)
   def unlink
     return if @unlinked
     begin
-      File.unlink(@tmpfile.path)
+      File.unlink(__getobj__.path)
     rescue Errno::ENOENT
     rescue Errno::EACCES
       # may not be able to unlink on Windows; just ignore
       return
     end
-    ObjectSpace.undefine_finalizer(self)
+    ObjectSpace.undefine_finalizer(@finalizer_obj)
     @unlinked = true
   end
   alias delete unlink
@@ -242,43 +265,56 @@ class Tempfile < DelegateClass(File)
   # Returns the full path name of the temporary file.
   # This will be nil if #unlink has been called.
   def path
-    @unlinked ? nil : @tmpfile.path
+    @unlinked ? nil : __getobj__.path
   end
 
   # Returns the size of the temporary file.  As a side effect, the IO
   # buffer is flushed before determining the size.
   def size
-    if !@tmpfile.closed?
+    if !__getobj__.closed?
-      @tmpfile.size # File#size calls rb_io_flush_raw()
+      __getobj__.size # File#size calls rb_io_flush_raw()
     else
-      File.size(@tmpfile.path)
+      File.size(__getobj__.path)
     end
   end
   alias length size
 
   # :stopdoc:
   def inspect
-    if @tmpfile.closed?
+    if __getobj__.closed?
       "#<#{self.class}:#{path} (closed)>"
     else
       "#<#{self.class}:#{path}>"
     end
   end
 
-  class Remover # :nodoc:
+  protected
+
+  attr_reader :unlinked, :mode, :opts, :finalizer_obj
+
+  class Closer # :nodoc:
     def initialize(tmpfile)
-      @pid = Process.pid
       @tmpfile = tmpfile
     end
 
+    def call(*args)
+      @tmpfile.close
+    end
+  end
+
+  class Remover # :nodoc:
+    def initialize(path)
+      @pid = Process.pid
+      @path = path 
+    end
+
     def call(*args)
       return if @pid != Process.pid
 
-      $stderr.puts "removing #{@tmpfile.path}..." if $DEBUG
+      $stderr.puts "removing #{@path}..." if $DEBUG
 
-      @tmpfile.close
       begin
-        File.unlink(@tmpfile.path)
+        File.unlink(@path)
       rescue Errno::ENOENT
       end
 

test/test_tempfile.rb

@@ -63,6 +63,22 @@ class TestTempfile < Test::Unit::TestCase
     assert_match(/\.txt$/, File.basename(t.path))
   end
 
+  def test_dup
+    t = tempfile
+    t2 = t.dup
+    t2.close
+    assert_equal true, t2.closed?
+    assert_equal false, t.closed?
+  end
+
+  def test_clone
+    t = tempfile
+    t2 = t.clone
+    t2.close
+    assert_equal true, t2.closed?
+    assert_equal false, t.closed?
+  end
+
   def test_unlink
     t = tempfile("foo")
     path = t.path