FIX: Release GIL during blocking ODBC connect/disconnect calls

[Saurabh Singh (saurabh500)]

Summary

The Python GIL is held during the entire ODBC connection establishment (SQLDriverConnect) and teardown (SQLDisconnect) calls. These are blocking I/O operations involving DNS resolution, TCP handshake, TLS negotiation, and SQL Server authentication that can take hundreds of milliseconds to seconds. While the GIL is held, all other Python threads are blocked, even those doing unrelated work.

This causes severe throughput degradation in multi-threaded applications that establish connections concurrently.

Relates to #491

Benchmark Results

A/B benchmark with 10 threads concurrently establishing connections (pooling disabled, SQL Server 2025 on localhost):

Metric	Before (GIL held)	After (GIL released)
Serial single connection	85.1 ms	86.2 ms
10 threads concurrent (wall-clock)	862.9 ms	150.1 ms
Speedup vs serial estimate	0.99× (fully serialized)	5.74× (parallel I/O)

Before this change, 10 concurrent threads take the same time as 10 serial connections — the GIL completely serializes them. After, threads overlap their I/O and achieve ~6× speedup.

Raw benchmark output (click to expand)

Before (GIL held during ODBC calls):

--- Serial Baseline (5 rounds) ---
Individual: [86.0ms, 89.8ms, 82.9ms, 85.0ms, 85.1ms]
Median: 85.1 ms
--- Concurrent (10 threads) ---
Trial 1: wall=862.9ms per-thread=[257, 172, 86, 255, 428, 173, 341, 172, 88, 173]ms
Trial 2: wall=873.5ms per-thread=[90, 350, 599, 354, 433, 187, 269, 101, 173, 427]ms
Trial 3: wall=852.2ms per-thread=[337, 91, 258, 170, 89, 253, 338, 172, 254, 848]ms
Speedup: 0.99x ❌

After (GIL released during ODBC calls):

--- Serial Baseline (5 rounds) ---
Individual: [89.1ms, 85.7ms, 86.2ms, 85.6ms, 87.7ms]
Median: 86.2 ms
--- Concurrent (10 threads) ---
Trial 1: wall=150.1ms per-thread=[91, 98, 137, 134, 133, 109, 137, 143, 136, 117]ms
Trial 2: wall=148.2ms per-thread=[105, 112, 131, 108, 118, 110, 111, 111, 142, 108]ms
Trial 3: wall=184.6ms per-thread=[87, 92, 97, 180, 91, 178, 179, 92, 175, 181]ms
Speedup: 5.74x ✅

Solution

1. GIL Release in connection.cpp

• Connection::connect(): Wrap SQLDriverConnect_ptr in a py::gil_scoped_release block. All py::dict access (applyAttrsBefore) completes before the release scope, and checkError() runs after GIL is re-acquired.
• Connection::disconnect(): Conditionally release GIL around SQLDisconnect_ptr, guarded by PyGILState_Check() to safely handle destructor/shutdown paths where the GIL may not be held.
• Connection::reset(): Removed internal disconnect() calls from failure paths. The pool caller already adds failed connections to to_disconnect for cleanup outside the mutex, so the internal disconnect was both redundant and problematic (it would release the GIL while the pool mutex was held).

2. Pool Lock Restructuring in connection_pool.cpp

Releasing the GIL inside ODBC calls while holding C++ mutexes creates a mutex/GIL lock-ordering deadlock. The pool changes are a necessary prerequisite for the GIL release to be safe.

Why the pool changes are required

Without restructuring, we get this deadlock:

Thread A Thread B
──────── ────────
pool.acquire()
lock(_mutex) ✅
connect()
gil_scoped_release → GIL freed
SQLDriverConnect... (blocking)
Acquires GIL ✅
pool.acquire()
lock(_mutex) ❌ BLOCKED (Thread A holds it)
...SQLDriverConnect done
gil_scoped_release → tries to
destructor re-acquire GIL ❌ BLOCKED (Thread B holds it)
💀 DEADLOCK: Thread A has mutex, needs GIL
Thread B has GIL, needs mutex

How the fix works — ConnectionPool::acquire()

The solution separates mutex-protected bookkeeping from blocking ODBC calls:

Thread A Thread B
──────── ────────
pool.acquire() pool.acquire()
lock(_mutex) ✅
reserve slot (++_current_size)
create Connection object
unlock(_mutex) → released
lock(_mutex) ✅
reserve slot
create Connection
unlock(_mutex) → released
connect() connect()
gil_scoped_release → GIL freed gil_scoped_release → GIL freed
SQLDriverConnect... (parallel) SQLDriverConnect... (parallel)
...done → GIL back ...done → GIL back
✅ Both threads connect in parallel — no deadlock

If connect() fails, the reserved slot is rolled back:

catch (...) {
std::lock_guard<std::mutex> lock(_mutex);
if (_current_size > 0) --_current_size;
throw;
}

The same pattern applies to all pool/manager methods

