Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions mssql_python/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,17 @@ def __del__(self):
"""
if "_closed" not in self.__dict__ or not self._closed:
try:
import sys
# If the interpreter is finalizing, skip cleanup that might call into
# C extension destructors which use the Python C-API.
if sys and getattr(sys, "_is_finalizing", lambda: False)():
return
self.close()
except Exception as e:
# Dont raise exceptions from __del__ to avoid issues during garbage collection
log('error', f"Error during connection cleanup: {e}")
# During interpreter shutdown avoid logging or raising further errors
try:
if not (hasattr(sys, "_is_finalizing") and sys._is_finalizing()):
log('error', f"Error during connection cleanup: {e}")
except Exception:
# If logging isn't available (e.g., during shutdown), silently ignore
pass
43 changes: 37 additions & 6 deletions mssql_python/pybind/connection/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,21 @@ Connection::Connection(const std::wstring& conn_str, bool use_pool)
}

Connection::~Connection() {
disconnect(); // fallback if user forgets to disconnect
// During interpreter shutdown, destructors must not throw or call into Python runtime.
// Call disconnect() but swallow exceptions to avoid terminate() during stack unwinding.
try {
disconnect(); // fallback if user forgets to disconnect
} catch (const std::exception &e) {
// Avoid using LOG (acquires GIL). Write to stderr directly only if Python is alive.
if (Py_IsInitialized()) {
std::cerr << "Exception during Connection::~Connection cleanup: " << e.what() << std::endl;
}
// Swallow - can't safely call Python APIs here.
} catch (...) {
if (Py_IsInitialized()) {
std::cerr << "Unknown exception during Connection::~Connection cleanup" << std::endl;
}
}
}

// Allocates connection handle
Expand Down Expand Up @@ -89,12 +103,22 @@ void Connection::connect(const py::dict& attrs_before) {

void Connection::disconnect() {
if (_dbcHandle) {
// Use LOG for normal operations where Python is available
LOG("Disconnecting from database");
SQLRETURN ret = SQLDisconnect_ptr(_dbcHandle->get());
checkError(ret);
try {
SQLRETURN ret = SQLDisconnect_ptr(_dbcHandle->get());
// checkError may throw; catch and write to stderr instead of calling LOG
try {
checkError(ret);
} catch (const std::exception &e) {
std::cerr << "SQLDisconnect reported error: " << e.what() << std::endl;
// swallow - destructor callers must not throw
}
} catch (const std::exception &e) {
std::cerr << "Exception during SQLDisconnect call: " << e.what() << std::endl;
}
_dbcHandle.reset(); // triggers SQLFreeHandle via destructor, if last owner
}
else {
} else {
LOG("No connection handle to disconnect");
}
}
Expand Down Expand Up @@ -270,8 +294,15 @@ ConnectionHandle::ConnectionHandle(const std::string& connStr, bool usePool, con
}

ConnectionHandle::~ConnectionHandle() {
// Make destructor noexcept-safe: close but swallow exceptions.
if (_conn) {
close();
try {
close();
} catch (const std::exception &e) {
LOG("Exception during ConnectionHandle destructor close: {}", e.what());
} catch (...) {
LOG("Unknown exception during ConnectionHandle destructor close");
}
}
}

Expand Down
32 changes: 32 additions & 0 deletions tests/test_005_connection_cursor_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,38 @@ def test_sql_syntax_error_no_segfault_on_shutdown(conn_str):
# Should not segfault (exit code 139 on Unix, 134 on macOS)
assert result.returncode == 1, f"Expected exit code 1 due to syntax error, but got {result.returncode}. STDERR: {result.stderr}"


def test_connection_error_then_shutdown_no_core_dump(conn_str):
"""Test that connection-level errors followed by interpreter shutdown do not cause core dumps or pybind11 aborts"""
escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"')
code = f"""
from mssql_python import connect

# Create a connection, trigger an error on the native side, then let interpreter shutdown handle cleanup
conn = connect("{escaped_conn_str}")
cursor = conn.cursor()

# Execute an invalid query to create an error state on the STMT/DBC
try:
cursor.execute('syntax error at shutdown_test')
except Exception:
# Intentionally ignore the Python-level exception; we want to ensure teardown is safe
pass

# Don't close cursor or connection explicitly - allow objects to be cleaned up during interpreter finalization
print('done')
"""

result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True)

# Expect a non-zero return value due to the syntax error, but not a crash/abort
assert result.returncode != 0, f"Expected non-zero exit due to syntax error, got 0. STDERR: {result.stderr}"

# Ensure there is no segmentation fault or pybind11 abort text in stderr
assert "Segmentation fault" not in result.stderr
assert "pybind11::error_already_set" not in result.stderr
assert "sys.meta_path is None" not in result.stderr

def test_multiple_sql_syntax_errors_no_segfault(conn_str):
"""Test multiple SQL syntax errors don't cause segfault during cleanup"""
escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"')
Expand Down
Loading