FIX: prevent AttributeError in __del__ on partially-initialized Cursor

[subrata-ms]

Work Item / Issue Reference

AB#45943

GitHub Issue: #642

---

Summary

This pull request improves the robustness of the Cursor class initialization and cleanup logic, ensuring that partially-initialized or half-constructed cursor objects are handled safely and do not cause unraisable exceptions during garbage collection. It also adds regression tests to prevent recurrence of related bugs.

Initialization and cleanup robustness:

• The __init__ method in cursor.py now sets self.closed = False and self.hstmt = None as the very first statements, before any code that might raise an exception, ensuring that even partially-initialized Cursor instances have a consistent state for cleanup. [1] [2]
• The close() method now safely checks for the existence of the closed attribute using getattr(self, "closed", True), preventing AttributeError if the attribute is missing (e.g., in half-initialized objects).
• The __del__ method now uses the correct sys.is_finalizing() function (instead of the incorrect sys._is_finalizing()) and guards logging calls to prevent unraisable exceptions during interpreter shutdown.

Testing and regression prevention:

• Adds test_cursor_del_half_initialized_cursor_no_errors and test_cursor_init_failure_leaves_consistent_state to ensure that half-initialized cursors do not raise unraisable exceptions during garbage collection and that failed initialization leaves the cursor and connection in a consistent, recoverable state.

[Copilot]

Pull request overview

This PR hardens mssql_python.cursor.Cursor lifecycle behavior to avoid unraisable exceptions during garbage collection when a Cursor is only partially initialized, and adds regression tests to prevent recurrence of the CI PytestUnraisableExceptionWarning seen in issue #642.

Changes:

• Establishes Cursor cleanup invariants earlier in __init__ (closed=False, hstmt=None) so close()/__del__ can run safely even after initialization failures.
• Makes Cursor.close() tolerant of instances missing the closed attribute (e.g., objects created via Cursor.__new__).
• Fixes __del__ finalization check to use sys.is_finalizing() and guards debug logging during interpreter shutdown; adds lifecycle regression tests.

Reviewed changes

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

File	Description
mssql_python/cursor.py	Makes cursor initialization/cleanup resilient to partial construction and interpreter shutdown edge cases.
tests/test_005_connection_cursor_lifecycle.py	Adds regression tests for half-initialized cursor GC paths and failed cursor construction scenarios.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bewithgaurav]

+1

[saurabh500]

@subrata-ms generally looks good to me. The copilot comment on the test is interesting.

[bewithgaurav]

fix itself lgtm but the tests aren't defending the same as of now
added a comment that should close that gap, copilot's comments are also working taking a look at

[bewithgaurav]

this test passes even on main with the unfixed cursor.py (switched to main back in & ran this test - all green), actually it doesn't reinforce corrections done in the PR.

the unraisable from __del__ goes through sys.unraisablehook, not warnings, so catch_warnings(record=True) never sees it and offenders is always empty. wrapping conn.cursor() instead (as suggested above) doesn't help, same reason.

also the dealloc happens during conn.cursor(), not the gc.collect() wrapped (_cursors is a WeakSet).

the version that breaks (fails on main, passes here):

import sys
captured = []
old = sys.unraisablehook
sys.unraisablehook = lambda u: captured.append(u)
try:
    with pytest.raises(RuntimeError, match="simulated HSTMT"):
        conn.cursor()
    gc.collect()
finally:
    sys.unraisablehook = old
assert not [u for u in captured if isinstance(u.exc_value, AttributeError)]

[bewithgaurav]

+1

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 60%

🎯 Overall Coverage 80%

📈 Total Lines Covered: 6743 out of 8335 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/cursor.py (60.0%): Missing lines 3247,3253

Summary

• Total: 5 lines
• Missing: 2 lines
• Coverage: 60%

mssql_python/cursor.py

Lines 3243-3251

  3243                 # Don't raise an exception in __del__, just log it
  3244                 # If interpreter is shutting down, we might not have logging set up
  3245                 import sys
  3246 
! 3247                 if sys and sys.is_finalizing():
  3248                     # Suppress logging during interpreter shutdown
  3249                     return
  3250                 # ``logger`` could be torn down or have its handlers closed
  3251                 # late in interpreter shutdown; guard so the debug call

---

Lines 3249-3257

  3249                     return
  3250                 # ``logger`` could be torn down or have its handlers closed
  3251                 # late in interpreter shutdown; guard so the debug call
  3252                 # cannot itself raise an unraisable exception.
! 3253                 if logger is not None:
  3254                     logger.debug("Exception during cursor cleanup in __del__: %s", e)
  3255 
  3256     def scroll(
  3257         self, value: int, mode: str = "relative"

---

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.cpp: 76.3%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%

---

🔗 Quick Links

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