fix: suspend SQLite busy waits only when restorable

The adapter reads the configured busy wait through the connection pool's database configuration. If a future Active Record release stops exposing it, restoration would silently fall back to writing a zero pragma and strip lock waiting from a pooled connection the whole application shares. Skip the suspension instead, which only loosens the synchronous deadline bound, and assert the timeout stays discoverable so the looser bound cannot be adopted unnoticed.

Author: cardmagic

CHANGELOG.md

@@ -8,7 +8,11 @@
   and silently replaces, so the previous save and restore left pooled
   connections with no busy handler at all. Every later writer on that
   connection, inside or outside Solid Objects, then failed immediately with
-  `SQLite3::BusyException` instead of waiting for the lock.
+  `SQLite3::BusyException` instead of waiting for the lock. Suspend the busy
+  wait only when the adapter can identify how to restore it, so an Active
+  Record release that stops exposing the configured timeout loosens
+  synchronous deadline bounds instead of stripping lock waiting from a shared
+  pooled connection.
 
 - Run the doctor round-trip probe on a dedicated caller process, and accept an
   explicit process registry in `SynchronousInvocation`, so the probe can no

docs/correctness.md

@@ -173,7 +173,15 @@ transaction and restores it afterwards. Restoration reinstalls the Ruby busy
 handler Rails configures from the sqlite3 `timeout` setting, which
 `PRAGMA busy_timeout` neither reports nor preserves, so a synchronous call
 leaves the connection's lock waiting behaviour exactly as it found it for
-later writers inside and outside Solid Objects. If enqueue cannot commit, `SyncEnqueueTimeout` is
+later writers inside and outside Solid Objects.
+
+The adapter suspends the busy wait only when it can identify how to restore
+it. When a future Active Record release stops exposing the configured
+timeout, the adapter leaves the connection untouched: synchronous deadlines
+lose their tight bound and wait as long as the configured busy wait allows,
+rather than stripping lock waiting from a pooled connection the rest of the
+application shares. A test asserts the timeout stays discoverable so the
+looser bound cannot be adopted silently. If enqueue cannot commit, `SyncEnqueueTimeout` is
 raised and no message reference exists. MySQL lock waits have one-second InnoDB
 granularity. Ruby handlers that already started are not preempted.
 

lib/solid_objects/database_adapters/sqlite.rb

@@ -67,34 +67,37 @@ def with_lock_probe
       def with_transaction_deadline(connection)
         return yield unless SyncDeadline.active?
 
-        busy_wait = suspend_busy_wait(connection)
+        busy_wait = restorable_busy_wait(connection)
+        return yield unless busy_wait
+
         begin
+          connection.execute("PRAGMA busy_timeout = 0")
           yield
         ensure
           restore_busy_wait(connection, busy_wait)
         end
       end
 
-      # @rbs (untyped) -> Hash[Symbol, untyped]
-      def suspend_busy_wait(connection)
-        busy_wait = {
-          pragma_timeout: connection.select_value("PRAGMA busy_timeout").to_i,
-          handler_timeout: configured_busy_handler_timeout(connection)
-        }
-        connection.execute("PRAGMA busy_timeout = 0")
-        busy_wait
+      # @rbs (untyped) -> Hash[Symbol, untyped]?
+      def restorable_busy_wait(connection)
+        pragma_timeout = connection.select_value("PRAGMA busy_timeout").to_i
+        return { pragma_timeout: } if pragma_timeout.positive?
+
+        handler_timeout = configured_busy_handler_timeout(connection)
+        return nil unless handler_timeout
+
+        { pragma_timeout:, handler_timeout: }
       end
 
       # @rbs (untyped, Hash[Symbol, untyped]) -> void
       def restore_busy_wait(connection, busy_wait)
-        handler_timeout = busy_wait.fetch(:handler_timeout)
-        pragma_timeout = busy_wait.fetch(:pragma_timeout)
-        if handler_timeout && pragma_timeout.zero?
+        handler_timeout = busy_wait[:handler_timeout]
+        if handler_timeout
           connection.raw_connection.busy_handler_timeout = handler_timeout
           return
         end
 
-        connection.execute("PRAGMA busy_timeout = #{pragma_timeout}")
+        connection.execute("PRAGMA busy_timeout = #{busy_wait.fetch(:pragma_timeout)}")
       end
 
       # @rbs (untyped) -> Integer?

sig/generated/lib/solid_objects/database_adapters/sqlite.rbs

@@ -26,8 +26,8 @@ module SolidObjects
       # @rbs (untyped) { () -> untyped } -> untyped
       def with_transaction_deadline: (untyped) { () -> untyped } -> untyped
 
-      # @rbs (untyped) -> Hash[Symbol, untyped]
-      def suspend_busy_wait: (untyped) -> Hash[Symbol, untyped]
+      # @rbs (untyped) -> Hash[Symbol, untyped]?
+      def restorable_busy_wait: (untyped) -> Hash[Symbol, untyped]?
 
       # @rbs (untyped, Hash[Symbol, untyped]) -> void
       def restore_busy_wait: (untyped, Hash[Symbol, untyped]) -> void

test/integration/synchronous_invocation_test.rb

@@ -502,6 +502,36 @@ def wait(timeout:)
     release_sqlite_write_lock(lock) if lock
   end
 
+  test "sync discovers the configured SQLite busy wait it has to restore" do
+    skip unless SolidObjects::Record.connection.adapter_name.match?(/sqlite/i)
+
+    SolidObjects::Record.connection_pool.with_connection do |connection|
+      discovered = SolidObjects
+        .database_adapter
+        .send(:configured_busy_handler_timeout, connection)
+
+      assert_equal configured_sqlite_busy_handler_timeout, discovered,
+        "the adapter can no longer read the configured busy wait, so it stops " \
+        "suspending lock waits and synchronous deadlines lose their bound"
+    end
+  end
+
+  test "sync leaves an unrestorable busy wait alone" do
+    skip unless SolidObjects::Record.connection.adapter_name.match?(/sqlite/i)
+    database_adapter = SolidObjects.database_adapter
+    database_adapter.define_singleton_method(:configured_busy_handler_timeout) { |_connection| nil }
+
+    SolidObjects::Record.connection_pool.with_connection do
+      CounterActor.ref("unrestorable").increment
+
+      assert_nothing_raised do
+        write_while_write_lock_is_briefly_held
+      end
+    end
+  ensure
+    database_adapter&.singleton_class&.send(:remove_method, :configured_busy_handler_timeout)
+  end
+
   test "sync restores the SQLite busy handler it suspended for the deadline" do
     skip unless SolidObjects::Record.connection.adapter_name.match?(/sqlite/i)