FIX: Fixing test that does not work properly / Failing in connection pool

mssql_python/connection.py

@@ -1623,6 +1623,14 @@ def __del__(self) -> None:
         This is a safety net to ensure resources are cleaned up
         even if close() was not called explicitly.
         """
+        import sys
+
+        # During interpreter shutdown, ODBC handles may already be invalid.
+        # Attempting to free them can cause segfaults (SIGSEGV).
+        # The OS will reclaim all resources when the process exits.
+        if sys is None or sys._is_finalizing():
+            return
+
         if "_closed" not in self.__dict__ or not self._closed:
             try:
                 self.close()

mssql_python/cursor.py

@@ -3021,17 +3021,19 @@ def __del__(self):
         even if close() was not called explicitly.
         If the cursor is already closed, it will not raise an exception during cleanup.
         """
+        import sys
+
+        # During interpreter shutdown, ODBC handles may already be invalid.
+        # Attempting to free them can cause segfaults (SIGSEGV).
+        # The OS will reclaim all resources when the process exits.
+        if sys is None or sys._is_finalizing():
+            return
+
         if "closed" not in self.__dict__ or not self.closed:
             try:
                 self.close()
             except Exception as e:  # pylint: disable=broad-exception-caught
                 # Don't raise an exception in __del__, just log it
-                # If interpreter is shutting down, we might not have logging set up
-                import sys
-
-                if sys and sys._is_finalizing():
-                    # Suppress logging during interpreter shutdown
-                    return
                 logger.debug("Exception during cursor cleanup in __del__: %s", e)
 
     def scroll(

mssql_python/pooling.py

@@ -45,10 +45,6 @@ def enable(cls, max_size: int = 100, idle_timeout: int = 600) -> None:
             idle_timeout,
         )
         with cls._lock:
-            if cls._enabled:
-                logger.debug("PoolingManager.enable: Pooling already enabled, skipping")
-                return
-
             if max_size <= 0 or idle_timeout < 0:
                 logger.error(
                     "PoolingManager.enable: Invalid parameters - max_size=%d, idle_timeout=%d",
@@ -57,16 +53,34 @@ def enable(cls, max_size: int = 100, idle_timeout: int = 600) -> None:
                 )
                 raise ValueError("Invalid pooling parameters")
 
-            logger.info(
-                "PoolingManager.enable: Enabling connection pooling - max_size=%d, idle_timeout=%d seconds",
-                max_size,
-                idle_timeout,
-            )
+            if cls._enabled:
+                # Already enabled — reconfigure if parameters changed
+                if (
+                    cls._config["max_size"] == max_size
+                    and cls._config["idle_timeout"] == idle_timeout
+                ):
+                    logger.debug(
+                        "PoolingManager.enable: Pooling already enabled with same config, skipping"
+                    )
+                    return
+                logger.info(
+                    "PoolingManager.enable: Reconfiguring pooling - max_size=%d, idle_timeout=%d seconds",
+                    max_size,
+                    idle_timeout,
+                )
+            else:
+                logger.info(
+                    "PoolingManager.enable: Enabling connection pooling - max_size=%d, idle_timeout=%d seconds",
+                    max_size,
+                    idle_timeout,
+                )
+
             ddbc_bindings.enable_pooling(max_size, idle_timeout)
             cls._config["max_size"] = max_size
             cls._config["idle_timeout"] = idle_timeout
             cls._enabled = True
             cls._initialized = True
+            cls._pools_closed = False
             logger.info("PoolingManager.enable: Connection pooling enabled successfully")
 
     @classmethod

mssql_python/pybind/connection/connection.cpp

@@ -51,7 +51,7 @@ Connection::Connection(const std::wstring& conn_str, bool use_pool)
 }
 
 Connection::~Connection() {
-    disconnect();  // fallback if user forgets to disconnect
+    disconnect_nothrow();  // fallback if user forgets to disconnect
 }
 
 // Allocates connection handle
@@ -91,31 +91,30 @@ void Connection::connect(const py::dict& attrs_before) {
     updateLastUsed();
 }
 
