Skip to content

Release the GIL across libmaxminddb lookup calls #362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
efroemling wants to merge 1 commit into from
+17 −3
Closed

Release the GIL across libmaxminddb lookup calls #362

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions extension/maxminddb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,18 @@ static int get_record(PyObject *self, PyObject *args, PyObject **record) {
return -1;
}

// Release the GIL across the libmaxminddb tree walk. The walk only
// touches the read-only mmap and stack-local state, the rwlock is
// already held, and the module is declared Py_MOD_GIL_NOT_USED - so
// no Python state is accessed here. On systems where the mmdb's
// pages are not resident (cold cache, slow disk, memory pressure)
// an individual page-in can stall this call for seconds; without
// releasing the GIL, the entire interpreter stalls with it.
int mmdb_error = MMDB_SUCCESS;
MMDB_lookup_result_s result =
MMDB_lookup_sockaddr(mmdb, ip_address, &mmdb_error);
MMDB_lookup_result_s result;
Py_BEGIN_ALLOW_THREADS
result = MMDB_lookup_sockaddr(mmdb, ip_address, &mmdb_error);
Py_END_ALLOW_THREADS
Comment on lines +454 to +456
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Releasing the GIL here is unsafe when the extension is compiled in 'GIL-only' mode (the default for standard Python builds, as defined on lines 24-26). In this mode, the internal rwlock is a no-op, and the GIL is the only mechanism protecting the mmdb structure from being freed by a concurrent Reader.close() call in another thread. If the GIL is released, a race condition occurs that can lead to a use-after-free crash.

To fix this safely, wrap the block in #ifndef MAXMINDDB_USE_GIL_ONLY. To enable this optimization for standard Python, you should also update the locking logic at the top of the file to use real locks (pthreads/Windows) even when the GIL is present.

#ifndef MAXMINDDB_USE_GIL_ONLY
    Py_BEGIN_ALLOW_THREADS
#endif
    result = MMDB_lookup_sockaddr(mmdb, ip_address, &mmdb_error);
#ifndef MAXMINDDB_USE_GIL_ONLY
    Py_END_ALLOW_THREADS
#endif


if (mmdb_error != MMDB_SUCCESS) {
reader_release_read_lock(reader);
Expand Down Expand Up @@ -478,8 +487,13 @@ static int get_record(PyObject *self, PyObject *args, PyObject **record) {
return prefix_len;
}

// Same reasoning as above: read the data section from the mmap
// without holding the GIL.
MMDB_entry_data_list_s *entry_data_list = NULL;
int status = MMDB_get_entry_data_list(&result.entry, &entry_data_list);
int status;
Py_BEGIN_ALLOW_THREADS
status = MMDB_get_entry_data_list(&result.entry, &entry_data_list);
Py_END_ALLOW_THREADS
Comment on lines +494 to +496
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similar to the previous lookup call, releasing the GIL here is unsafe in 'GIL-only' mode because the rwlock does not provide actual synchronization. A concurrent call to Reader.close() could free the mmdb structure while MMDB_get_entry_data_list is accessing the mmap.

You should wrap this block in #ifndef MAXMINDDB_USE_GIL_ONLY or ensure that real locks are used even in standard Python builds.

#ifndef MAXMINDDB_USE_GIL_ONLY
    Py_BEGIN_ALLOW_THREADS
#endif
    status = MMDB_get_entry_data_list(&result.entry, &entry_data_list);
#ifndef MAXMINDDB_USE_GIL_ONLY
    Py_END_ALLOW_THREADS
#endif

if (status != MMDB_SUCCESS) {
reader_release_read_lock(reader);
char ipstr[INET6_ADDRSTRLEN] = {0};
Expand Down
Loading