Merge branch 'main' into bewithgaurav/pyproject-modernization

Author: bewithgaurav

PyPI_Description.md

@@ -35,27 +35,11 @@ PyBind11 provides:
 - Memory-safe bindings
 - Clean and Pythonic API, while performance-critical logic remains in robust, maintainable C++.
 
-## What's new in v1.2.0
-
-### Enhancements
-
-- **Connection.closed Property** - Added `Connection.closed` property to check if a connection is closed, improving connection state management.
-
-- **Parameter as Dictionary** - Added support for passing parameters as dictionaries, providing more flexible query parameterization.
-
-- **Copilot Prompts** - Introduced Copilot prompts for AI-assisted development, enhancing developer productivity.
+## What's new in v1.3.0
 
 ### Bug Fixes
 
-- **FetchMany with LOB Columns** - Fixed `fetchmany(n)` ignoring batch size when working with LOB (Large Object) columns.
-
-- **Non-ASCII Path Resolution** - Fixed path resolution for files with non-ASCII characters on Windows.
-
-### CI/Infrastructure
-
-- **SQL Server 2025 Test Support** - Added support for testing against SQL Server 2025 across Windows, macOS, and Linux CI pipelines, ensuring driver compatibility with the upcoming SQL Server release.
-
-- **Forked PR Coverage Workflow** - Implemented coverage comment workflow for pull requests from forked repositories, improving the contribution experience for external contributors.
+- **Segmentation Fault Fix** - Fixed segmentation fault in libmsodbcsql-18.5 during SQLFreeHandle() (#415).
 
 For more information, please visit the project link on Github: https://github.com/microsoft/mssql-python
 

eng/pipelines/pr-validation-pipeline.yml

@@ -56,6 +56,11 @@ jobs:
 
   strategy:
     matrix:
+      pytestonwindows:
+        # Temporary entry to clear stale CodeQL snapshot SM02986
+        # Remove this once the issue is resolved
+        sqlVersion: 'SQL2022'
+        pythonVersion: '3.13'
       SQLServer2022:
         sqlVersion: 'SQL2022'
         pythonVersion: '3.13'

es-metadata.yml

@@ -1,8 +1,12 @@
-schemaVersion: 0.0.1
-isProduction: true
-accountableOwners:
-  service: ae66a2ba-2c8a-4e77-8323-305cfad11f0e
-routing:
-  defaultAreaPath:
-    org: sqlclientdrivers
-    path: mssql-python
+schemaVersion: 1.0.0
+providers:
+- provider: InventoryAsCode
+  version: 1.0.0
+  metadata:
+    isProduction: true
+    accountableOwners:
+      service: ae66a2ba-2c8a-4e77-8323-305cfad11f0e
+    routing:
+      defaultAreaPath:
+        org: sqlclientdrivers
+        path: mssql-python

mssql_python/__init__.py

@@ -15,7 +15,7 @@
 from .helpers import Settings, get_settings, _settings, _settings_lock
 
 # Driver version
-__version__ = "1.2.0"
+__version__ = "1.3.0"
 
 # Exceptions
 # https://www.python.org/dev/peps/pep-0249/#exceptions

mssql_python/pybind/connection/connection.cpp

@@ -94,6 +94,47 @@ void Connection::connect(const py::dict& attrs_before) {
 void Connection::disconnect() {
     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("Marking %zu child statement handles as implicitly freed",
+                _childStatementHandles.size());
+            for (auto& weakHandle : _childStatementHandles) {
+                if (auto handle = weakHandle.lock()) {
+                    // SAFETY ASSERTION: Only STMT handles should be in this vector
+                    // 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());
+                        continue;  // Skip marking to prevent leak
+                    }
+                    handle->markImplicitlyFreed();
+                }
+            }
+            _childStatementHandles.clear();
+            _allocationsSinceCompaction = 0;
+        }  // Release lock before potentially slow SQLDisconnect call
+
         SQLRETURN ret = SQLDisconnect_ptr(_dbcHandle->get());
         checkError(ret);
         // triggers SQLFreeHandle via destructor, if last owner
@@ -173,7 +214,36 @@ SqlHandlePtr Connection::allocStatementHandle() {
     SQLHANDLE stmt = nullptr;
     SQLRETURN ret = SQLAllocHandle_ptr(SQL_HANDLE_STMT, _dbcHandle->get(), &stmt);
     checkError(ret);
-    return std::make_shared<SqlHandle>(static_cast<SQLSMALLINT>(SQL_HANDLE_STMT), stmt);
+    auto stmtHandle = std::make_shared<SqlHandle>(static_cast<SQLSMALLINT>(SQL_HANDLE_STMT), stmt);
+
+    // THREAD-SAFETY: Lock mutex before modifying _childStatementHandles
+    // This protects against concurrent disconnect() or allocStatementHandle() calls,
+    // 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);
+        _allocationsSinceCompaction++;
+
+        // Compact expired weak_ptrs only periodically to avoid O(n²) overhead
+        // This keeps allocation fast (O(1) amortized) while preventing unbounded growth
+        // disconnect() also compacts, so this is just for long-lived connections with many cursors
+        if (_allocationsSinceCompaction >= COMPACTION_INTERVAL) {
+            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());
+            _allocationsSinceCompaction = 0;
+            LOG("Periodic compaction: %zu -> %zu handles (removed %zu expired)",
+                originalSize, _childStatementHandles.size(),
+                originalSize - _childStatementHandles.size());
+        }
+    }  // Release lock
+
+    return stmtHandle;
 }
 
 SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) {
@@ -308,7 +378,7 @@ bool Connection::reset() {
         disconnect();
         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.
@@ -320,7 +390,7 @@ bool Connection::reset() {
         disconnect();
         return false;
     }
-    
+
     updateLastUsed();
     return true;
 }

mssql_python/pybind/connection/connection.h

@@ -5,10 +5,19 @@
 #include "../ddbc_bindings.h"
 #include <memory>
 #include <string>
+#include <mutex>
 
 // Represents a single ODBC database connection.
 // Manages connection handles.
 // Note: This class does NOT implement pooling logic directly.
+//
+// THREADING MODEL (per DB-API 2.0 threadsafety=1):
+// - Connections should NOT be shared between threads in normal usage
+// - However, _childStatementHandles is mutex-protected because:
+//   1. Python GC/finalizers can run from any thread
+//   2. Native code may release GIL during blocking ODBC calls
+//   3. Provides safety if user accidentally shares connection
+// - All accesses to _childStatementHandles are guarded by _childHandlesMutex
 
 class Connection {
   public:
@@ -61,6 +70,22 @@ class Connection {
     std::chrono::steady_clock::time_point _lastUsed;
     std::wstring wstrStringBuffer;  // wstr buffer for string attribute setting
     std::string strBytesBuffer;     // string buffer for byte attributes setting
+
+    // Track child statement handles to mark them as implicitly freed when connection closes
+    // 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
+    mutable std::mutex _childHandlesMutex;
 };
 
 class ConnectionHandle {

mssql_python/pybind/ddbc_bindings.cpp

@@ -1144,6 +1144,21 @@ SQLSMALLINT SqlHandle::type() const {
     return _type;
 }
 
+void SqlHandle::markImplicitlyFreed() {
+    // SAFETY: Only STMT handles should be marked as implicitly freed.
+    // When a DBC handle is freed, the ODBC driver automatically frees all child STMT handles.
+    // Other handle types (ENV, DBC, DESC) are NOT automatically freed by parents.
+    // Calling this on wrong handle types will cause silent handle leaks.
+    if (_type != SQL_HANDLE_STMT) {
+        // Log error but don't throw - we're likely in cleanup/destructor path
+        LOG_ERROR("SAFETY VIOLATION: Attempted to mark non-STMT handle as implicitly freed. "
+                  "Handle type=%d. This will cause handle leak. Only STMT handles are "
+                  "automatically freed by parent DBC handles.", _type);
+        return;  // Refuse to mark - let normal free() handle it
+    }
+    _implicitly_freed = true;
+}
+
 /*
  * IMPORTANT: Never log in destructors - it causes segfaults.
  * During program exit, C++ destructors may run AFTER Python shuts down.
@@ -1169,16 +1184,19 @@ void SqlHandle::free() {
             return;
         }
 
-        // Always clean up ODBC resources, regardless of Python state
+        // CRITICAL FIX: Check if handle was already implicitly freed by parent handle
+        // When Connection::disconnect() frees the DBC handle, the ODBC driver automatically
+        // frees all child STMT handles. We track this state to avoid double-free attempts.
+        // This approach avoids calling ODBC functions on potentially-freed handles, which
+        // would cause use-after-free errors.
+        if (_implicitly_freed) {
+            _handle = nullptr;  // Just clear the pointer, don't call ODBC functions
+            return;
+        }
+
+        // Handle is valid and not implicitly freed, proceed with normal freeing
         SQLFreeHandle_ptr(_type, _handle);
         _handle = nullptr;
-
-        // Only log if Python is not shutting down (to avoid segfault)
-        if (!pythonShuttingDown) {
-            // Don't log during destruction - even in normal cases it can be
-            // problematic If logging is needed, use explicit close() methods
-            // instead
-        }
     }
 }
 
@@ -2893,7 +2911,6 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
 
     // Cache decimal separator to avoid repeated system calls
 
-
     for (SQLSMALLINT i = 1; i <= colCount; ++i) {
         SQLWCHAR columnName[256];
         SQLSMALLINT columnNameLen;
@@ -3615,8 +3632,6 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
             columnInfos[col].processedColumnSize + 1;  // +1 for null terminator
     }
 
-
-
     // Performance: Build function pointer dispatch table (once per batch)
     // This eliminates the switch statement from the hot loop - 10,000 rows × 10
     // cols reduces from 100,000 switch evaluations to just 10 switch
@@ -4033,8 +4048,8 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch
             lobColumns.push_back(i + 1);  // 1-based
         }
     }
-    
-    // Initialized to 0 for LOB path counter; overwritten by ODBC in non-LOB path; 
+
+    // Initialized to 0 for LOB path counter; overwritten by ODBC in non-LOB path;
     SQLULEN numRowsFetched = 0;
     // If we have LOBs → fall back to row-by-row fetch + SQLGetData_wrap
     if (!lobColumns.empty()) {
@@ -4066,7 +4081,7 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch
         LOG("FetchMany_wrap: Error when binding columns - SQLRETURN=%d", ret);
         return ret;
     }
-    
+
     SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)(intptr_t)fetchSize, 0);
     SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROWS_FETCHED_PTR, &numRowsFetched, 0);
 

mssql_python/pybind/ddbc_bindings.h

@@ -379,9 +379,24 @@ class SqlHandle {
     SQLSMALLINT type() const;
     void free();
 
+    // Mark this handle as implicitly freed (freed by parent handle)
+    // This prevents double-free attempts when the ODBC driver automatically
+    // frees child handles (e.g., STMT handles when DBC handle is freed)
+    //
+    // SAFETY CONSTRAINTS:
+    // - ONLY call this on SQL_HANDLE_STMT handles
+    // - ONLY call this when the parent DBC handle is about to be freed
+    // - Calling on other handle types (ENV, DBC, DESC) will cause HANDLE LEAKS
+    // - The ODBC spec only guarantees automatic freeing of STMT handles by DBC parents
+    //
+    // Current usage: Connection::disconnect() marks all tracked STMT handles
+    // before freeing the DBC handle.
+    void markImplicitlyFreed();
+
   private:
     SQLSMALLINT _type;
     SQLHANDLE _handle;
+    bool _implicitly_freed = false;  // Tracks if handle was freed by parent
 };
 using SqlHandlePtr = std::shared_ptr<SqlHandle>;