From b9d646c46e4cc5d1e324dd00561db31db38727c7 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 3 Oct 2026 05:12:39 +0000 Subject: [PATCH 1/8] Release the read lock before building the network ReaderIter_next held the read lock while it called ipaddress.ip_network, which runs Python code. That caused two hangs on free-threaded builds: - If the code closed the reader on the same thread, for example from a signal handler, close() waited for the write lock that the thread's own read lock blocked. - If the code ran the garbage collector while another thread waited in close() for the write lock, the collector stopped the world and waited for that thread, which waited for the read lock. The network uses only the iterator's own record, so release the lock after the record is decoded. Now no thread runs Python code while it holds the lock. A SIGALRM handler that closes the reader during iteration, and a wrapped ip_network that calls gc.collect() while another thread calls close(), both hung before this change and work after it. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 2 ++ extension/maxminddb.c | 22 ++++++++--------- tests/reader_test.py | 55 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index ae86d46..6795cb1 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -25,6 +25,8 @@ History Reinitializing a ``Metadata`` changes nothing. * Fixed a ``RuntimeWarning`` or ``RuntimeError`` on free-threaded Python on macOS when a ``Reader`` failed to open or was used without ``__init__``. + * Fixed a deadlock on free-threaded Python when a ``Reader`` was closed + during iteration, from another thread or from a signal handler. 3.2.0 (2026-09-10) ++++++++++++++++++ diff --git a/extension/maxminddb.c b/extension/maxminddb.c index 96679f1..b013a9e 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -907,6 +907,9 @@ static PyObject *ReaderIter_next(PyObject *self) { case MMDB_RECORD_TYPE_EMPTY: break; case MMDB_RECORD_TYPE_DATA: { + // Read this before any Python code runs, which could close + // the reader. + uint16_t const depth = ri->reader->mmdb->depth; MMDB_entry_data_list_s *entry_data_list = NULL; int status = MMDB_get_entry_data_list(&cur->entry, &entry_data_list); @@ -926,15 +929,19 @@ static PyObject *ReaderIter_next(PyObject *self) { PyObject *record = from_entry_data_list(state, &entry_data_list); MMDB_free_entry_data_list(original_entry_data_list); + + // The rest uses only cur, which this call owns. Release the + // lock before ip_network runs Python code, which could close + // the reader on this thread. + reader_release_read_lock(ri->reader); if (record == NULL) { - reader_release_read_lock(ri->reader); free(cur); return NULL; } int ip_start = 0; Py_ssize_t ip_length = 4; - if (ri->reader->mmdb->depth == 128) { + if (depth == 128) { if (is_ipv6(cur->ip_packed)) { // IPv6 address ip_length = 16; @@ -948,37 +955,28 @@ static PyObject *ReaderIter_next(PyObject *self) { &(cur->ip_packed[ip_start]), ip_length, cur->depth - ip_start * 8); + free(cur); if (network_tuple == NULL) { - reader_release_read_lock(ri->reader); Py_DECREF(record); - free(cur); return NULL; } PyObject *args = PyTuple_Pack(1, network_tuple); Py_DECREF(network_tuple); if (args == NULL) { - reader_release_read_lock(ri->reader); Py_DECREF(record); - free(cur); return NULL; } PyObject *network = PyObject_CallObject(state->ipaddress_ip_network, args); Py_DECREF(args); if (network == NULL) { - reader_release_read_lock(ri->reader); Py_DECREF(record); - free(cur); return NULL; } PyObject *rv = PyTuple_Pack(2, network, record); Py_DECREF(network); Py_DECREF(record); - - reader_release_read_lock(ri->reader); - - free(cur); return rv; } default: diff --git a/tests/reader_test.py b/tests/reader_test.py index da91173..22e106f 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1271,6 +1271,61 @@ def test_freed_objects_release_their_type(self) -> None: iter(reader) self.assertEqual([sys.getrefcount(c) for c in classes], before) + def test_close_from_ip_network_during_iteration(self) -> None: + # The iterator calls ipaddress.ip_network, which can run Python code + # that closes the reader. If the iterator still held the read lock, + # close() would wait for it forever on free-threaded Python. Run in a + # subprocess with a timeout, and patch ip_network before the extension + # caches it. + program = textwrap.dedent( + """ + import ipaddress + import sys + + real_ip_network = ipaddress.ip_network + + def ip_network(*args, **kwargs): + reader.close() + return real_ip_network(*args, **kwargs) + + ipaddress.ip_network = ip_network + + from maxminddb.extension import Reader + + reader = Reader(sys.argv[1]) + iterator = iter(reader) + # The record was decoded before the close, so this call finishes. + next(iterator) + try: + next(iterator) + except ValueError: + pass + else: + sys.exit("next() after close() did not raise ValueError") + print("ok") + """, + ) + # Put this process's maxminddb first, and keep the harness's paths. + paths = [str(pathlib.Path(maxminddb.__file__).parent.parent)] + if os.environ.get("PYTHONPATH"): + paths.append(os.environ["PYTHONPATH"]) + env = {**os.environ, "PYTHONPATH": os.pathsep.join(paths)} + path = pathlib.Path(f"{_TEST_DATA_DIR}/GeoIP2-City-Test.mmdb").resolve() + with tempfile.TemporaryDirectory() as directory: + # Run from an empty directory so the child imports the same + # maxminddb as this process, not a source tree in the cwd. + result = subprocess.run( # noqa: S603 + [sys.executable, "-c", program, str(path)], + capture_output=True, + text=True, + check=False, + cwd=directory, + env=env, + timeout=60, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "ok") + def test_iterator_type_is_not_instantiable(self) -> None: with maxminddb.extension.Reader(_DECODER_DB) as reader: iterator_class = type(iter(reader)) From 83d517bf714ffc1f10e636d9d380cb531756d0b9 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 3 Oct 2026 05:14:02 +0000 Subject: [PATCH 2/8] Let one thread at a time advance an iterator ReaderIter_next takes records off the iterator's pending list and adds their children while it holds only the shared read lock. On free-threaded builds, two threads that called next() on the same iterator could take the same record and both free it. The process aborted with heap corruption. Hold a critical section on the iterator for each next() call. With the GIL, the list changes already run without interruption. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 2 ++ extension/maxminddb.c | 16 ++++++++++++++++ tests/reader_test.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 6795cb1..a791dcc 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -27,6 +27,8 @@ History macOS when a ``Reader`` failed to open or was used without ``__init__``. * Fixed a deadlock on free-threaded Python when a ``Reader`` was closed during iteration, from another thread or from a signal handler. + * Fixed a crash on free-threaded Python when two threads advanced the same + iterator. 3.2.0 (2026-09-10) ++++++++++++++++++ diff --git a/extension/maxminddb.c b/extension/maxminddb.c index b013a9e..efc2344 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -135,6 +135,7 @@ static inline maxminddb_state *get_maxminddb_state_from_self(PyObject *self) { static void reader_close_database(Reader_obj *reader); static bool can_read(const char *path); static int get_record(PyObject *self, PyObject *args, PyObject **record); +static PyObject *reader_iter_next(PyObject *self); static bool format_sockaddr(struct sockaddr *addr, char *dst); static PyObject *from_entry_data_list(maxminddb_state *state, MMDB_entry_data_list_s **entry_data_list); @@ -814,6 +815,21 @@ static bool is_ipv6(char ip[16]) { } static PyObject *ReaderIter_next(PyObject *self) { + PyObject *result; +#ifdef Py_GIL_DISABLED + // The iterator's list of pending records is not thread-safe, so let only + // one thread at a time advance an iterator. The read lock is shared, so + // it does not do this. + Py_BEGIN_CRITICAL_SECTION(self); +#endif + result = reader_iter_next(self); +#ifdef Py_GIL_DISABLED + Py_END_CRITICAL_SECTION(); +#endif + return result; +} + +static PyObject *reader_iter_next(PyObject *self) { maxminddb_state *state = get_maxminddb_state_from_self((PyObject *)self); if (state == NULL) { return NULL; diff --git a/tests/reader_test.py b/tests/reader_test.py index 22e106f..1ac59d0 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1326,6 +1326,48 @@ def ip_network(*args, **kwargs): self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(result.stdout.strip(), "ok") + @unittest.skipIf( + getattr(sys, "_is_gil_enabled", lambda: True)(), + "needs free-threaded Python", + ) + def test_threads_can_share_an_iterator(self) -> None: + path = f"{_TEST_DATA_DIR}/GeoIP2-City-Test.mmdb" + with maxminddb.extension.Reader(path) as reader: + expected = sorted(str(network) for network, _ in reader) + + def collect( + iterator: Iterator[tuple[object, object]], + barrier: threading.Barrier, + networks: list[str], + done: list[bool], + errors: list[BaseException], + ) -> None: + try: + barrier.wait() + networks.extend(str(network) for network, _ in iterator) + done.append(True) + except BaseException as e: # noqa: BLE001 + errors.append(e) + + # A race corrupts the heap only some of the time, so repeat. + for _ in range(5): + networks: list[str] = [] + done: list[bool] = [] + errors: list[BaseException] = [] + barrier = threading.Barrier(8, timeout=60) + args = (iter(reader), barrier, networks, done, errors) + threads = [ + threading.Thread(target=collect, args=args) for _ in range(8) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + self.assertEqual(errors, []) + self.assertEqual(len(done), 8) + # Each network comes out once, with none lost or repeated. + self.assertEqual(sorted(networks), expected) + def test_iterator_type_is_not_instantiable(self) -> None: with maxminddb.extension.Reader(_DECODER_DB) as reader: iterator_class = type(iter(reader)) From 1c39bf7870197fe304b68d16b2c84f32e24ac275 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 3 Oct 2026 16:34:48 +0000 Subject: [PATCH 3/8] Test on free-threaded Python in CI No CI job ran a free-threaded interpreter, so the reader locks compiled to no-ops in every test run. The tests for the lock lifetime, the lock release in the iterator, and the shared iterator could not fail, and macOS, where a zeroed pthread_rwlock_t is invalid, was never tested. Add a 3.14t tox environment and run it on each CI platform. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 2 +- pyproject.toml | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index eb4083b..5cdf061 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ jobs: strategy: fail-fast: false matrix: - env: ["3.10", 3.11, 3.12, 3.13, 3.14] + env: ["3.10", 3.11, 3.12, 3.13, 3.14, "3.14t"] os: [ubuntu-latest, ubuntu-24.04-arm, macos-latest, windows-latest] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/pyproject.toml b/pyproject.toml index fa1b12b..10f79e3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,6 +108,7 @@ env_list = [ "3.12", "3.13", "3.14", + "3.14t", "lint", ] skip_missing_interpreters = false @@ -143,6 +144,7 @@ commands = [ ] [tool.tox.gh.python] +"3.14t" = ["3.14t"] "3.14" = ["3.14", "lint"] "3.13" = ["3.13"] "3.12" = ["3.12"] From b3f88cad75655d6cc8e1a09c3fced42999d93027 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 6 Oct 2026 22:28:13 +0000 Subject: [PATCH 4/8] Run the ip_network close test with _run_program test_close_from_ip_network_during_iteration copied the body of _run_program, with only a different database. The program works with the decoder database too, so call _run_program and keep one copy of the subprocess setup. Co-Authored-By: Claude Opus 5.5 --- tests/reader_test.py | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/tests/reader_test.py b/tests/reader_test.py index 1ac59d0..eb7798f 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1305,26 +1305,7 @@ def ip_network(*args, **kwargs): print("ok") """, ) - # Put this process's maxminddb first, and keep the harness's paths. - paths = [str(pathlib.Path(maxminddb.__file__).parent.parent)] - if os.environ.get("PYTHONPATH"): - paths.append(os.environ["PYTHONPATH"]) - env = {**os.environ, "PYTHONPATH": os.pathsep.join(paths)} - path = pathlib.Path(f"{_TEST_DATA_DIR}/GeoIP2-City-Test.mmdb").resolve() - with tempfile.TemporaryDirectory() as directory: - # Run from an empty directory so the child imports the same - # maxminddb as this process, not a source tree in the cwd. - result = subprocess.run( # noqa: S603 - [sys.executable, "-c", program, str(path)], - capture_output=True, - text=True, - check=False, - cwd=directory, - env=env, - timeout=60, - ) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(result.stdout.strip(), "ok") + self._run_program(program) @unittest.skipIf( getattr(sys, "_is_gil_enabled", lambda: True)(), From 85bbe6185840f8b0ec7b9e957e9ec2fe75b64ada Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 6 Oct 2026 22:28:47 +0000 Subject: [PATCH 5/8] Make the shared-iterator test detect the race All 8 threads extended one shared list. list.extend holds the list's critical section while it pulls from the iterator, so only one thread at a time called next(), and the test passed without the iterator's critical section. The test also ran in the pytest process, so heap corruption would abort the whole run, and join() had no timeout. Give each thread its own list and merge them after join(). Run the test in a subprocess through _run_program, which now takes the database path. Without the critical section in ReaderIter_next, the test now fails with an abort on 3.14t. The done list is gone, because an empty errors list already shows that every thread finished. Co-Authored-By: Claude Opus 5.5 --- tests/reader_test.py | 60 +++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/tests/reader_test.py b/tests/reader_test.py index eb7798f..65503be 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1192,18 +1192,17 @@ def __del__(self): ) self._run_program(program) - def _run_program(self, program: str) -> None: + def _run_program(self, program: str, path: str = _DECODER_DB) -> None: # Put this process's maxminddb first, and keep the harness's paths. paths = [str(pathlib.Path(maxminddb.__file__).parent.parent)] if os.environ.get("PYTHONPATH"): paths.append(os.environ["PYTHONPATH"]) env = {**os.environ, "PYTHONPATH": os.pathsep.join(paths)} - path = pathlib.Path(_DECODER_DB).resolve() with tempfile.TemporaryDirectory() as directory: # Run from an empty directory so the child imports the same # maxminddb as this process, not a source tree in the cwd. result = subprocess.run( # noqa: S603 - [sys.executable, "-c", program, str(path)], + [sys.executable, "-c", program, str(pathlib.Path(path).resolve())], capture_output=True, text=True, check=False, @@ -1312,42 +1311,57 @@ def ip_network(*args, **kwargs): "needs free-threaded Python", ) def test_threads_can_share_an_iterator(self) -> None: - path = f"{_TEST_DATA_DIR}/GeoIP2-City-Test.mmdb" - with maxminddb.extension.Reader(path) as reader: + # Run in a subprocess, so heap corruption or a hang fails only this + # test. + program = textwrap.dedent( + """ + import sys + import threading + + from maxminddb.extension import Reader + + reader = Reader(sys.argv[1]) expected = sorted(str(network) for network, _ in reader) - def collect( - iterator: Iterator[tuple[object, object]], - barrier: threading.Barrier, - networks: list[str], - done: list[bool], - errors: list[BaseException], - ) -> None: + def collect(iterator, barrier, networks, errors): try: barrier.wait() - networks.extend(str(network) for network, _ in iterator) - done.append(True) - except BaseException as e: # noqa: BLE001 + # Each thread appends to its own list. With one shared + # list, list.extend would hold the list's critical + # section, and only one thread would call next() at a + # time. + for network, _ in iterator: + networks.append(str(network)) + except BaseException as e: errors.append(e) # A race corrupts the heap only some of the time, so repeat. for _ in range(5): - networks: list[str] = [] - done: list[bool] = [] - errors: list[BaseException] = [] + iterator = iter(reader) barrier = threading.Barrier(8, timeout=60) - args = (iter(reader), barrier, networks, done, errors) + results = [[] for _ in range(8)] + errors = [] threads = [ - threading.Thread(target=collect, args=args) for _ in range(8) + threading.Thread( + target=collect, + args=(iterator, barrier, networks, errors), + ) + for networks in results ] for thread in threads: thread.start() for thread in threads: thread.join() - self.assertEqual(errors, []) - self.assertEqual(len(done), 8) + if errors: + sys.exit(f"a thread failed: {errors!r}") # Each network comes out once, with none lost or repeated. - self.assertEqual(sorted(networks), expected) + networks = sorted(n for result in results for n in result) + if networks != expected: + sys.exit("networks were lost or repeated") + print("ok") + """, + ) + self._run_program(program, f"{_TEST_DATA_DIR}/GeoIP2-City-Test.mmdb") def test_iterator_type_is_not_instantiable(self) -> None: with maxminddb.extension.Reader(_DECODER_DB) as reader: From 38d04bcb984700b6b47b9d96707efa00b60930af Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 6 Oct 2026 22:30:16 +0000 Subject: [PATCH 6/8] Test a close from another thread during ip_network The existing test closes the reader from inside ip_network, on the iterating thread. Nothing covered the two-thread deadlock that the fix in "Release the read lock before building the network" describes. In the new test, thread A iterates with an ip_network that waits until thread B calls close(), then starts a GC. On free-threaded Python the GC waits for B, and B waits for the write lock. If A still held the read lock, neither could continue. The test runs in a subprocess with a timeout. With the fix reverted, it times out on 3.14t. Co-Authored-By: Claude Opus 5.5 --- tests/reader_test.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/reader_test.py b/tests/reader_test.py index 65503be..22a82c9 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1306,6 +1306,49 @@ def ip_network(*args, **kwargs): ) self._run_program(program) + def test_close_from_another_thread_during_ip_network(self) -> None: + # Thread B calls close() and waits for the write lock while thread A + # is in ip_network. ip_network then starts a GC, which on free-threaded + # Python waits for every thread, B included. If A still held the read + # lock, B would never get the lock, and both would wait forever. + program = textwrap.dedent( + """ + import gc + import ipaddress + import sys + import threading + import time + + real_ip_network = ipaddress.ip_network + closing = threading.Event() + + def ip_network(*args, **kwargs): + if not closing.is_set(): + closing.set() + # Give the other thread time to wait for the write lock. + time.sleep(0.2) + gc.collect() + return real_ip_network(*args, **kwargs) + + ipaddress.ip_network = ip_network + + from maxminddb.extension import Reader + + reader = Reader(sys.argv[1]) + + def close(): + closing.wait() + reader.close() + + closer = threading.Thread(target=close) + closer.start() + next(iter(reader)) + closer.join() + print("ok") + """, + ) + self._run_program(program) + @unittest.skipIf( getattr(sys, "_is_gil_enabled", lambda: True)(), "needs free-threaded Python", From 33f8352ad811959d9589932a9c4d3184c3435ceb Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 6 Oct 2026 22:30:27 +0000 Subject: [PATCH 7/8] Document that no Python code runs under the read lock The fix for the ip_network deadlock depends on a rule that nothing in the code stated: no Python code runs while a thread holds the read lock. State the rule and the reason at the lock functions, so that a later change does not bring the deadlock back. Also record why the lock does not wait detached from the interpreter: that let a thread take the lock during a stop-the-world pause, which can hang a forked child. Co-Authored-By: Claude Opus 5.5 --- extension/maxminddb.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/extension/maxminddb.c b/extension/maxminddb.c index efc2344..90cbeda 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -203,6 +203,13 @@ static void reader_lock_destroy(reader_rwlock_t *lock) { #endif } +// No Python code may run while a thread holds the read lock. On free-threaded +// Python, close() and __init__ wait for the write lock while they stay +// attached to the interpreter. If Python code under the read lock started a +// GC, the stop-the-world pause would wait for the writer, and the writer would +// wait for the read lock, so both would hang. Waiting detached instead lets a +// thread take the lock during a stop-the-world pause, which can hang a forked +// child. static int reader_acquire_read_lock(Reader_obj *reader) { #ifdef MAXMINDDB_USE_WINDOWS_LOCKS AcquireSRWLockShared(&(reader->rwlock)); From cf87107d0cd0162af22bca242fb472d84ad3953a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 6 Oct 2026 22:31:21 +0000 Subject: [PATCH 8/8] Test on free-threaded Python 3.13 in CI README.rst says that the extension supports free-threading on Python 3.13 and later, but CI ran only 3.14t. On 3.13t, a recursive critical section takes a different code path, so CI did not cover the critical section in ReaderIter_next there. Add 3.13t to the CI matrix and to the tox environments. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 2 +- pyproject.toml | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5cdf061..a5e033e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ jobs: strategy: fail-fast: false matrix: - env: ["3.10", 3.11, 3.12, 3.13, 3.14, "3.14t"] + env: ["3.10", 3.11, 3.12, 3.13, 3.14, "3.13t", "3.14t"] os: [ubuntu-latest, ubuntu-24.04-arm, macos-latest, windows-latest] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/pyproject.toml b/pyproject.toml index 10f79e3..c51facb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,6 +108,7 @@ env_list = [ "3.12", "3.13", "3.14", + "3.13t", "3.14t", "lint", ] @@ -145,6 +146,7 @@ commands = [ [tool.tox.gh.python] "3.14t" = ["3.14t"] +"3.13t" = ["3.13t"] "3.14" = ["3.14", "lint"] "3.13" = ["3.13"] "3.12" = ["3.12"]