Method	Before (❌ deadlock risk)	After (✅ safe)
Pool::acquire()	connect() inside _mutex	Reserve slot inside, connect() outside
Pool::release()	disconnect() inside _mutex	Decide inside, disconnect() outside
Manager::acquireConnection()	pool->acquire() inside _manager_mutex	Lookup pool inside, acquire() outside
Manager::returnConnection()	pool->release() inside _manager_mutex	Lookup pool inside, release() outside
Manager::closePools()	pool->close() inside _manager_mutex	Collect pools inside, close() outside

The invariant is: never hold a C++ mutex across a scope that releases the GIL.

Testing

Added test_021_concurrent_connection_perf.py with three @pytest.mark.stress tests:

1. test_concurrent_connection_gil_release — 10 threads establish connections concurrently (pooling disabled); asserts >2× speedup over serial baseline, proving I/O parallelism.
2. test_concurrent_disconnect_gil_release — 10 threads disconnect concurrently; asserts >1.5× speedup.
3. test_mixed_connect_disconnect_under_load — I/O threads (connect/disconnect) run alongside CPU-bound Python threads for 5 seconds; verifies no starvation or deadlocks.

Added test_pool_release_overflow_disconnects_outside_mutex in test_009_pooling.py to cover the pool release overflow path.

Added benchmarks/bench_concurrent_connect.py — standalone A/B benchmark script for reproducing the results above.

Safety

• LOG() macro acquires GIL internally via py::gil_scoped_acquire — safe to call from any context
• checkError() is pure C++ (throws std::runtime_error) — caught by pybind11 at the boundary where GIL is held
• PyGILState_Check() guard in disconnect() handles destructor paths safely
• All pool data structures remain mutex-protected; only blocking ODBC calls moved outside lock scopes

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 83%

🎯 Overall Coverage 79%

📈 Total Lines Covered:6677 out of 8449 📁 Project:mssql-python

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql_python/pybind/connection/connection.cpp (83.3%): Missing lines 160-161,170
• mssql_python/pybind/connection/connection_pool.cpp (83.7%): Missing lines 107-108,113-118

Summary

• Total: 67 lines
• Missing: 11 lines
• Coverage: 83%

mssql_python/pybind/connection/connection.cpp

Lines 156-165

156 py::gil_scoped_release release;
157 ret = SQLDisconnect_ptr(_dbcHandle->get());
158 } else {
159// Destructor / shutdown path — GIL is not held, call directly.
! 160 ret = SQLDisconnect_ptr(_dbcHandle->get());
! 161 }
162// In destructor/shutdown paths, suppress errors to avoid163// std::terminate() if this throws during stack unwinding.164if (hasGil) {
165checkError(ret);

---

Lines 166-174

166 } elseif (!SQL_SUCCEEDED(ret)) {
167// Intentionally no LOG() here: LOG() acquires the GIL internally168// via py::gil_scoped_acquire, which is unsafe during interpreter169// shutdown or stack unwinding (can deadlock or call std::terminate).
! 170 }
171// triggers SQLFreeHandle via destructor, if last owner172 _dbcHandle.reset();
173 } else {
174LOG("No connection handle to disconnect");

---

mssql_python/pybind/connection/connection_pool.cpp

Lines 103-122

103if (_pool.size() < _max_size) {
104 conn->updateLastUsed();
105 _pool.push_back(conn);
106 } else {
! 107 should_disconnect = true;
! 108 }
109 }
110// Disconnect outside the mutex to avoid holding it during the111// blocking ODBC call (which releases the GIL).112if (should_disconnect) {
! 113try {
! 114 conn->disconnect();
! 115 } catch (const std::exception& ex) {
! 116LOG("ConnectionPool::release: disconnect failed: %s", ex.what());
! 117 }
! 118 std::lock_guard<std::mutex> lock(_mutex);
119if (_current_size > 0)
120 --_current_size;
121 }
122 }

---

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 67.8%
mssql_python.row.py: 70.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 74.2%
mssql_python.pybind.connection.connection.cpp: 75.8%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.2%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

[Copilot]

Pull request overview

This PR improves multi-threaded throughput by releasing the Python GIL around the blocking ODBC connect (SQLDriverConnect) and disconnect (SQLDisconnect) calls, and restructures connection pooling/manager locking to avoid mutex↔GIL deadlocks during those operations.

Changes:

• Release the GIL during Connection::connect() and (conditionally) during Connection::disconnect().
• Restructure ConnectionPool / ConnectionPoolManager to avoid holding C++ mutexes across connect/disconnect operations.
• Add new stress/performance tests for concurrent connect/disconnect behavior, plus a pooling regression test.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
mssql_python/pybind/connection/connection.cpp	Releases GIL during ODBC connect/disconnect; adjusts reset failure handling.
mssql_python/pybind/connection/connection_pool.cpp	Moves connect/disconnect work outside pool/manager mutexes to prevent GIL/mutex deadlocks.
tests/test_021_concurrent_connection_perf.py	Adds stress tests to validate parallelism and absence of starvation/deadlocks under load.
tests/test_009_pooling.py	Adds a test intended to cover pool release overflow/disconnect behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.