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

[subrata-ms]

Work Item / Issue Reference

AB#40818

GitHub Issue: #<ISSUE_NUMBER>

---

Summary

This pull request improves the robustness, reliability, and debuggability of the connection pooling and cleanup logic in the SQL Server Python driver. It introduces safer error handling in destructors, enhances logging for connection pool operations, and makes the pooling tests more reliable and meaningful by using unique connection identifiers and removing unnecessary skips.

Error handling and resource management:

• The Connection destructor now safely handles exceptions from disconnect() by logging and suppressing them, preventing potential crashes due to exceptions during stack unwinding.
• In disconnect(), errors from SQLDisconnect are logged instead of thrown, ensuring cleanup is always attempted and never throws from contexts like destructors or pool cleanup.

Logging and diagnostics:

• Added detailed logging in the connection pool's acquire() method to track pool size, idle timeout, and connection idle times, aiding in diagnosing pool behavior.
• Improved formatting and consistency of log messages in connection handle compaction routines. [1] [2]
• Enhanced error log formatting for non-statement handle detection in disconnect().

Connection pooling configuration:

• Removed the unnecessary std::once_flag from enable_pooling(), allowing reconfiguration of the pool for each test run.

Testing improvements:

• Re-enabled and rewrote previously skipped pooling tests to be more reliable:
  • test_pool_idle_timeout_removes_connections now uses connection_id instead of @@SPID to uniquely identify physical connections and increases the idle wait to avoid flakiness in CI environments.
  • test_pool_removes_invalid_connections is now robust, directly simulates a dirty connection, and verifies that the pool discards invalid connections and creates new ones using connection_id.
  • Improved password replacement logic in test_pool_recovery_after_failed_connection for broader compatibility with different connection string formats.

[Copilot]

Pull request overview

This PR aims to make connection pooling behavior easier to diagnose and make pooling-related tests more reliable, while also hardening cleanup paths (notably destructors / disconnect) to avoid crashes during shutdown or error unwinding.

Changes:

• Updates pooling tests to avoid SPID reuse issues and re-enables previously skipped pooling recovery tests.
• Makes Connection cleanup paths more destructor-safe by suppressing exceptions and logging failures.
• Adjusts pooling configuration to be reconfigurable across test runs and adds more pool acquisition diagnostics.

Reviewed changes

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

File	Description
tests/test_009_pooling.py	Reworks idle-timeout and invalid-connection pooling tests; improves bad-password mutation logic.
mssql_python/pybind/ddbc_bindings.cpp	Allows enable_pooling() to reconfigure defaults on each call (removes call_once).
mssql_python/pybind/connection/connection.cpp	Makes destructor/disconnect safer by suppressing throws and improving log formatting.
mssql_python/pybind/connection/connection_pool.cpp	Adds detailed debug logging around pool acquisition and idle pruning.

---

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

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 80%

🎯 Overall Coverage 77%

📈 Total Lines Covered: 5791 out of 7438 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/pybind/connection/connection.cpp (65.2%): Missing lines 59-60,132-135,160-161
• mssql_python/pybind/connection/connection_pool.cpp (100%)
• mssql_python/pybind/ddbc_bindings.cpp (100%)

Summary

• Total: 40 lines
• Missing: 8 lines
• Coverage: 80%

mssql_python/pybind/connection/connection.cpp

Lines 55-64

  55         disconnect();  // fallback if user forgets to disconnect
  56     } catch (...) {
  57         // Never throw from a destructor — doing so during stack unwinding
  58         // causes std::terminate().  Log and swallow.
! 59         LOG_ERROR("Exception suppressed in ~Connection destructor");
! 60     }
  61 }
  62 
  63 // Allocates connection handle
  64 void Connection::allocateDbcHandle() {

---

Lines 128-139

  128                     // SAFETY ASSERTION: Only STMT handles should be in this vector
  129                     // This is guaranteed by allocStatementHandle() which only creates STMT handles
  130                     // If this assertion fails, it indicates a serious bug in handle tracking
  131                     if (handle->type() != SQL_HANDLE_STMT) {
! 132                         LOG_ERROR(
! 133                             "CRITICAL: Non-STMT handle (type=%d) found in _childStatementHandles. "
! 134                             "This will cause a handle leak!",
! 135                             handle->type());
  136                         continue;  // Skip marking to prevent leak
  137                     }
  138                     handle->markImplicitlyFreed();
  139                 }

---

Lines 156-165

  156                 std::string diagMsg = WideToUTF8(err.ddbcErrorMsg);
  157                 LOG_ERROR("SQLDisconnect diagnostics: %s", diagMsg.c_str());
  158             } catch (...) {
  159                 // Swallow all exceptions: cleanup paths must not throw.
! 160                 LOG_ERROR("SQLDisconnect: failed to retrieve ODBC diagnostics");
! 161             }
  162         }
  163         // Always free the handle regardless of SQLDisconnect result
  164         _dbcHandle.reset();
  165     } else {

---

---

📋 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.pybind.ddbc_bindings.cpp: 70.4%
mssql_python.row.py: 70.5%
mssql_python.__init__.py: 77.1%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 85.2%
mssql_python.cursor.py: 85.6%
mssql_python.helpers.py: 89.3%
mssql_python.auth.py: 89.8%

---

🔗 Quick Links

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