FIX: Closed Attribute in cursor __del__

[bewithgaurav]

Work Item / Issue Reference

AB#<WORK_ITEM_ID>

GitHub Issue: #182

---

Summary

This pull request makes improvements to the cursor resource management and error handling in the mssql_python/cursor.py module. The main changes include making the cursor close operation idempotent, refining exception types, and improving logging during cleanup. These updates help ensure more robust and predictable behavior when working with cursors.

Error handling improvements:

• Changed the exception raised when attempting to close an already closed cursor from a generic Exception to a more specific ProgrammingError, providing clearer feedback to users.
• Updated the import statement to use ProgrammingError instead of InterfaceError, aligning with the new error handling approach.

Resource cleanup and logging:

• Improved the destructor (__del__) to check for the correct closed attribute and made sure no exception is raised if the cursor is already closed, enhancing reliability during garbage collection.
• Changed the logging level in the destructor from 'error' to 'debug' for exceptions during cleanup, reducing unnecessary noise in error logs.

[Copilot]

Pull Request Overview

This PR improves cursor resource management and error handling in the mssql_python cursor module. The changes make cursor operations more robust by implementing idempotent close behavior and better cleanup procedures.

• Enhanced cursor close operation to be idempotent and use more specific exception types
• Fixed destructor to properly check the closed attribute instead of _closed
• Improved logging during cleanup by reducing error noise to debug level

---

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

[Copilot]

The close() method documentation states it should be idempotent ('subsequent calls have no effect'), but the implementation still raises an exception when called on an already closed cursor. Consider removing this exception to make the method truly idempotent as documented.

[bewithgaurav]

The "idempotency" in this context means that calling close() multiple times won't free resources twice or cause corruption - the first call frees resources and subsequent calls raise a predictable exception. This is actually the standard behavior in pyodbc as well. Added a comment here specifying the same.

[bewithgaurav]

Deleted unwanted whl file which was committed earlier

[bewithgaurav]

cleaned this up as well, this is already present inside vcredist

[sumitmsft]

Can't we define self.closed in the constructor, so you can simply check if not self.closed:`?
This may avoid reliance on dict, and guards against subtle bugs if the attribute is missing.

Let me know if I am missing anything wrt to dict if its used anywhere else.

[sumitmsft]

These changes still keeps cursor.close() non‑idempotent while the comments claims it is idempotent.

close() now calls self._check_closed() at the start.
_check_closed() raises ProgrammingError when self.closed is True.
Therefore a second close() raises instead of being a no‑op. That is NOT idempotent.

First, the issue raised in 183 requires no-op\no error during second close of the cursor. Secondly, DB-API 2.0 convention defines calling .close() multiple times must be safe (no exception).

We can keep _check_closed() raising ProgrammingError only for operational methods (execute/fetch) after closure - NOT for close() itself.

Fix the tests to make sure double closing of cursor is silent.

[sumitmsft]

Left a couple of comments.