From b3bf2a76eb6fd0330117691a5a467a057d1abef0 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 24 Sep 2025 19:33:05 +0530 Subject: [PATCH 1/8] FIX: Resource cleanup post failed cursor operations --- mssql_python/pybind/ddbc_bindings.cpp | 70 ++++++++++++--- tests/test_005_connection_cursor_lifecycle.py | 86 +++++++++++++++++++ 2 files changed, 145 insertions(+), 11 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index e5c979b7..40fc91dd 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -665,21 +665,43 @@ void HandleZeroColumnSizeAtFetch(SQLULEN& columnSize) { // TODO: Revisit GIL considerations if we're using python's logger template void LOG(const std::string& formatString, Args&&... args) { - py::gil_scoped_acquire gil; // <---- this ensures safe Python API usage + // Check if Python is shutting down to avoid crash during cleanup + try { + if (Py_IsInitialized() == 0) { + return; // Python is already shut down + } + + py::gil_scoped_acquire gil; // <---- this ensures safe Python API usage + + // Check if sys module is available and not finalizing + py::object sys_module = py::module_::import("sys"); + if (!sys_module.is_none()) { + py::object finalizing_func = sys_module.attr("_is_finalizing"); + if (!finalizing_func.is_none() && finalizing_func().cast()) { + return; // Python is finalizing, don't log + } + } - py::object logger = py::module_::import("mssql_python.logging_config").attr("get_logger")(); - if (py::isinstance(logger)) return; + py::object logger = py::module_::import("mssql_python.logging_config").attr("get_logger")(); + if (py::isinstance(logger)) return; - try { - std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; - if constexpr (sizeof...(args) == 0) { - logger.attr("debug")(py::str(ddbcFormatString)); - } else { - py::str message = py::str(ddbcFormatString).format(std::forward(args)...); - logger.attr("debug")(message); + try { + std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; + if constexpr (sizeof...(args) == 0) { + logger.attr("debug")(py::str(ddbcFormatString)); + } else { + py::str message = py::str(ddbcFormatString).format(std::forward(args)...); + logger.attr("debug")(message); + } + } catch (const std::exception& e) { + std::cerr << "Logging error: " << e.what() << std::endl; } + } catch (const py::error_already_set& e) { + // Python is shutting down or in an inconsistent state, silently ignore + return; } catch (const std::exception& e) { - std::cerr << "Logging error: " << e.what() << std::endl; + // Any other error, ignore to prevent crash during cleanup + return; } } @@ -990,6 +1012,32 @@ SQLSMALLINT SqlHandle::type() const { */ void SqlHandle::free() { if (_handle && SQLFreeHandle_ptr) { + // Check if Python is shutting down - if so, don't call into ODBC driver + // as it may have already been unloaded or become invalid + try { + if (Py_IsInitialized() == 0) { + // Python is shut down, just clear our handle without calling driver + _handle = nullptr; + return; + } + + // Additional check for Python finalization state + py::gil_scoped_acquire gil; + py::object sys_module = py::module_::import("sys"); + if (!sys_module.is_none()) { + py::object finalizing_func = sys_module.attr("_is_finalizing"); + if (!finalizing_func.is_none() && finalizing_func().cast()) { + // Python is finalizing, don't call driver + _handle = nullptr; + return; + } + } + } catch (...) { + // Any exception means Python is in bad state, don't call driver + _handle = nullptr; + return; + } + const char* type_str = nullptr; switch (_type) { case SQL_HANDLE_ENV: type_str = "ENV"; break; diff --git a/tests/test_005_connection_cursor_lifecycle.py b/tests/test_005_connection_cursor_lifecycle.py index d87c3f21..d01f2413 100644 --- a/tests/test_005_connection_cursor_lifecycle.py +++ b/tests/test_005_connection_cursor_lifecycle.py @@ -475,3 +475,89 @@ def test_mixed_cursor_cleanup_scenarios(conn_str, tmp_path): assert "All tests passed" in result.stdout # Should not have error logs assert "Exception during cursor cleanup" not in result.stderr + + +def test_sql_syntax_error_no_segfault_on_shutdown(conn_str): + """Test that SQL syntax errors don't cause segfault during Python shutdown""" + # This test reproduces the exact scenario that was causing segfaults + escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') + code = f""" +from mssql_python import connect + +try: + # Create connection + conn = connect("{escaped_conn_str}") + cursor = conn.cursor() + + # Execute invalid SQL that causes syntax error - this was causing segfault + cursor.execute("syntax error") + +except Exception as e: + print(f"Caught expected SQL error: {{type(e).__name__}}") + +# Don't explicitly close connection/cursor - let Python shutdown handle cleanup +# This is where the segfault was occurring before the fix +print("Script completed, shutting down...") +# Segfault would happen here during Python shutdown +""" + + # Run in subprocess to catch segfaults + result = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True + ) + + # Should not segfault (exit code 139 on Unix, 134 on macOS) + assert result.returncode == 0, f"Expected clean shutdown, but got exit code {result.returncode}. STDERR: {result.stderr}" + assert "Caught expected SQL error" in result.stdout + assert "Script completed, shutting down..." in result.stdout + # Should not have segfault indicators + assert "segmentation fault" not in result.stderr.lower() + assert "segfault" not in result.stderr.lower() + + +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('"', '\\"') + code = f""" +from mssql_python import connect + +try: + conn = connect("{escaped_conn_str}") + + # Multiple cursors with syntax errors + cursors = [] + for i in range(3): + cursor = conn.cursor() + cursors.append(cursor) + try: + cursor.execute(f"invalid sql syntax {{i}}") + except Exception as e: + print(f"Cursor {{i}} error: {{type(e).__name__}}") + + # Mix of syntax errors and valid queries + cursor_valid = conn.cursor() + cursor_valid.execute("SELECT 1") + cursor_valid.fetchall() + cursors.append(cursor_valid) + +except Exception as e: + print(f"Connection error: {{type(e).__name__}}") + +# Don't close anything - test Python shutdown cleanup +print("Multiple syntax errors handled, shutting down...") +""" + + result = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True + ) + + assert result.returncode == 0, f"Expected clean shutdown after multiple syntax errors, got exit code {result.returncode}. STDERR: {result.stderr}" + assert "Multiple syntax errors handled, shutting down..." in result.stdout + # Should handle multiple syntax errors without segfault + assert "Cursor 0 error:" in result.stdout + assert "Cursor 1 error:" in result.stdout + assert "Cursor 2 error:" in result.stdout From 877d4b4b8fb9ba5f44a0b18919319850e9e3608f Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 24 Sep 2025 21:47:57 +0530 Subject: [PATCH 2/8] unused variable fix --- mssql_python/pybind/ddbc_bindings.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 40fc91dd..76208d47 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -698,9 +698,11 @@ void LOG(const std::string& formatString, Args&&... args) { } } catch (const py::error_already_set& e) { // Python is shutting down or in an inconsistent state, silently ignore + (void)e; // Suppress unused variable warning return; } catch (const std::exception& e) { // Any other error, ignore to prevent crash during cleanup + (void)e; // Suppress unused variable warning return; } } From ef2f9eec8283f18d88fa091c114f2e75b8d3f7ec Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 25 Sep 2025 12:24:15 +0530 Subject: [PATCH 3/8] fix --- mssql_python/pybind/ddbc_bindings.cpp | 48 +++++++++++---------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 76208d47..e23ca1a7 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1014,43 +1014,35 @@ SQLSMALLINT SqlHandle::type() const { */ void SqlHandle::free() { if (_handle && SQLFreeHandle_ptr) { - // Check if Python is shutting down - if so, don't call into ODBC driver - // as it may have already been unloaded or become invalid + // Check if Python is shutting down - if so, skip logging but still clean up ODBC resources + bool pythonShuttingDown = false; try { if (Py_IsInitialized() == 0) { - // Python is shut down, just clear our handle without calling driver - _handle = nullptr; - return; - } - - // Additional check for Python finalization state - py::gil_scoped_acquire gil; - py::object sys_module = py::module_::import("sys"); - if (!sys_module.is_none()) { - py::object finalizing_func = sys_module.attr("_is_finalizing"); - if (!finalizing_func.is_none() && finalizing_func().cast()) { - // Python is finalizing, don't call driver - _handle = nullptr; - return; + pythonShuttingDown = true; + } else { + py::gil_scoped_acquire gil; + py::object sys_module = py::module_::import("sys"); + if (!sys_module.is_none()) { + py::object finalizing_func = sys_module.attr("_is_finalizing"); + if (!finalizing_func.is_none() && finalizing_func().cast()) { + pythonShuttingDown = true; + } } } } catch (...) { - // Any exception means Python is in bad state, don't call driver - _handle = nullptr; - return; + // Any exception during Python state check means Python is likely shutting down + pythonShuttingDown = true; } - const char* type_str = nullptr; - switch (_type) { - case SQL_HANDLE_ENV: type_str = "ENV"; break; - case SQL_HANDLE_DBC: type_str = "DBC"; break; - case SQL_HANDLE_STMT: type_str = "STMT"; break; - case SQL_HANDLE_DESC: type_str = "DESC"; break; - default: type_str = "UNKNOWN"; break; - } + // Always clean up ODBC resources, regardless of Python state SQLFreeHandle_ptr(_type, _handle); _handle = nullptr; - // Don't log during destruction - it can cause segfaults during Python shutdown + + // 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 + } } } From 099047633b9c0753eab963bd856815904d8d8ba8 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 25 Sep 2025 15:38:03 +0530 Subject: [PATCH 4/8] added fix and tests --- mssql_python/pybind/ddbc_bindings.cpp | 8 +++ tests/test_005_connection_cursor_lifecycle.py | 69 +++++++------------ 2 files changed, 31 insertions(+), 46 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index e23ca1a7..e2edf0c3 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1034,6 +1034,14 @@ void SqlHandle::free() { pythonShuttingDown = true; } + // CRITICAL FIX: During Python shutdown, don't free STMT handles as their parent DBC may already be freed + // This prevents segfault when handles are freed in wrong order during interpreter shutdown + // Type 3 = SQL_HANDLE_STMT, Type 2 = SQL_HANDLE_DBC, Type 1 = SQL_HANDLE_ENV + if (pythonShuttingDown && _type == 3) { + _handle = nullptr; // Mark as freed to prevent double-free attempts + return; + } + // Always clean up ODBC resources, regardless of Python state SQLFreeHandle_ptr(_type, _handle); _handle = nullptr; diff --git a/tests/test_005_connection_cursor_lifecycle.py b/tests/test_005_connection_cursor_lifecycle.py index d01f2413..a5b8fa81 100644 --- a/tests/test_005_connection_cursor_lifecycle.py +++ b/tests/test_005_connection_cursor_lifecycle.py @@ -484,20 +484,15 @@ def test_sql_syntax_error_no_segfault_on_shutdown(conn_str): code = f""" from mssql_python import connect -try: - # Create connection - conn = connect("{escaped_conn_str}") - cursor = conn.cursor() - - # Execute invalid SQL that causes syntax error - this was causing segfault - cursor.execute("syntax error") - -except Exception as e: - print(f"Caught expected SQL error: {{type(e).__name__}}") +# Create connection +conn = connect("{escaped_conn_str}") +cursor = conn.cursor() -# Don't explicitly close connection/cursor - let Python shutdown handle cleanup -# This is where the segfault was occurring before the fix -print("Script completed, shutting down...") +# Execute invalid SQL that causes syntax error - this was causing segfault +cursor.execute("syntax error") + +# Don't explicitly close cursor/connection - let Python shutdown handle cleanup +print("Script completed, shutting down...") # This would NOT print anyways # Segfault would happen here during Python shutdown """ @@ -510,12 +505,6 @@ 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 == 0, f"Expected clean shutdown, but got exit code {result.returncode}. STDERR: {result.stderr}" - assert "Caught expected SQL error" in result.stdout - assert "Script completed, shutting down..." in result.stdout - # Should not have segfault indicators - assert "segmentation fault" not in result.stderr.lower() - assert "segfault" not in result.stderr.lower() - def test_multiple_sql_syntax_errors_no_segfault(conn_str): """Test multiple SQL syntax errors don't cause segfault during cleanup""" @@ -523,28 +512,21 @@ def test_multiple_sql_syntax_errors_no_segfault(conn_str): code = f""" from mssql_python import connect -try: - conn = connect("{escaped_conn_str}") - - # Multiple cursors with syntax errors - cursors = [] - for i in range(3): - cursor = conn.cursor() - cursors.append(cursor) - try: - cursor.execute(f"invalid sql syntax {{i}}") - except Exception as e: - print(f"Cursor {{i}} error: {{type(e).__name__}}") - - # Mix of syntax errors and valid queries - cursor_valid = conn.cursor() - cursor_valid.execute("SELECT 1") - cursor_valid.fetchall() - cursors.append(cursor_valid) - -except Exception as e: - print(f"Connection error: {{type(e).__name__}}") +conn = connect("{escaped_conn_str}") + +# Multiple cursors with syntax errors +cursors = [] +for i in range(3): + cursor = conn.cursor() + cursors.append(cursor) + cursor.execute(f"invalid sql syntax {{i}}") +# Mix of syntax errors and valid queries +cursor_valid = conn.cursor() +cursor_valid.execute("SELECT 1") +cursor_valid.fetchall() +cursors.append(cursor_valid) + # Don't close anything - test Python shutdown cleanup print("Multiple syntax errors handled, shutting down...") """ @@ -555,9 +537,4 @@ def test_multiple_sql_syntax_errors_no_segfault(conn_str): text=True ) - assert result.returncode == 0, f"Expected clean shutdown after multiple syntax errors, got exit code {result.returncode}. STDERR: {result.stderr}" - assert "Multiple syntax errors handled, shutting down..." in result.stdout - # Should handle multiple syntax errors without segfault - assert "Cursor 0 error:" in result.stdout - assert "Cursor 1 error:" in result.stdout - assert "Cursor 2 error:" in result.stdout + assert result.returncode == 0, f"Expected clean shutdown, but got exit code {result.returncode}. STDERR: {result.stderr}" From 0694bb22ee5a05bf747002a7ac9fe72be6db7e22 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 25 Sep 2025 21:41:25 +0530 Subject: [PATCH 5/8] added last fix for connection busy, and corrected test --- mssql_python/pybind/ddbc_bindings.cpp | 15 ++++++++++----- tests/test_005_connection_cursor_lifecycle.py | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index e2edf0c3..1a84fef4 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1016,22 +1016,27 @@ void SqlHandle::free() { if (_handle && SQLFreeHandle_ptr) { // Check if Python is shutting down - if so, skip logging but still clean up ODBC resources bool pythonShuttingDown = false; + try { if (Py_IsInitialized() == 0) { pythonShuttingDown = true; } else { + // Try to check sys._is_finalizing(), but don't fail if it doesn't exist py::gil_scoped_acquire gil; py::object sys_module = py::module_::import("sys"); if (!sys_module.is_none()) { - py::object finalizing_func = sys_module.attr("_is_finalizing"); - if (!finalizing_func.is_none() && finalizing_func().cast()) { - pythonShuttingDown = true; + // Check if the attribute exists before accessing it + if (py::hasattr(sys_module, "_is_finalizing")) { + py::object finalizing_func = sys_module.attr("_is_finalizing"); + if (!finalizing_func.is_none() && finalizing_func().cast()) { + pythonShuttingDown = true; + } } } } } catch (...) { - // Any exception during Python state check means Python is likely shutting down - pythonShuttingDown = true; + // Only consider it shutdown if we absolutely can't check Python state + // Be more conservative - don't assume shutdown on any exception } // CRITICAL FIX: During Python shutdown, don't free STMT handles as their parent DBC may already be freed diff --git a/tests/test_005_connection_cursor_lifecycle.py b/tests/test_005_connection_cursor_lifecycle.py index a5b8fa81..0612767c 100644 --- a/tests/test_005_connection_cursor_lifecycle.py +++ b/tests/test_005_connection_cursor_lifecycle.py @@ -504,7 +504,7 @@ 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 == 0, f"Expected clean shutdown, but got exit code {result.returncode}. STDERR: {result.stderr}" + assert result.returncode == 1, f"Expected exit code 1 due to syntax error, but got {result.returncode}. STDERR: {result.stderr}" def test_multiple_sql_syntax_errors_no_segfault(conn_str): """Test multiple SQL syntax errors don't cause segfault during cleanup""" @@ -537,4 +537,4 @@ def test_multiple_sql_syntax_errors_no_segfault(conn_str): text=True ) - assert result.returncode == 0, f"Expected clean shutdown, but got exit code {result.returncode}. STDERR: {result.stderr}" + assert result.returncode == 1, f"Expected exit code 1 due to syntax errors, but got {result.returncode}. STDERR: {result.stderr}" From 55f33495aa1541ffa76c91617315e5a56eef27ff Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 25 Sep 2025 22:29:24 +0530 Subject: [PATCH 6/8] more tests --- tests/test_005_connection_cursor_lifecycle.py | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/tests/test_005_connection_cursor_lifecycle.py b/tests/test_005_connection_cursor_lifecycle.py index 0612767c..df777c08 100644 --- a/tests/test_005_connection_cursor_lifecycle.py +++ b/tests/test_005_connection_cursor_lifecycle.py @@ -538,3 +538,149 @@ def test_multiple_sql_syntax_errors_no_segfault(conn_str): ) assert result.returncode == 1, f"Expected exit code 1 due to syntax errors, but got {result.returncode}. STDERR: {result.stderr}" + + +def test_connection_close_during_active_query_no_segfault(conn_str): + """Test closing connection while cursor has pending results doesn't cause segfault""" + escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') + code = f""" +from mssql_python import connect + +# Create connection and cursor +conn = connect("{escaped_conn_str}") +cursor = conn.cursor() + +# Execute query but don't fetch results - leave them pending +cursor.execute("SELECT COUNT(*) FROM sys.objects") + +# Close connection while results are still pending +# This tests handle cleanup when STMT has pending results but DBC is freed +conn.close() + +print("Connection closed with pending cursor results") +# Cursor destructor will run during normal cleanup, not shutdown +""" + + result = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True + ) + + # Should not segfault - should exit cleanly + assert result.returncode == 0, f"Expected clean exit, but got exit code {result.returncode}. STDERR: {result.stderr}" + assert "Connection closed with pending cursor results" in result.stdout + + +def test_concurrent_cursor_operations_no_segfault(conn_str): + """Test concurrent cursor operations don't cause segfaults or race conditions""" + escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') + code = f""" +import threading +from mssql_python import connect + +conn = connect("{escaped_conn_str}") +results = [] +exceptions = [] + +def worker(thread_id): + try: + for i in range(15): + cursor = conn.cursor() + cursor.execute(f"SELECT {{thread_id * 100 + i}} as value") + result = cursor.fetchone() + results.append(result[0]) + # Don't explicitly close cursor - test concurrent destructors + except Exception as e: + exceptions.append(f"Thread {{thread_id}}: {{e}}") + +# Create multiple threads doing concurrent cursor operations +threads = [] +for i in range(4): + t = threading.Thread(target=worker, args=(i,)) + threads.append(t) + t.start() + +for t in threads: + t.join() + +print(f"Completed: {{len(results)}} results, {{len(exceptions)}} exceptions") + +# Report any exceptions for debugging +for exc in exceptions: + print(f"Exception: {{exc}}") + +print("Concurrent operations completed") +""" + + result = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True + ) + + # Should not segfault + assert result.returncode == 0, f"Expected clean exit, but got exit code {result.returncode}. STDERR: {result.stderr}" + assert "Concurrent operations completed" in result.stdout + + # Check that most operations completed successfully + # Allow for some exceptions due to threading, but shouldn't be many + output_lines = result.stdout.split('\n') + completed_line = [line for line in output_lines if 'Completed:' in line] + if completed_line: + # Extract numbers from "Completed: X results, Y exceptions" + import re + match = re.search(r'Completed: (\d+) results, (\d+) exceptions', completed_line[0]) + if match: + results_count = int(match.group(1)) + exceptions_count = int(match.group(2)) + # Should have completed most operations (allow some threading issues) + assert results_count >= 50, f"Too few successful operations: {results_count}" + assert exceptions_count <= 10, f"Too many exceptions: {exceptions_count}" + + +def test_aggressive_threading_abrupt_exit_no_segfault(conn_str): + """Test abrupt exit with active threads and pending queries doesn't cause segfault""" + escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') + code = f""" +import threading +import sys +import time +from mssql_python import connect + +conn = connect("{escaped_conn_str}") + +def aggressive_worker(thread_id): + '''Worker that creates cursors with pending results and doesn't clean up''' + for i in range(8): + cursor = conn.cursor() + # Execute query but don't fetch - leave results pending + cursor.execute(f"SELECT COUNT(*) FROM sys.objects WHERE object_id > {{thread_id * 1000 + i}}") + + # Create another cursor immediately without cleaning up the first + cursor2 = conn.cursor() + cursor2.execute(f"SELECT TOP 3 * FROM sys.objects WHERE object_id > {{thread_id * 1000 + i}}") + + # Don't fetch results, don't close cursors - maximum chaos + time.sleep(0.005) # Let other threads interleave + +# Start multiple daemon threads +for i in range(3): + t = threading.Thread(target=aggressive_worker, args=(i,), daemon=True) + t.start() + +# Let them run briefly then exit abruptly +time.sleep(0.3) +print("Exiting abruptly with active threads and pending queries") +sys.exit(0) # Abrupt exit without joining threads +""" + + result = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True + ) + + # Should not segfault - should exit cleanly even with abrupt exit + assert result.returncode == 0, f"Expected clean exit, but got exit code {result.returncode}. STDERR: {result.stderr}" + assert "Exiting abruptly with active threads and pending queries" in result.stdout From a1b8b057c9e5a6a612ab24a08eba04f52acea670 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 25 Sep 2025 22:36:30 +0530 Subject: [PATCH 7/8] is_python_finalizing() is a new function now --- mssql_python/pybind/ddbc_bindings.cpp | 66 +++++++++++++-------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 1a84fef4..1d6d4245 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -662,25 +662,43 @@ void HandleZeroColumnSizeAtFetch(SQLULEN& columnSize) { } // namespace -// TODO: Revisit GIL considerations if we're using python's logger -template -void LOG(const std::string& formatString, Args&&... args) { - // Check if Python is shutting down to avoid crash during cleanup +// Helper function to check if Python is shutting down or finalizing +// This centralizes the shutdown detection logic to avoid code duplication +static bool is_python_finalizing() { try { if (Py_IsInitialized() == 0) { - return; // Python is already shut down + return true; // Python is already shut down } - py::gil_scoped_acquire gil; // <---- this ensures safe Python API usage - - // Check if sys module is available and not finalizing + py::gil_scoped_acquire gil; py::object sys_module = py::module_::import("sys"); if (!sys_module.is_none()) { - py::object finalizing_func = sys_module.attr("_is_finalizing"); - if (!finalizing_func.is_none() && finalizing_func().cast()) { - return; // Python is finalizing, don't log + // Check if the attribute exists before accessing it (for Python version compatibility) + if (py::hasattr(sys_module, "_is_finalizing")) { + py::object finalizing_func = sys_module.attr("_is_finalizing"); + if (!finalizing_func.is_none() && finalizing_func().cast()) { + return true; // Python is finalizing + } } } + return false; + } catch (...) { + // Be conservative - don't assume shutdown on any exception + // Only return true if we're absolutely certain Python is shutting down + return false; + } +} + +// TODO: Revisit GIL considerations if we're using python's logger +template +void LOG(const std::string& formatString, Args&&... args) { + // Check if Python is shutting down to avoid crash during cleanup + if (is_python_finalizing()) { + return; // Python is shutting down or finalizing, don't log + } + + try { + py::gil_scoped_acquire gil; // <---- this ensures safe Python API usage py::object logger = py::module_::import("mssql_python.logging_config").attr("get_logger")(); if (py::isinstance(logger)) return; @@ -1014,30 +1032,8 @@ SQLSMALLINT SqlHandle::type() const { */ void SqlHandle::free() { if (_handle && SQLFreeHandle_ptr) { - // Check if Python is shutting down - if so, skip logging but still clean up ODBC resources - bool pythonShuttingDown = false; - - try { - if (Py_IsInitialized() == 0) { - pythonShuttingDown = true; - } else { - // Try to check sys._is_finalizing(), but don't fail if it doesn't exist - py::gil_scoped_acquire gil; - py::object sys_module = py::module_::import("sys"); - if (!sys_module.is_none()) { - // Check if the attribute exists before accessing it - if (py::hasattr(sys_module, "_is_finalizing")) { - py::object finalizing_func = sys_module.attr("_is_finalizing"); - if (!finalizing_func.is_none() && finalizing_func().cast()) { - pythonShuttingDown = true; - } - } - } - } - } catch (...) { - // Only consider it shutdown if we absolutely can't check Python state - // Be more conservative - don't assume shutdown on any exception - } + // Check if Python is shutting down using centralized helper function + bool pythonShuttingDown = is_python_finalizing(); // CRITICAL FIX: During Python shutdown, don't free STMT handles as their parent DBC may already be freed // This prevents segfault when handles are freed in wrong order during interpreter shutdown From 3d312ece4e06268004717147cc2b47e9465f6096 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 30 Sep 2025 10:55:12 +0530 Subject: [PATCH 8/8] added cerr for error during python finalization state --- mssql_python/pybind/ddbc_bindings.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 1d6d4245..58f23749 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -683,6 +683,7 @@ static bool is_python_finalizing() { } return false; } catch (...) { + std::cerr << "Error occurred while checking Python finalization state." << std::endl; // Be conservative - don't assume shutdown on any exception // Only return true if we're absolutely certain Python is shutting down return false;