From 32ab316f2cafbf9ecd6e21ad2b54201e040f191a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Maillet?= <fmaillet@gmail.com>
Date: Tue, 1 Sep 2026 14:54:00 -0400
Subject: [PATCH] fix: release the GIL while covering a polygon
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

polygon_to_geohashes took `_py: Python` and never used it, so the entire
cover walk ran with the GIL held. The walk touches no Python objects once
__geo_interface__ has been read, and at fine precisions it runs for a long
time — verdun at precision 10 takes minutes — blocking every other thread in
the interpreter for the duration. Every other heavy function in this module
already detaches.

Read the geometry under the GIL as before, then detach for the walk.

The test runs two concurrent calls and compares against one. Without the fix
it reports 2.06x, i.e. fully serialised.

Squashed with:
- test: make the GIL test robust off a multi-core runner

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/lib.rs              |  7 +++--
 tests/test_geohasher.py | 60 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 0d62f17..a863625 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -528,7 +528,7 @@ fn extract_multipolygon(coordinates: &Bound<'_, PyAny>) -> PyResult<Vec<Polygon<
 
 #[pyfunction]
 fn polygon_to_geohashes(
-    _py: Python,
+    py: Python<'_>,
     py_polygon: Bound<'_, PyAny>,
     precision: usize,
     inner: bool,
@@ -569,7 +569,10 @@ fn polygon_to_geohashes(
         }
     };
 
-    polygons_to_geohashes(polygons, precision, inner)
+    // The cover walk touches no Python objects, and at fine precisions it runs for
+    // seconds — verdun at precision 10 takes minutes. Hold the GIL for the
+    // __geo_interface__ read above only, not for the geometry.
+    py.detach(|| polygons_to_geohashes(polygons, precision, inner))
         .map_err(|e| pyo3::exceptions::PyValueError::new_err(format!("{e:?}")))
 }
 
diff --git a/tests/test_geohasher.py b/tests/test_geohasher.py
index 1e08746..0434e8a 100644
--- a/tests/test_geohasher.py
+++ b/tests/test_geohasher.py
@@ -1,3 +1,4 @@
+import os
 import shapely
 import geohash_polygon
 from polygon_geohasher.polygon_geohasher import (
@@ -249,3 +250,62 @@ def test_hole(level, inner, polygon_hole):
     assert geohash_polygon.polygon_to_geohashes(
         polygon_hole, level, inner
     ) == polygon_to_geohashes_py(polygon_hole, level, inner)
+
+
+@pytest.mark.skipif(
+    (os.cpu_count() or 1) < 2,
+    reason="two CPU-bound threads cannot overlap on a single core, GIL or not",
+)
+def test_polygon_to_geohashes_releases_the_gil(polygon_verdun):
+    """The cover walk must not hold the GIL.
+
+    Two concurrent calls should overlap rather than serialise. If the GIL were
+    held for the whole walk, the pair would take about twice a single call.
+    """
+    import threading
+    import time
+
+    precision = 9
+    # One call is short enough that thread start-up and scheduler noise move the
+    # ratio by more than the effect being measured. Repeating inside each timing
+    # makes the walk itself dominate.
+    repeats = 10
+
+    def cover():
+        for _ in range(repeats):
+            geohash_polygon.polygon_to_geohashes(polygon_verdun, precision, False)
+
+    # Warm up so neither timing pays one-off costs.
+    cover()
+
+    start = time.perf_counter()
+    cover()
+    single = time.perf_counter() - start
+
+    # Threads start one at a time, so without a gate the first would have a head
+    # start on the second and the pair would look more overlapped than it is.
+    gate = threading.Barrier(3)
+    failures = []
+
+    def work():
+        gate.wait()
+        try:
+            cover()
+        except Exception as exc:  # threading swallows these otherwise
+            failures.append(exc)
+
+    threads = [threading.Thread(target=work) for _ in range(2)]
+    for t in threads:
+        t.start()
+    gate.wait()
+    start = time.perf_counter()
+    for t in threads:
+        t.join()
+    concurrent = time.perf_counter() - start
+    assert not failures, f"worker thread raised: {failures[0]!r}"
+
+    # Serialised would be ~2.0x. Allow generous headroom for a loaded machine.
+    assert concurrent < single * 1.7, (
+        f"two concurrent calls took {concurrent:.3f}s against {single:.3f}s for one "
+        f"({concurrent / single:.2f}x) — the GIL looks held for the duration"
+    )