Define Closer finalizer on dup and clone #33
Conversation
As @jeremyevans pointed out: > Each Tempfile instance has a separate File instance and file descriptor: > > t = Tempfile.new > t.to_i # => 6 > t.dup.to_i => 7 This commit adds the Closer finalizer on dup and clone so that the File object is closed on GC.
jeremyevans
left a comment
jeremyevans
left a comment
There was a problem hiding this comment.
I don't think this is correct. This gives every Tempfile dup/clone it's own @finalizer_obj, which will break the Remover, which is only supposed to remove the Tempfile path when all related Tempfile instances have stopped using it. This was handled previously by all dup/clone instances having a reference to the same @finalizer_obj.
See dafabf9 and https://bugs.ruby-lang.org/issues/19441 for the reasons behind the Remover/Closer split.
Maybe to handle Windows, we revert eb2d8b1, and on Windows we have the Remover do nothing (or do not add a finalizer in that case), and the Closer close and then unlink, and just swallow both ENOENT and EACCES for unlink. That should fix the finalizer order issue.
|
I took a different approach in #34 that I think fixes all the issues. |
As @jeremyevans pointed out:
This commit adds the Closer finalizer on dup and clone so that the File object is closed on GC.