FIX: Resource cleanup on Python shutdown to avoid segfaults

[bewithgaurav]

Work Item / Issue Reference

AB#38824

GitHub Issue: #254

---

Summary

This pull request addresses a critical segfault issue that occurred during Python interpreter shutdown when using the mssql_python package, especially in scenarios with unclosed cursors, syntax errors, or concurrent operations. The changes introduce robust detection of Python's shutdown state to prevent unsafe resource cleanup and improve test coverage for these edge cases.

Key changes include:

Python Shutdown Safety & Resource Cleanup

• Added a centralized helper function is_python_finalizing() in ddbc_bindings.cpp to reliably detect when Python is shutting down or finalizing, using both Py_IsInitialized() and the sys._is_finalizing attribute for compatibility across Python versions.
• Updated the LOG function to skip logging if Python is shutting down, and to handle exceptions gracefully to avoid crashes during interpreter cleanup. [1] [2]
• Modified the SqlHandle::free() method to prevent freeing SQL_HANDLE_STMT handles during shutdown, as their parent DBC handles may already be freed, avoiding double-free and segfaults. Handles are marked as freed instead.

Test Coverage for Shutdown and Concurrency Edge Cases

• Added multiple new tests in test_005_connection_cursor_lifecycle.py to ensure no segfaults occur during Python shutdown, including:
  • Tests for syntax errors without explicit resource cleanup.
  • Tests for multiple concurrent cursor operations and abrupt interpreter exits.
  • Tests for closing connections with pending results and aggressive threading scenarios.

These changes significantly improve the stability of the mssql_python package in complex and error-prone scenarios, especially during interpreter shutdown and multithreaded usage.

[Copilot]

Pull Request Overview

This PR addresses critical stability issues by implementing proper Python shutdown detection and resource cleanup in the native extension code to prevent segmentation faults during interpreter shutdown after SQL errors.

• Added Python shutdown/finalization state checks in the LOG function and SqlHandle::free method to avoid unsafe API calls during cleanup
• Enhanced exception handling to silently ignore errors when Python is in an inconsistent state
• Added comprehensive tests to verify that SQL syntax errors don't cause segfaults during Python shutdown

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
mssql_python/pybind/ddbc_bindings.cpp	Enhanced LOG function and SqlHandle::free method with Python shutdown detection and safe cleanup logic
tests/test_005_connection_cursor_lifecycle.py	Added new test cases to verify SQL syntax errors don't cause segfaults during Python shutdown scenarios

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 53%

🎯 Overall Coverage 74%

📈 Total Lines Covered: 4179 out of 5603 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/pybind/ddbc_bindings.cpp (53.1%): Missing lines 681-685,689,692-693,710-719,721,723-724,727-728

Summary

• Total: 49 lines
• Missing: 23 lines
• Coverage: 53%

mssql_python/pybind/ddbc_bindings.cpp

Lines 677-697

  677         py::object sys_module = py::module_::import("sys");
  678         if (!sys_module.is_none()) {
  679             // Check if the attribute exists before accessing it (for Python version compatibility)
  680             if (py::hasattr(sys_module, "_is_finalizing")) {
! 681                 py::object finalizing_func = sys_module.attr("_is_finalizing");
! 682                 if (!finalizing_func.is_none() && finalizing_func().cast<bool>()) {
! 683                     return true; // Python is finalizing
! 684                 }
! 685             }
  686         }
  687         return false;
  688     } catch (...) {
! 689         std::cerr << "Error occurred while checking Python finalization state." << std::endl;
  690         // Be conservative - don't assume shutdown on any exception
  691         // Only return true if we're absolutely certain Python is shutting down
! 692         return false;
! 693     }
  694 }
  695 
  696 // TODO: Revisit GIL considerations if we're using python's logger
  697 template <typename... Args>

---

Lines 706-732

  706 
  707         py::object logger = py::module_::import("mssql_python.logging_config").attr("get_logger")();
  708         if (py::isinstance<py::none>(logger)) return;
  709 
! 710         try {
! 711             std::string ddbcFormatString = "[DDBC Bindings log] " + formatString;
! 712             if constexpr (sizeof...(args) == 0) {
! 713                 logger.attr("debug")(py::str(ddbcFormatString));
! 714             } else {
! 715                 py::str message = py::str(ddbcFormatString).format(std::forward<Args>(args)...);
! 716                 logger.attr("debug")(message);
! 717             }
! 718         } catch (const std::exception& e) {
! 719             std::cerr << "Logging error: " << e.what() << std::endl;
  720         }
! 721     } catch (const py::error_already_set& e) {
  722         // Python is shutting down or in an inconsistent state, silently ignore
! 723         (void)e; // Suppress unused variable warning
! 724         return;
  725     } catch (const std::exception& e) {
  726         // Any other error, ignore to prevent crash during cleanup
! 727         (void)e; // Suppress unused variable warning
! 728         return;
  729     }
  730 }
  731 
  732 // TODO: Add more nuanced exception classes

---

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.connection.connection.cpp: 67.6%
mssql_python.ddbc_bindings.py: 68.5%
mssql_python.pybind.ddbc_bindings.cpp: 69.3%
mssql_python.cursor.py: 79.6%
mssql_python.connection.py: 81.7%
mssql_python.helpers.py: 84.7%
mssql_python.auth.py: 85.3%
mssql_python.type.py: 86.8%
mssql_python.pooling.py: 88.8%
mssql_python.exceptions.py: 90.4%

---

🔗 Quick Links

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

[sumitmsft]

Left a comment

[sumitmsft]

LGTM. Approved