Skip to content

Commit d3b1229

Browse files
subrata-mssubrata-ms
andauthored
FIX: Fixing segmentation fault issue due to double free on SqlHandle::free in Linux (#361)
### Work Item / Issue Reference > [AB#40714](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/40714) <!-- External contributors: GitHub Issue --> > GitHub Issue: #341 ------------------------------------------------------------------- ### Summary **Problem:** Segmentation fault occurs during Python's garbage collection when freeing ODBC handles. **Stack Trace Analysis:** 0-2: Signal handler (SIGSEGV) 3: libmsodbcsql-18.5.so.1.1 - CRASH LOCATION 4: SQLFreeHandle() from ODBC driver 5: SqlHandle::free() from ddbc_bindings 6-11: pybind11 call stack 12: Python method call 20: slot_tp_finalize during __del__ 21-27: Python GC finalizing objects **Root Cause:** The crash occurs when; 1. Python's garbage collector runs during finalization 2. Objects with [__del__] methods are being cleaned up 3. The [__del__] calls [close()] which internally calls SQLFreeHandle() 4. The ODBC driver crashes because: - Handle is already freed (double-free) - Handle is in invalid state - Connection handle freed before cursor statement handle - Wrong finalization order due to circular references **Fix Details:** This pull request introduces a critical fix to the handle cleanup logic in the SQL bindings for Python, specifically addressing potential segfaults during interpreter shutdown. The change ensures that both statement and database connection handles are not freed if Python is shutting down, preventing invalid memory access. Handle cleanup logic improvements: * Updated the `SqlHandle::free()` method in `mssql_python/pybind/ddbc_bindings.cpp` to skip freeing both statement (`SQL_HANDLE_STMT`) and database connection (`SQL_HANDLE_DBC`) handles during Python shutdown, rather than only statement handles. This prevents segfaults caused by freeing handles in the wrong order when their parent resources may have already been released. --------- Co-authored-by: subrata-ms <subrata@microsoft.com>
1 parent a3fde2c commit d3b1229

File tree

4 files changed

+1445
-5
lines changed

4 files changed

+1445
-5
lines changed

mssql_python/__init__.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
This module initializes the mssql_python package.
55
"""
66

7+
import atexit
78
import sys
9+
import threading
810
import types
11+
import weakref
912
from typing import Dict
1013

1114
# Import settings from helpers to avoid circular imports
@@ -67,6 +70,49 @@
6770
# Pooling
6871
from .pooling import PoolingManager
6972

73+
# Global registry for tracking active connections (using weak references)
74+
_active_connections = weakref.WeakSet()
75+
_connections_lock = threading.Lock()
76+
77+
78+
def _register_connection(conn):
79+
"""Register a connection for cleanup before shutdown."""
80+
with _connections_lock:
81+
_active_connections.add(conn)
82+
83+
84+
def _cleanup_connections():
85+
"""
86+
Cleanup function called by atexit to close all active connections.
87+
88+
This prevents resource leaks during interpreter shutdown by ensuring
89+
all ODBC handles are freed in the correct order before Python finalizes.
90+
"""
91+
# Make a copy of the connections to avoid modification during iteration
92+
with _connections_lock:
93+
connections_to_close = list(_active_connections)
94+
95+
for conn in connections_to_close:
96+
try:
97+
# Check if connection is still valid and not closed
98+
if hasattr(conn, "_closed") and not conn._closed:
99+
# Close will handle both cursors and the connection
100+
conn.close()
101+
except Exception as e:
102+
# Log errors during shutdown cleanup for debugging
103+
# We're prioritizing crash prevention over error propagation
104+
try:
105+
driver_logger.error(
106+
f"Error during connection cleanup at shutdown: {type(e).__name__}: {e}"
107+
)
108+
except Exception:
109+
# If logging fails during shutdown, silently ignore
110+
pass
111+
112+
113+
# Register cleanup function to run before Python exits
114+
atexit.register(_cleanup_connections)
115+
70116
# GLOBALS
71117
# Read-Only
72118
apilevel: str = "2.0"

mssql_python/connection.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from typing import Any, Dict, Optional, Union, List, Tuple, Callable, TYPE_CHECKING
1818
import threading
1919

20+
import mssql_python
2021
from mssql_python.cursor import Cursor
2122
from mssql_python.helpers import (
2223
add_driver_to_connection_str,
@@ -312,6 +313,22 @@ def __init__(
312313
)
313314
self.setautocommit(autocommit)
314315

316+
# Register this connection for cleanup before Python shutdown
317+
# This ensures ODBC handles are freed in correct order, preventing leaks
318+
try:
319+
if hasattr(mssql_python, "_register_connection"):
320+
mssql_python._register_connection(self)
321+
except AttributeError as e:
322+
# If registration fails, continue - cleanup will still happen via __del__
323+
logger.warning(
324+
f"Failed to register connection for shutdown cleanup: {type(e).__name__}: {e}"
325+
)
326+
except Exception as e:
327+
# Catch any other unexpected errors during registration
328+
logger.error(
329+
f"Unexpected error during connection registration: {type(e).__name__}: {e}"
330+
)
331+
315332
def _construct_connection_string(self, connection_str: str = "", **kwargs: Any) -> str:
316333
"""
317334
Construct the connection string by parsing, validating, and merging parameters.

mssql_python/pybind/ddbc_bindings.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,11 +1163,15 @@ void SqlHandle::free() {
11631163
// Check if Python is shutting down using centralized helper function
11641164
bool pythonShuttingDown = is_python_finalizing();
11651165

1166-
// CRITICAL FIX: During Python shutdown, don't free STMT handles as
1167-
// their parent DBC may already be freed This prevents segfault when
1168-
// handles are freed in wrong order during interpreter shutdown Type 3 =
1169-
// SQL_HANDLE_STMT, Type 2 = SQL_HANDLE_DBC, Type 1 = SQL_HANDLE_ENV
1170-
if (pythonShuttingDown && _type == 3) {
1166+
// RESOURCE LEAK MITIGATION:
1167+
// When handles are skipped during shutdown, they are not freed, which could
1168+
// cause resource leaks. However, this is mitigated by:
1169+
// 1. Python-side atexit cleanup (in __init__.py) that explicitly closes all
1170+
// connections before shutdown, ensuring handles are freed in correct order
1171+
// 2. OS-level cleanup at process termination recovers any remaining resources
1172+
// 3. This tradeoff prioritizes crash prevention over resource cleanup, which
1173+
// is appropriate since we're already in shutdown sequence
1174+
if (pythonShuttingDown && (_type == SQL_HANDLE_STMT || _type == SQL_HANDLE_DBC)) {
11711175
_handle = nullptr; // Mark as freed to prevent double-free attempts
11721176
return;
11731177
}

0 commit comments

Comments
 (0)