Fix test_finalize test

`test_finalize` broke in ruby 2.4 because of changes in Ruby's GC. The
initial fix was to fork the process to make sure directories where being
cleaned properly.

Unfortunately, this caused a segfault when running the entire test suite.
The issue was related to where `ObjectSpace.define_finalizer` was being
called from (a class that was being extended by a module). The issue is
probably related to Nokogiri, but I wasn't able to develop a trivial
example of the error.

Creating the finalizer in the class fixed the issue in Ruby 2.4.0.

lib/roo/base.rb

@@ -22,6 +22,10 @@ def self.TEMP_PREFIX
     Roo::TEMP_PREFIX
   end
 
+  def self.finalize(object_id)
+    proc { finalize_tempdirs(object_id) }
+  end
+
   def initialize(filename, options = {}, _file_warning = :error, _tmpdir = nil)
     @filename = filename
     @options = options

lib/roo/excelx.rb

@@ -4,12 +4,11 @@
 require 'roo/tempdir'
 require 'roo/utils'
 require 'forwardable'
+require 'set'
 
 module Roo
   class Excelx < Roo::Base
     extend Roo::Tempdir
-
-    require 'set'
     extend Forwardable
 
     ERROR_VALUES = %w(#N/A #REF! #NAME? #DIV/0! #NULL! #VALUE! #NUM!).to_set
@@ -46,7 +45,13 @@ def initialize(filename_or_stream, options = {})
         basename = find_basename(filename_or_stream)
       end
 
+      # NOTE: Create temp directory and allow Ruby to cleanup the temp directory
+      #       when the object is garbage collected. Initially, the finalizer was
+      #       created in the Roo::Tempdir module, but that led to a segfault
+      #       when testing in Ruby 2.4.0.
       @tmpdir = self.class.make_tempdir(self, basename, options[:tmpdir_root])
+      ObjectSpace.define_finalizer(self, self.class.finalize(object_id))
+
       @shared = Shared.new(@tmpdir)
       @filename = local_filename(filename_or_stream, @tmpdir, packed)
       process_zipfile(@filename || filename_or_stream)

lib/roo/open_office.rb

@@ -22,7 +22,12 @@ def initialize(filename, options = {})
 
       @only_visible_sheets = options[:only_visible_sheets]
       file_type_check(filename, '.ods', 'an Roo::OpenOffice', file_warning, packed)
-      @tmpdir   = self.class.make_tempdir(self, find_basename(filename), options[:tmpdir_root])
+      # NOTE: Create temp directory and allow Ruby to cleanup the temp directory
+      #       when the object is garbage collected. Initially, the finalizer was
+      #       created in the Roo::Tempdir module, but that led to a segfault
+      #       when testing in Ruby 2.4.0.
+      @tmpdir = self.class.make_tempdir(self, find_basename(filename), options[:tmpdir_root])
+      ObjectSpace.define_finalizer(self, self.class.finalize(object_id))
       @filename = local_filename(filename, @tmpdir, packed)
       # TODO: @cells_read[:default] = false
       open_oo_file(options)

lib/roo/tempdir.rb

@@ -11,14 +11,9 @@ def finalize_tempdirs(object_id)
 
     def make_tempdir(object, prefix, root)
       root ||= ENV["ROO_TMP"]
-      # folder is cleaned up in .finalize_tempdirs
+      # NOTE: This folder is cleaned up by finalize_tempdirs.
       ::Dir.mktmpdir("#{Roo::TEMP_PREFIX}#{prefix}", root).tap do |tmpdir|
         @tempdirs ||= Hash.new { |h, k| h[k] = [] }
-
-        if @tempdirs[object.object_id].empty?
-          ObjectSpace.define_finalizer(object, method(:finalize_tempdirs))
-        end
-
         @tempdirs[object.object_id] << tmpdir
       end
     end

test/test_helper.rb

@@ -4,7 +4,6 @@
 require 'fileutils'
 require 'minitest/autorun'
 require 'shoulda'
-require 'fileutils'
 require 'timeout'
 require 'logger'
 require 'date'
@@ -14,7 +13,7 @@
 
 TESTDIR = File.join(File.dirname(__FILE__), 'files')
 TEST_RACK_PORT = (ENV["ROO_TEST_PORT"] || 5000).to_i
-TEST_URL= "http://0.0.0.0:#{TEST_RACK_PORT}"
+TEST_URL = "http://0.0.0.0:#{TEST_RACK_PORT}"
 
 # very simple diff implementation
 # output is an empty string if the files are equal

test/test_roo.rb

@@ -1495,13 +1495,13 @@ def test_close
   #       order to check if they were removed properly.
   def test_finalize
     read, write = IO.pipe
-    Process.fork do
+    pid = Process.fork do
       with_each_spreadsheet(name: "numbers1") do |oo|
         write.puts oo.instance_variable_get("@tmpdir")
       end
     end
 
-    Process.wait
+    Process.wait(pid)
     write.close
     tempdirs = read.read.split("\n")
     read.close