FIX: Bound pooled connection checkout on unresponsive sockets

README.md

@@ -129,6 +129,8 @@ EntraID authentication is now fully supported on MacOS and Linux but with certai
 
 The Microsoft mssql_python driver provides built-in support for connection pooling, which helps improve performance and scalability by reusing active database connections instead of creating a new connection for every request. This feature is enabled by default. For more information, refer [Connection Pooling Wiki](https://github.com/microsoft/mssql-python/wiki/Connection#connection-pooling).
 
+Pooled checkout validates and resets a reused connection with a temporary native connection timeout. It uses the positive login timeout from `connect(timeout=N)` or, with higher precedence, `attrs_before[SQL_ATTR_LOGIN_TIMEOUT]`; if neither specifies a positive value, validation uses 15 seconds. A failed validation discards that candidate and attempts a new physical connection instead of probing every idle socket sequentially. The previous connection timeout is restored after successful checkout, and query timeouts remain controlled separately by `conn.timeout`. These are driver operation timeouts, not a strict end-to-end `connect()` deadline: cancellation/cleanup and a replacement login can add time. Pool return still requires network I/O and is not bounded by the checkout validation timeout.
+
 > **Interactive / Device-code authentication in multi-user processes:** For `ActiveDirectoryInteractive` and `ActiveDirectoryDeviceCode`, the driver caches one credential instance per authentication type for the lifetime of the process so that pooled reconnects refresh silently instead of re-prompting. As a result, every interactive connection opened in the same process shares that one signed-in account — the first user to authenticate. If your application serves multiple end users from a single process, do **not** rely on interactive/device-code auth to isolate them; instead supply your own per-user token via a token provider so each user's connection is keyed to their own identity.
 
 > **Bring-your-own token (`token_provider` / raw access token):** When you supply your own access token (via a token provider or a raw token in `attrs_before`), managing its lifetime is **your application's responsibility**. The driver stores a pooled connection under the identity that opened it, but it cannot refresh a token it did not mint. If a pooled connection is reused after its token has expired, the reconnect (ODBC's implicit connection resiliency) will fail. Ensure your token provider returns a currently-valid token on every call, and account for token expiry in your own logic. The driver's built-in expiry-aware checkout (refreshing a pooled connection whose token is near expiry) only applies to Managed Identity and interactive/device-code pools, whose tokens the driver can silently re-mint — not to bring-your-own tokens.

mssql_python/connection.py

@@ -417,7 +417,11 @@ def __init__(
                 query timeout. An explicit ``attrs_before[SQL_ATTR_LOGIN_TIMEOUT]``
                 takes precedence over this value. (pyodbc migration note: pyodbc
                 lets a positive ``timeout=`` override ``attrs_before``; here the
-                explicit ``attrs_before`` entry wins.)
+                explicit ``attrs_before`` entry wins.) Pooled candidate reset
+                also uses this positive timeout, or 15 seconds when zero/default,
+                temporarily as SQL_ATTR_CONNECTION_TIMEOUT. Driver cancellation
+                and a replacement login can add time; this is not an overall
+                connect deadline.
             native_uuid (bool, optional): Controls whether UNIQUEIDENTIFIER columns return
                 uuid.UUID objects (True) or str (False) for cursors created from this connection.
                 None (default) defers to the module-level ``mssql_python.native_uuid`` setting (True).
@@ -858,17 +862,22 @@ def _token_factory():
         _provider = ProviderManager.ensure_available()
         ddbc_bindings._set_odbc_provider(_provider)
 
+        # Configure a reused connection's mode inside native checkout, while
+        # its temporary connection timeout still bounds attribute-setting I/O.
+        native_attrs = self._attrs_before.copy()
+        native_attrs[ConstantsDDBC.SQL_ATTR_AUTOCOMMIT.value] = int(bool(autocommit))
         try:
             self._conn = ddbc_bindings.Connection(
                 self.connection_str,
                 self._pooling,
-                self._attrs_before,
+                native_attrs,
                 self._pool_key,
                 self._token_factory,
             )
+            if self._conn.get_autocommit() != autocommit:
+                self.setautocommit(autocommit)
         except RuntimeError as e:
             _raise_connection_error(e)
-        self.setautocommit(autocommit)
 
         # Register this connection for cleanup before Python shutdown
         # This ensures ODBC handles are freed in correct order, preventing leaks

