roo-rb / roo

Roo provides an interface to spreadsheets of several sorts.
MIT License
2.78k stars 501 forks source link

Tempfile cleanup causes warning on Ruby 3.1 when using Spring (or other forking preloader) #585

Closed owst closed 1 year ago

owst commented 1 year ago

Issue summary

When using the Spring preloader any spreadsheets that are opened by Roo in the parent process before forking are GC'd in the parent and child processes and thus the finalizer runs multiple times. ::FileUtils.remove_entry(dir) will raise if dir does not exist (e.g. on the second finalizer call). In Ruby 3.1 exceptions in finalizers are no longer silenced and cause a noisy warning, whereas in 3.0 and before there is no warning.

Reproduction

We don't actually need to use spring to reproduce, we can simply fork a single child process after reading a spreadsheet (one of the test spreadsheets from this repo):

require 'roo'

def log(msg)
  puts "#{Process.pid}: #{msg}"
end

# Monkeypatch to add logging...
module Roo
  module Tempdir
    def finalize_tempdirs(object_id)
      if @tempdirs && (dirs_to_remove = @tempdirs[object_id])
        @tempdirs.delete(object_id)
        dirs_to_remove.each do |dir|
          log "removing #{dir}, exists? #{File.exist?(dir)}" # Added this line
          ::FileUtils.remove_entry(dir)
        end
      end
    end
  end
end

xlsx = Roo::Excelx.new("/tmp/roo/test/files/simple_spreadsheet.xlsx")
sheet_data = xlsx.sheet("Sheet1").to_matrix.to_a

Process.fork do
  log "child"
end

log "sleeping in parent so child has exiting and deleted tmpdir..."
sleep 1

Using Ruby 3.0.5 gives output:

87478: sleeping in parent so child has exiting and deleted tmpdir...
87489: child
87489: removing /var/folders/dv/3xygzxf963b8948fkx1btt0w0000gn/T/roo_simple_spreadsheet.xlsx20230216-87478-7qchit, exists? true
87478: removing /var/folders/dv/3xygzxf963b8948fkx1btt0w0000gn/T/roo_simple_spreadsheet.xlsx20230216-87478-7qchit, exists? false

Whereas Ruby 3.1.3 gives output

87421: sleeping in parent so child has exiting and deleted tmpdir...
87433: child
87433: removing /var/folders/dv/3xygzxf963b8948fkx1btt0w0000gn/T/roo_simple_spreadsheet.xlsx20230216-87421-q5o4vy, exists? true
87421: removing /var/folders/dv/3xygzxf963b8948fkx1btt0w0000gn/T/roo_simple_spreadsheet.xlsx20230216-87421-q5o4vy, exists? false
repro.rb: warning: Exception in finalizer #<Proc:0x0000000109533b08 /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/gems/3.1.0/gems/roo-2.9.0/lib/roo/base.rb:34>
/Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:1446:in `unlink': No such file or directory @ apply2files - /var/folders/dv/3xygzxf963b8948fkx1btt0w0000gn/T/roo_simple_spreadsheet.xlsx20230216-87421-q5o4vy (Errno::ENOENT)
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:1446:in `block in remove_file'
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:1451:in `platform_support'
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:1445:in `remove_file'
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:1434:in `remove'
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:773:in `block in remove_entry'
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:1489:in `postorder_traverse'
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/3.1.0/fileutils.rb:771:in `remove_entry'
    from repro.rb:15:in `block in finalize_tempdirs'
    from repro.rb:13:in `each'
    from repro.rb:13:in `finalize_tempdirs'
    from /Users/owenstephens/.rbenv/versions/3.1.3/lib/ruby/gems/3.1.0/gems/roo-2.9.0/lib/roo/base.rb:34:in `block in finalize'

Note that in both cases the parent log says exists? false, however the resulting exception is swallowed in Ruby 3.0.5, but not in Ruby 3.1.3.