-void Connection::disconnect() {
+SQLRETURN Connection::disconnect_impl() noexcept {
     if (_dbcHandle) {
         LOG("Disconnecting from database");
 
         // CRITICAL FIX: Mark all child statement handles as implicitly freed
         // When we free the DBC handle below, the ODBC driver will automatically free
         // all child STMT handles. We need to tell the SqlHandle objects about this
         // so they don't try to free the handles again during their destruction.
-        
+
         // THREAD-SAFETY: Lock mutex to safely access _childStatementHandles
         // This protects against concurrent allocStatementHandle() calls or GC finalizers
         {
             std::lock_guard<std::mutex> lock(_childHandlesMutex);
-            
+
             // First compact: remove expired weak_ptrs (they're already destroyed)
             size_t originalSize = _childStatementHandles.size();
             _childStatementHandles.erase(
                 std::remove_if(_childStatementHandles.begin(), _childStatementHandles.end(),
                                [](const std::weak_ptr<SqlHandle>& wp) { return wp.expired(); }),
                 _childStatementHandles.end());
-
-            LOG("Compacted child handles: %zu -> %zu (removed %zu expired)",
-                originalSize, _childStatementHandles.size(),
-                originalSize - _childStatementHandles.size());
-
+
+            LOG("Compacted child handles: %zu -> %zu (removed %zu expired)", originalSize,
+                _childStatementHandles.size(), originalSize - _childStatementHandles.size());
+
             LOG("Marking %zu child statement handles as implicitly freed",
                 _childStatementHandles.size());
             for (auto& weakHandle : _childStatementHandles) {
@@ -124,8 +123,10 @@ void Connection::disconnect() {
                     // This is guaranteed by allocStatementHandle() which only creates STMT handles
                     // If this assertion fails, it indicates a serious bug in handle tracking
                     if (handle->type() != SQL_HANDLE_STMT) {
-                        LOG_ERROR("CRITICAL: Non-STMT handle (type=%d) found in _childStatementHandles. "
-                                  "This will cause a handle leak!", handle->type());
+                        LOG_ERROR(
+                            "CRITICAL: Non-STMT handle (type=%d) found in _childStatementHandles. "
+                            "This will cause a handle leak!",
+                            handle->type());
                         continue;  // Skip marking to prevent leak
                     }
                     handle->markImplicitlyFreed();
@@ -136,11 +137,30 @@ void Connection::disconnect() {
         }  // Release lock before potentially slow SQLDisconnect call
 
         SQLRETURN ret = SQLDisconnect_ptr(_dbcHandle->get());
-        checkError(ret);
-        // triggers SQLFreeHandle via destructor, if last owner
+        // Always free the handle regardless of SQLDisconnect result
         _dbcHandle.reset();
+        return ret;
     } else {
         LOG("No connection handle to disconnect");
+        return SQL_SUCCESS;
+    }
+}
+
+void Connection::disconnect() {
+    SQLRETURN ret = disconnect_impl();
+    if (!SQL_SUCCEEDED(ret)) {
+        // For user-facing disconnect, report the error.
+        // The handle is already freed by disconnect_impl(), so build the
+        // message from the saved return code.
+        std::string msg = "SQLDisconnect failed with return code " + std::to_string(ret);
+        ThrowStdException(msg.c_str());
+    }
+}
+
+void Connection::disconnect_nothrow() noexcept {
+    SQLRETURN ret = disconnect_impl();
+    if (!SQL_SUCCEEDED(ret)) {
+        LOG_ERROR("SQLDisconnect failed (ret=%d), error suppressed (nothrow path)", ret);
     }
 }
 
@@ -221,7 +241,7 @@ SqlHandlePtr Connection::allocStatementHandle() {
     // or GC finalizers running from different threads
     {
         std::lock_guard<std::mutex> lock(_childHandlesMutex);
-        
+
         // Track this child handle so we can mark it as implicitly freed when connection closes
         // Use weak_ptr to avoid circular references and allow normal cleanup
         _childStatementHandles.push_back(stmtHandle);
@@ -237,9 +257,8 @@ SqlHandlePtr Connection::allocStatementHandle() {
                                [](const std::weak_ptr<SqlHandle>& wp) { return wp.expired(); }),
                 _childStatementHandles.end());
             _allocationsSinceCompaction = 0;
-            LOG("Periodic compaction: %zu -> %zu handles (removed %zu expired)",
-                originalSize, _childStatementHandles.size(),
-                originalSize - _childStatementHandles.size());
+            LOG("Periodic compaction: %zu -> %zu handles (removed %zu expired)", originalSize,
+                _childStatementHandles.size(), originalSize - _childStatementHandles.size());
         }
     }  // Release lock
 
@@ -375,7 +394,7 @@ bool Connection::reset() {
                                           (SQLPOINTER)SQL_RESET_CONNECTION_YES, SQL_IS_INTEGER);
     if (!SQL_SUCCEEDED(ret)) {
         LOG("Failed to reset connection (ret=%d). Marking as dead.", ret);
-        disconnect();
+        disconnect_nothrow();
         return false;
     }
 
@@ -387,7 +406,7 @@ bool Connection::reset() {
                                 (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);
-        disconnect();
+        disconnect_nothrow();
         return false;
     }
 

mssql_python/pybind/connection/connection.h

@@ -4,8 +4,8 @@
 #pragma once
 #include "../ddbc_bindings.h"
 #include <memory>
-#include <string>
 #include <mutex>
+#include <string>
 
 // Represents a single ODBC database connection.
 // Manages connection handles.
@@ -29,8 +29,12 @@ class Connection {
     void connect(const py::dict& attrs_before = py::dict());
 
     // Disconnect and free the connection handle.
+    // Throws on SQLDisconnect failure (use for user-facing close()).
     void disconnect();
 
+    // Disconnect without throwing — safe for destructors, pool cleanup, and reset() failure paths.
+    void disconnect_nothrow() noexcept;
+
     // Commit the current transaction.
     void commit();
 
@@ -63,6 +67,10 @@ class Connection {
     void checkError(SQLRETURN ret) const;
     void applyAttrsBefore(const py::dict& attrs_before);
 
+    // Shared disconnect logic: marks child handles, calls SQLDisconnect, frees handle.
+    // Returns the SQLRETURN from SQLDisconnect (or SQL_SUCCESS if no handle).
+    SQLRETURN disconnect_impl() noexcept;
+
     std::wstring _connStr;
     bool _fromPool = false;
     bool _autocommit = true;
@@ -75,13 +83,13 @@ class Connection {
     // Uses weak_ptr to avoid circular references and allow normal cleanup
     // THREAD-SAFETY: All accesses must be guarded by _childHandlesMutex
     std::vector<std::weak_ptr<SqlHandle>> _childStatementHandles;
-    
+
     // Counter for periodic compaction of expired weak_ptrs
     // Compact every N allocations to avoid O(n²) overhead in hot path
     // THREAD-SAFETY: Protected by _childHandlesMutex
     size_t _allocationsSinceCompaction = 0;
     static constexpr size_t COMPACTION_INTERVAL = 100;
-    
+
     // Mutex protecting _childStatementHandles and _allocationsSinceCompaction
     // Prevents data races between allocStatementHandle() and disconnect(),
     // or concurrent GC finalizers running from different threads

mssql_python/pybind/connection/connection_pool.cpp

@@ -21,6 +21,9 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::wstring& connStr,
         auto now = std::chrono::steady_clock::now();
         size_t before = _pool.size();
 
+        LOG("ConnectionPool::acquire: pool_size=%zu, max_size=%zu, idle_timeout=%d", before,
+            _max_size, _idle_timeout_secs);
+
         // Phase 1: Remove stale connections, collect for later disconnect
         _pool.erase(std::remove_if(_pool.begin(), _pool.end(),
                                    [&](const std::shared_ptr<Connection>& conn) {
@@ -69,11 +72,7 @@ std::shared_ptr<Connection> ConnectionPool::acquire(const std::wstring& connStr,
 
     // Phase 3: Disconnect expired/bad connections outside lock
     for (auto& conn : to_disconnect) {
-        try {
-            conn->disconnect();
-        } catch (const std::exception& ex) {
-            LOG("Disconnect bad/expired connections failed: %s", ex.what());
-        }
+        conn->disconnect_nothrow();
     }
     return valid_conn;
 }
@@ -84,7 +83,7 @@ void ConnectionPool::release(std::shared_ptr<Connection> conn) {
         conn->updateLastUsed();
         _pool.push_back(conn);
     } else {
-        conn->disconnect();
+        conn->disconnect_nothrow();
         if (_current_size > 0)
             --_current_size;
     }
@@ -101,11 +100,7 @@ void ConnectionPool::close() {
         _current_size = 0;
     }
     for (auto& conn : to_close) {
-        try {
-            conn->disconnect();
-        } catch (const std::exception& ex) {
-            LOG("ConnectionPool::close: disconnect failed: %s", ex.what());
-        }
+        conn->disconnect_nothrow();
     }
 }
 

mssql_python/pybind/ddbc_bindings.cpp

@@ -5683,10 +5683,8 @@ SQLLEN SQLRowCount_wrap(SqlHandlePtr StatementHandle) {
     return rowCount;
 }
 
-static std::once_flag pooling_init_flag;
 void enable_pooling(int maxSize, int idleTimeout) {
-    std::call_once(pooling_init_flag,
-                   [&]() { ConnectionPoolManager::getInstance().configure(maxSize, idleTimeout); });
+    ConnectionPoolManager::getInstance().configure(maxSize, idleTimeout);
 }
 
 // Thread-safe decimal separator setting