mssql_python/pybind/connection/connection.cpp

@@ -600,11 +600,31 @@ bool Connection::isAlive() const {
     return SQL_SUCCEEDED(ret) && status == SQL_CD_FALSE;
 }
 
-bool Connection::reset() {
+bool Connection::reset(unsigned int timeoutSeconds, bool autocommit) {
     if (!_dbcHandle) {
         ThrowStdException("Connection handle not allocated");
     }
     clearResultMetadata();
+    // Login and statement timeouts do not bound connection-attribute calls.
+    // Keep a finite connection timeout through reset and lease configuration;
+    // retain it on failure so discarding the candidate is bounded too (#854).
+    SQLULEN originalTimeout = 0;
+    SQLRETURN ret = SQLGetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_CONNECTION_TIMEOUT,
+                                         &originalTimeout, 0, nullptr);
+    if (!SQL_SUCCEEDED(ret)) {
+        LOG("Failed to read pooled connection timeout (ret=%d). Marking as dead.", ret);
+        return false;
+    }
+    {
+        py::gil_scoped_release release;
+        ret = SQLSetConnectAttr_ptr(
+            _dbcHandle->get(), SQL_ATTR_CONNECTION_TIMEOUT,
+            reinterpret_cast<SQLPOINTER>(static_cast<SQLULEN>(timeoutSeconds)), SQL_IS_UINTEGER);
+    }
+    if (!SQL_SUCCEEDED(ret)) {
+        LOG("Failed to set pooled connection timeout (ret=%d). Marking as dead.", ret);
+        return false;
+    }
     LOG("Resetting connection via SQL_ATTR_RESET_CONNECTION");
     // NOTE: SQL_ATTR_RESET_CONNECTION is a pool-checkin reset: it asks the
     // driver to wipe per-session state (temp tables, open cursors, SET
@@ -618,7 +638,6 @@ bool Connection::reset() {
     // Connection (_attrStringBuffers / _attrBytesBuffers) are intentionally
     // retained here. Clearing them would reintroduce issue #594 in a new
     // form (UAF during silent reconnect).
-    SQLRETURN ret;
     {
         // Release the GIL around the ODBC call for consistency with the
         // other connection-attribute paths; some attributes can block.
@@ -631,17 +650,27 @@ bool Connection::reset() {
         return false;
     }
 
-    // SQL_ATTR_RESET_CONNECTION does NOT reset the transaction isolation level.
-    // Explicitly reset it to the default (SQL_TXN_READ_COMMITTED) to prevent
-    // isolation level settings from leaking between pooled connection usages.
+    // SQL_ATTR_RESET_CONNECTION does not reset transaction isolation.
     LOG("Resetting transaction isolation level to READ COMMITTED");
     {
         py::gil_scoped_release release;
         ret = SQLSetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_TXN_ISOLATION,
                                     (SQLPOINTER)SQL_TXN_READ_COMMITTED, SQL_IS_INTEGER);
     }
     if (!SQL_SUCCEEDED(ret)) {
-        LOG("Failed to reset transaction isolation level (ret=%d). Marking as dead.", ret);
+        ErrorInfo error = SQLReadError(SQL_HANDLE_DBC, _dbcHandle->get(), ret);
+        LOG("Pooled connection validation failed: %s", error.ddbcErrorMsg.c_str());
+        return false;
+    }
+    setAutocommit(autocommit);
+    {
+        py::gil_scoped_release release;
+        ret = SQLSetConnectAttr_ptr(
+            _dbcHandle->get(), SQL_ATTR_CONNECTION_TIMEOUT,
+            reinterpret_cast<SQLPOINTER>(originalTimeout), SQL_IS_UINTEGER);
+    }
+    if (!SQL_SUCCEEDED(ret)) {
+        LOG("Failed to restore pooled connection timeout (ret=%d). Marking as dead.", ret);
         return false;
     }
 
@@ -905,6 +934,21 @@ py::object Connection::getInfo(SQLUSMALLINT infoType) const {
     return result;
 }
 
+SQLULEN ConnectionHandle::getConnectionTimeout() const {
+    if (!_conn) {
+        ThrowStdException("Connection object is not initialized");
+    }
+    SQLULEN timeout = 0;
+    const auto& handle = _conn->getDbcHandle();
+    SQLRETURN ret = SQLGetConnectAttr_ptr(handle->get(), SQL_ATTR_CONNECTION_TIMEOUT,
+                                         &timeout, 0, nullptr);
+    if (!SQL_SUCCEEDED(ret)) {
+        ErrorInfo error = SQLCheckError_Wrap(SQL_HANDLE_DBC, handle, ret);
+        ThrowStdException("Failed to read connection timeout: " + error.ddbcErrorMsg);
+    }
+    return timeout;
+}
+
 py::object ConnectionHandle::getInfo(SQLUSMALLINT infoType) const {
     PERF_TIMER("ConnectionHandle::getInfo");
     if (!_conn) {

mssql_python/pybind/connection/connection.h

@@ -58,7 +58,7 @@ class Connection {
     //  Check whether autocommit is enabled.
     bool getAutocommit() const;
     bool isAlive() const;
-    bool reset();
+    bool reset(unsigned int timeoutSeconds, bool autocommit = true);
     void prepareForPool(bool transactionAlreadyRolledBack = false);
     void updateLastUsed();
     std::chrono::steady_clock::time_point lastUsed() const;
@@ -158,6 +158,7 @@ class ConnectionHandle {
     void setAutocommit(bool enabled);
     bool getAutocommit() const;
     SqlHandlePtr allocStatementHandle();
+    SQLULEN getConnectionTimeout() const;
     void setAttr(int attribute, py::object value);
 
     // Get information about the driver and data source

mssql_python/pybind/connection/connection_pool.cpp

@@ -6,6 +6,7 @@
 #include <chrono>
 #include <exception>
 #include <memory>
+#include <limits>
 #include <vector>
 
 // Logging uses LOG() macro for all diagnostic output
@@ -60,6 +61,22 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::u16string& connSt
                                                     const py::dict& attrs_before,
                                                     const py::object& token_factory) {
     PERF_TIMER("ConnectionPool::acquire");
+    // A login timeout does not cover SQLSetConnectAttr on an existing socket.
+    // Use it for bounded candidate reset as well; zero/unspecified
+    // retains a finite validation default rather than an OS TCP retry budget.
+    unsigned int validation_timeout = 15;
+    const py::int_ login_timeout_attr(SQL_ATTR_LOGIN_TIMEOUT);
+    if (attrs_before.contains(login_timeout_attr)) {
+        const auto requested = attrs_before[login_timeout_attr].cast<long long>();
+        if (requested > 0) {
+            validation_timeout = static_cast<unsigned int>(std::min<long long>(
+                requested, std::numeric_limits<unsigned int>::max()));
+        }
+    }
+    const py::int_ autocommit_attr(SQL_ATTR_AUTOCOMMIT);
+    const bool autocommit = attrs_before.contains(autocommit_attr)
+                                ? attrs_before[autocommit_attr].cast<int>() != SQL_AUTOCOMMIT_OFF
+                                : true;
     std::vector<std::shared_ptr<Connection>> to_disconnect;
     std::shared_ptr<Connection> valid_conn = nullptr;
     bool needs_connect = false;
@@ -131,6 +148,7 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::u16string& connSt
 
         // Validate the candidate outside the mutex.
         bool reuse_candidate = false;
+        bool validation_attempted = false;
         try {
             if (token_factory && !token_factory.is_none() &&
                 candidate->isTokenNearExpiry(TOKEN_EXPIRY_THRESHOLD_SECS)) {
@@ -169,7 +187,10 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::u16string& connSt
                     // is taken in practice. We favor never handing out a token
                     // that may expire mid-query over avoiding the churn.
                     candidate->setTokenExpiry(fresh_expiry);
-                    reuse_candidate = candidate->isAlive() && candidate->reset();
+                    if (candidate->isAlive()) {
+                        validation_attempted = true;
+                        reuse_candidate = candidate->reset(validation_timeout, autocommit);
+                    }
                     if (!reuse_candidate) {
                         // The token is still valid but the socket is dead
                         // (isAlive()/reset() failed). We already minted the
@@ -213,7 +234,10 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::u16string& connSt
                     }
                 }
             } else {
-                reuse_candidate = candidate->isAlive() && candidate->reset();
+                if (candidate->isAlive()) {
+                    validation_attempted = true;
+                    reuse_candidate = candidate->reset(validation_timeout, autocommit);
+                }
             }
         } catch (const std::exception& ex) {
             LOG("Candidate connection validation failed: %s", ex.what());
@@ -232,10 +256,10 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::u16string& connSt
             if (_current_size > 0) --_current_size;
         }
 
-        // If a rotated token was captured, reserve a slot and reopen with it
-        // immediately instead of churning through the remaining candidates
-        // (which hold the same stale token and would all be discarded anyway).
-        if (have_pending_token) {
+        // After a reset failure, open a replacement instead of spending another
+        // timeout on each idle socket. Token rotation likewise opens directly
+        // using the captured token.
+        if (have_pending_token || validation_attempted) {
             std::lock_guard<std::mutex> lock(_mutex);
             if (_current_size < _max_size) {
                 // Reserve the slot here but construct the Connection outside
@@ -246,11 +270,12 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::u16string& connSt
                 needs_connect = true;
                 break;
             }
-            // Pool momentarily full; fall through and retry the loop. On the
-            // retry another near-expiry candidate may re-invoke the factory and
-            // overwrite pending_attrs/pending_expiry with a newer token. That
-            // needs a full pool AND a simultaneous rotation, is rare, and is
-            // harmless: we simply reopen with the most recently minted token.
+            if (validation_attempted) {
+                throw std::runtime_error(
+                    "ConnectionPool::acquire: pool size limit reached");
+            }
+            // A concurrent acquire reserved the slot freed by token rotation.
+            // Retry token comparison with another idle candidate.
         }
     }
 

mssql_python/pybind/ddbc_bindings.cpp

@@ -6476,6 +6476,8 @@ PYBIND11_MODULE(ddbc_bindings, m) {
         .def("rollback", &ConnectionHandle::rollback, "Rollback the current transaction")
         .def("set_autocommit", &ConnectionHandle::setAutocommit)
         .def("get_autocommit", &ConnectionHandle::getAutocommit)
+        .def("_get_connection_timeout", &ConnectionHandle::getConnectionTimeout,
+             "Internal: read the native SQL_ATTR_CONNECTION_TIMEOUT value")
         .def("set_attr", &ConnectionHandle::setAttr, py::arg("attribute"), py::arg("value"),
              "Set connection attribute")
         .def("alloc_statement_handle", &ConnectionHandle::allocStatementHandle)

tests/test_006_exceptions.py

@@ -301,11 +301,12 @@ def test_close_cleans_up_after_autocommit_read_failure():
     from unittest.mock import MagicMock, patch
 
     mock_conn = MagicMock()
-    mock_conn.get_autocommit.side_effect = RuntimeError("SQLSTATE:08S01:Communication link failure")
+    mock_conn.get_autocommit.return_value = False
 
     with patch("mssql_python.connection.ddbc_bindings.Connection", return_value=mock_conn):
         conn = connect("Server=testserver;Database=mydb;Trusted_Connection=yes;")
 
+    mock_conn.get_autocommit.side_effect = RuntimeError("SQLSTATE:08S01:Communication link failure")
     with pytest.raises(OperationalError, match="Communication link failure"):
         conn.close()
 
@@ -356,16 +357,16 @@ def test_native_close_error_takes_precedence_over_preclose_failure(preclose_fail
     from unittest.mock import MagicMock, patch
 
     mock_conn = MagicMock()
+    mock_conn.get_autocommit.return_value = False
+
+    with patch("mssql_python.connection.ddbc_bindings.Connection", return_value=mock_conn):
+        conn = connect("Server=testserver;Database=mydb;Trusted_Connection=yes;")
+
     if preclose_failure == "autocommit":
         mock_conn.get_autocommit.side_effect = RuntimeError("SQLSTATE:08S01:Autocommit read failed")
     else:
-        mock_conn.get_autocommit.return_value = False
         mock_conn.rollback.side_effect = RuntimeError("SQLSTATE:08S01:Rollback failed")
     mock_conn.close.side_effect = RuntimeError("SQLSTATE:08003:Native close failed")
-
-    with patch("mssql_python.connection.ddbc_bindings.Connection", return_value=mock_conn):
-        conn = connect("Server=testserver;Database=mydb;Trusted_Connection=yes;")
-
     with pytest.raises(OperationalError, match="Native close failed"):
         conn.close()