Skip to content

Commit

Permalink
Merge pull request #586 from owst/avoid_ruby_31_finalizer_warnings_wh…
Browse files Browse the repository at this point in the history
…en_called_twice

Avoid warnings in Ruby 3.1 when finalizer is called twice [fix #585]
  • Loading branch information
patrickkulling authored Jun 29, 2023
2 parents 3fecab5 + f14a1e2 commit 00850f1
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## Unreleased

### Changed/Added
- Prevent warnings on Ruby 3.1 if finalizer is called twice [586](https://github.com/roo-rb/roo/pull/586)

## [2.10.0] 2023-02-07

Expand Down
5 changes: 4 additions & 1 deletion lib/roo/tempdir.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ def finalize_tempdirs(object_id)
if @tempdirs && (dirs_to_remove = @tempdirs[object_id])
@tempdirs.delete(object_id)
dirs_to_remove.each do |dir|
::FileUtils.remove_entry(dir)
# Pass force=true to avoid an exception (and thus warnings in Ruby 3.1) if dir has
# already been removed. This can occur when the finalizer is called both in a forked
# child process and in the parent.
::FileUtils.remove_entry(dir, true)
end
end
end
Expand Down
21 changes: 21 additions & 0 deletions test/helpers/test_accessing_files.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,27 @@ def test_finalize
end
end

def test_finalize_twice
skip if defined? JRUBY_VERSION

instance = Class.new { include Roo::Tempdir }.new

tempdir = instance.make_tempdir(instance, "my_temp_prefix", nil)
assert File.exist?(tempdir), "Expected #{tempdir} to initially exist"

pid = Process.fork do
# Inside the forked process finalize does not affect the parent process's state, but does
# delete the tempfile on disk
instance.finalize_tempdirs(instance.object_id)
end

Process.wait(pid)
refute File.exist?(tempdir), "Expected #{tempdir} to have been cleaned up by child process"

instance.finalize_tempdirs(instance.object_id)
refute File.exist?(tempdir), "Expected #{tempdir} to still have been cleaned up"
end

def test_cleanup_on_error
# NOTE: This test was occasionally failing because when it started running
# other tests would have already added folders to the temp directory,
Expand Down

0 comments on commit 00850f1

Please sign in to comment.