From 39717d865dcb5b51236a091caa8fdb91b35b3c0a Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Mon, 4 May 2026 23:31:43 -0700 Subject: [PATCH 1/2] fix: Replace thread-isolated `ST_GeomFromKML` with SAVEPOINT to avoid Rails 8 deadlock `Importers::KML#geom_from_kml` ran each `ST_GeomFromKML` SQL in a freshly-spawned thread, manually checking out a separate connection from `ActiveRecord::Base.connection_pool`. The intent was fault isolation: if a single feature's KML was malformed and Postgres raised, the failure shouldn't poison the surrounding transaction. On Rails 7.x this worked because the test's transaction wasn't connection-pinned. Rails 7.1+ introduced per-test connection pinning under `use_transactional_fixtures = true`, and Rails 8.1 made the pin stricter. Now the spawned worker thread blocks indefinitely at `ConnectionPool#checkout` (it can't get a connection because the test thread has effectively claimed pool access), and the main thread blocks on `Thread#join`. Every KMZ-import controller spec hangs for as long as the test runner allows. Instead of crossing thread boundaries for fault containment, run the query inside a `transaction(requires_new: true)` SAVEPOINT on the existing connection. A `StatementInvalid` from a malformed feature rolls back only the savepoint; the surrounding transaction is unaffected. No second connection, no `Thread.new`, and no `.join` to deadlock on. KMZ specs now run in well under a second instead of timing out. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/spatial_features/importers/kml.rb | 31 +++++++++++++-------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/spatial_features/importers/kml.rb b/lib/spatial_features/importers/kml.rb index 03a4b9e..7b33f1c 100644 --- a/lib/spatial_features/importers/kml.rb +++ b/lib/spatial_features/importers/kml.rb @@ -56,24 +56,23 @@ def blank_feature?(feature) end def geom_from_kml(kml) - geom = nil - conn = nil - strip_altitude(kml) - # Do query in a new thread so we use a new connection (if the query fails it will poison the transaction of the current connection) - # - # We manually checkout a new connection since Rails re-uses DB connections across threads. - Thread.new do - conn = ActiveRecord::Base.connection_pool.checkout - geom = conn.select_value("SELECT ST_GeomFromKML(#{conn.quote(kml.to_s)})") - rescue ActiveRecord::StatementInvalid => e # Discard Invalid KML features - geom = nil - ensure - ActiveRecord::Base.connection_pool.checkin(conn) if conn - end.join - - return geom + # Run the parse inside a SAVEPOINT so a `ST_GeomFromKML` failure on a single + # invalid feature rolls back only that savepoint, not the surrounding + # transaction. The previous implementation spawned a thread and checked out + # a fresh connection for the same isolation reason, but on Rails 8 that + # pattern deadlocks under `use_transactional_fixtures` — the test pins its + # connection, the spawned thread blocks indefinitely waiting on + # `ActiveRecord::Base.connection_pool.checkout`. A nested transaction + # (`requires_new: true`) gives identical fault containment without crossing + # thread boundaries. + ActiveRecord::Base.transaction(requires_new: true) do + conn = ActiveRecord::Base.connection + conn.select_value("SELECT ST_GeomFromKML(#{conn.quote(kml.to_s)})") + end + rescue ActiveRecord::StatementInvalid # Discard invalid KML features + nil end def images_from_metadata(metadata) From b068483eab2e8a25b0926ffaabcd620b03104557 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Mon, 4 May 2026 23:42:50 -0700 Subject: [PATCH 2/2] Bump version to 3.10.1 --- lib/spatial_features/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spatial_features/version.rb b/lib/spatial_features/version.rb index fe32665..a79d9d3 100644 --- a/lib/spatial_features/version.rb +++ b/lib/spatial_features/version.rb @@ -1,3 +1,3 @@ module SpatialFeatures - VERSION = "3.10.0" + VERSION = "3.10.1" end