Merge pull request #532 from sparklemotion/flavorjones-improve-gvl-test

test: improve the "busy handler timeout" GVL test

rakelib/test.rake

@@ -1,11 +1,17 @@
-require "rake/testtask"
+require "minitest/test_task"
 test_config = lambda do |t|
   t.libs << "test"
   t.libs << "lib"
-  t.test_files = FileList["test/**/test_*.rb"]
+
+  glob = "test/**/test_*.rb"
+  if t.respond_to?(:test_files=)
+    t.test_files = FileList[glob] # Rake::TestTask (RubyMemcheck)
+  else
+    t.test_globs = [glob] # Minitest::TestTask
+  end
 end
 
-Rake::TestTask.new(:test, &test_config)
+Minitest::TestTask.create(:test, &test_config)
 
 begin
   require "ruby_memcheck"

test/test_integration_pending.rb

@@ -17,13 +17,13 @@ def send_to_main state
       @thread_to_main.push state
     end
 
-    def wait_for_thread expected_state
-      state = @thread_to_main.pop
+    def wait_for_thread expected_state, non_block = false
+      state = @thread_to_main.pop(non_block)
       raise "Invalid state #{state}. #{expected_state} is expected" if state != expected_state
     end
 
-    def wait_for_main expected_state
-      state = @main_to_thread.pop
+    def wait_for_main expected_state, non_block = false
+      state = @main_to_thread.pop(non_block)
       raise "Invalid state #{state}. #{expected_state} is expected" if state != expected_state
     end
 
@@ -34,6 +34,11 @@ def close_thread
     def close_main
       @main_to_thread.close
     end
+
+    def close
+      close_thread
+      close_main
+    end
   end
 
   def setup
@@ -75,12 +80,11 @@ def test_busy_handler_impatient
     assert_raise(SQLite3::BusyException) do
       @db.execute "insert into foo (b) values ( 'from 2' )"
     end
+    assert_equal 1, handler_call_count
 
     synchronizer.send_to_thread :end_1
     synchronizer.close_main
     t.join
-
-    assert_equal 1, handler_call_count
   end
 
   def test_busy_timeout
@@ -97,55 +101,83 @@ def test_busy_timeout
       db2&.close
       sync.close_thread
     end
-
     synchronizer.wait_for_thread :ready_0
+
     time = Benchmark.measure do
       assert_raise(SQLite3::BusyException) do
         @db.execute "insert into foo (b) values ( 'from 2' )"
       end
     end
+    assert_operator time.real * 1000, :>=, 1000
 
     synchronizer.send_to_thread :end_1
     synchronizer.close_main
     t.join
-
-    assert_operator time.real * 1000, :>=, 1000
   end
 
   def test_busy_handler_timeout_releases_gvl
-    work = []
+    @db.busy_handler_timeout = 100
 
-    Thread.new do
-      loop do
-        sleep 0.1
-        work << "."
-      end
-    end
-    sleep 1
+    t1sync = ThreadSynchronizer.new
+    t2sync = ThreadSynchronizer.new
 
-    @db.busy_handler_timeout = 1000
     busy = Mutex.new
     busy.lock
 
-    t = Thread.new do
+    count = 0
+    active_thread = Thread.new(t1sync) do |sync|
+      sync.send_to_main :ready
+      sync.wait_for_main :start
+
+      loop do
+        sleep 0.005
+        count += 1
+        begin
+          sync.wait_for_main :end, true
+          break
+        rescue ThreadError
+        end
+      end
+      sync.send_to_main :done
+    end
+
+    blocking_thread = Thread.new(t2sync) do |sync|
       db2 = SQLite3::Database.open("test.db")
       db2.transaction(:exclusive) do
+        sync.send_to_main :ready
         busy.lock
       end
+      sync.send_to_main :done
     ensure
       db2&.close
     end
-    sleep 1
 
-    work << "|"
+    t1sync.wait_for_thread :ready
+    t2sync.wait_for_thread :ready
+
+    t1sync.send_to_thread :start
     assert_raises(SQLite3::BusyException) do
       @db.execute "insert into foo (b) values ( 'from 2' )"
     end
+    t1sync.send_to_thread :end
 
     busy.unlock
-    t.join
-
-    assert_operator work.size - work.find_index("|"), :>, 3
+    t2sync.wait_for_thread :done
+
+    expected = if RUBY_PLATFORM.include?("linux")
+      # 20 is the theoretical max if timeout is 100ms and active thread sleeps 5ms
+      15
+    else
+      # in CI, macos and windows systems seem to really not thread very well, so let's set a lower bar.
+      2
+    end
+    assert_operator(count, :>=, expected)
+  ensure
+    active_thread&.join
+    blocking_thread&.join
+
+    t1sync&.close
+    t2sync&.close
   end
 
   def test_busy_handler_outwait
@@ -163,6 +195,7 @@ def test_busy_handler_outwait
       db2&.close
       sync.close_thread
     end
+    synchronizer.wait_for_thread :ready_0
 
     @db.busy_handler do |count|
       handler_call_count += 1
@@ -171,14 +204,12 @@ def test_busy_handler_outwait
       true
     end
 
-    synchronizer.wait_for_thread :ready_0
     assert_nothing_raised do
       @db.execute "insert into foo (b) values ( 'from 2' )"
     end
+    assert_equal 1, handler_call_count
 
     synchronizer.close_main
     t.join
-
-    assert_equal 1, handler_call_count
   end
 end