Skip to content

Commit 7a5a7e7

Browse files
committed
Avoid warnings in Ruby 3.1 when finalizer is called twice [fix #585]
1 parent 3fecab5 commit 7a5a7e7

File tree

3 files changed

+26
-1
lines changed

3 files changed

+26
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## Unreleased
22

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

56
## [2.10.0] 2023-02-07
67

lib/roo/tempdir.rb

+4-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ def finalize_tempdirs(object_id)
44
if @tempdirs && (dirs_to_remove = @tempdirs[object_id])
55
@tempdirs.delete(object_id)
66
dirs_to_remove.each do |dir|
7-
::FileUtils.remove_entry(dir)
7+
# Pass force=true to avoid an exception (and thus warnings in Ruby 3.1) if dir has
8+
# already been removed. This can occur when the finalizer is called both in a forked
9+
# child process and in the parent.
10+
::FileUtils.remove_entry(dir, true)
811
end
912
end
1013
end

test/helpers/test_accessing_files.rb

+21
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,27 @@ def test_finalize
3636
end
3737
end
3838

39+
def test_finalize_twice
40+
skip if defined? JRUBY_VERSION
41+
42+
instance = Class.new { include Roo::Tempdir }.new
43+
44+
tempdir = instance.make_tempdir(instance, "my_temp_prefix", nil)
45+
assert File.exist?(tempdir), "Expected #{tempdir} to initially exist"
46+
47+
pid = Process.fork do
48+
# Inside the forked process finalize does not affect the parent process, but does delete the
49+
# tempfile on disk
50+
instance.finalize_tempdirs(instance.object_id)
51+
end
52+
53+
Process.wait(pid)
54+
refute File.exist?(tempdir), "Expected #{tempdir} to have been cleaned up by child process"
55+
56+
instance.finalize_tempdirs(instance.object_id)
57+
refute File.exist?(tempdir), "Expected #{tempdir} to still have been cleaned up"
58+
end
59+
3960
def test_cleanup_on_error
4061
# NOTE: This test was occasionally failing because when it started running
4162
# other tests would have already added folders to the temp directory,

0 commit comments

Comments
 (0)