Skip to content
Open
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
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,7 @@ build/
*.swp

# .DS_Store files
.DS_Store
.DS_Store

# WHL files
*.whl
Binary file not shown.
24 changes: 17 additions & 7 deletions mssql_python/cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from mssql_python.constants import ConstantsDDBC as ddbc_sql_const
from mssql_python.helpers import check_error, log
from mssql_python import ddbc_bindings
from mssql_python.exceptions import InterfaceError
from mssql_python.exceptions import ProgrammingError
from .row import Row


Expand Down Expand Up @@ -435,13 +435,17 @@ def _reset_cursor(self) -> None:

def close(self) -> None:
"""
Close the cursor now (rather than whenever __del__ is called).
Close the connection now (rather than whenever .__del__() is called).
Idempotent: subsequent calls have no effect.

The idempotency here means that resources are freed only once - subsequent
calls won't double-free resources or cause corruption, but will raise a
predictable exception instead.

Raises:
Error: If any operation is attempted with the cursor after it is closed.
"""
if self.closed:
raise Exception("Cursor is already closed.")
self._check_closed()

if self.hstmt:
self.hstmt.free()
Expand All @@ -457,7 +461,7 @@ def _check_closed(self):
Error: If the cursor is closed.
"""
if self.closed:
raise Exception("Operation cannot be performed: the cursor is closed.")
raise ProgrammingError("Cursor is already closed.", "")
Copy link
Contributor

@sumitmsft sumitmsft Aug 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


def _create_parameter_types_list(self, parameter, param_info, parameters_list, i):
"""
Expand Down Expand Up @@ -794,10 +798,16 @@ def __del__(self):
Destructor to ensure the cursor is closed when it is no longer needed.
This is a safety net to ensure resources are cleaned up
even if close() was not called explicitly.
If the cursor is already closed, it will not raise an exception during cleanup.
"""
if "_closed" not in self.__dict__ or not self._closed:
if "closed" not in self.__dict__ or not self.closed:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

try:
self.close()
except Exception as e:
# Don't raise an exception in __del__, just log it
log('error', "Error during cursor cleanup in __del__: %s", e)
# If interpreter is shutting down, we might not have logging set up
import sys
if sys and sys._is_finalizing():
# Suppress logging during interpreter shutdown
return
log('debug', "Exception during cursor cleanup in __del__: %s", e)
Binary file removed mssql_python/msvcp140.dll
Binary file not shown.
224 changes: 223 additions & 1 deletion tests/test_005_connection_cursor_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,15 @@
- test_connection_close_idempotent: Tests that calling close() multiple times is safe.
- test_cursor_after_connection_close: Tests that creating a cursor after closing the connection raises an error.
- test_multiple_cursor_operations_cleanup: Tests cleanup with multiple cursor operations.
- test_cursor_close_raises_on_double_close: Tests that closing a cursor twice raises a ProgrammingError.
- test_cursor_del_no_logging_during_shutdown: Tests that cursor __del__ doesn't log errors during interpreter shutdown.
- test_cursor_del_on_closed_cursor_no_errors: Tests that __del__ on already closed cursor doesn't produce error logs.
- test_cursor_del_unclosed_cursor_cleanup: Tests that __del__ properly cleans up unclosed cursors without errors.
- test_cursor_operations_after_close_raise_errors: Tests that all cursor operations raise appropriate errors after close.
- test_mixed_cursor_cleanup_scenarios: Tests various mixed cleanup scenarios in one script.
"""

import os
import pytest
import subprocess
import sys
Expand Down Expand Up @@ -263,4 +270,219 @@ def test_multiple_cursor_operations_cleanup(conn_str):
# All cursors should be closed
assert cursor_insert.closed is True
assert cursor_select1.closed is True
assert cursor_select2.closed is True
assert cursor_select2.closed is True

def test_cursor_close_raises_on_double_close(conn_str):
"""Test that closing a cursor twice raises ProgrammingError"""
conn = connect(conn_str)
cursor = conn.cursor()
cursor.execute("SELECT 1")
cursor.fetchall()

# First close should succeed
cursor.close()
assert cursor.closed is True

# Second close should raise ProgrammingError
from mssql_python.exceptions import ProgrammingError
with pytest.raises(ProgrammingError) as excinfo:
cursor.close()
assert "Cursor is already closed" in str(excinfo.value)

# Third close should also raise
with pytest.raises(ProgrammingError):
cursor.close()

conn.close()


def test_cursor_del_no_logging_during_shutdown(conn_str, tmp_path):
"""Test that cursor __del__ doesn't log errors during interpreter shutdown"""
code = f"""
from mssql_python import connect

# Create connection and cursor
conn = connect(\"\"\"{conn_str}\"\"\")
cursor = conn.cursor()
cursor.execute("SELECT 1")
cursor.fetchall()

# Don't close cursor - let __del__ handle it during shutdown
# This should not produce any log output during interpreter shutdown
print("Test completed successfully")
"""

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

# Should exit cleanly
assert result.returncode == 0, f"Script failed: {result.stderr}"
# Should not have any debug/error logs about cursor cleanup
assert "Exception during cursor cleanup" not in result.stderr
assert "Exception during cursor cleanup" not in result.stdout
# Should have our success message
assert "Test completed successfully" in result.stdout


def test_cursor_del_on_closed_cursor_no_errors(conn_str, caplog):
"""Test that __del__ on already closed cursor doesn't produce error logs"""
import logging
caplog.set_level(logging.DEBUG)

conn = connect(conn_str)
cursor = conn.cursor()
cursor.execute("SELECT 1")
cursor.fetchall()

# Close cursor explicitly
cursor.close()

# Clear any existing logs
caplog.clear()

# Delete the cursor - should not produce any logs
del cursor
import gc
gc.collect()

# Check that no error logs were produced
for record in caplog.records:
assert "Exception during cursor cleanup" not in record.message
assert "Cursor is already closed" not in record.message

conn.close()


def test_cursor_del_unclosed_cursor_cleanup(conn_str):
"""Test that __del__ properly cleans up unclosed cursors without errors"""
code = f"""
from mssql_python import connect

# Create connection and cursor
conn = connect(\"\"\"{conn_str}\"\"\")
cursor = conn.cursor()
cursor.execute("SELECT 1")
cursor.fetchall()

# Store cursor state before deletion
cursor_closed_before = cursor.closed

# Delete cursor without closing - __del__ should handle cleanup
del cursor
import gc
gc.collect()

# Verify cursor was not closed before deletion
assert cursor_closed_before is False, "Cursor should not be closed before deletion"

# Close connection
conn.close()
print("Cleanup successful")
"""

result = subprocess.run(
[sys.executable, "-c", code],
capture_output=True,
text=True
)
assert result.returncode == 0, f"Expected successful cleanup, but got: {result.stderr}"
assert "Cleanup successful" in result.stdout
# Should not have any error messages
assert "Exception" not in result.stderr


def test_cursor_operations_after_close_raise_errors(conn_str):
"""Test that all cursor operations raise appropriate errors after close"""
conn = connect(conn_str)
cursor = conn.cursor()
cursor.execute("SELECT 1")
cursor.fetchall()

# Close the cursor
cursor.close()

# All operations should raise exceptions
with pytest.raises(Exception) as excinfo:
cursor.execute("SELECT 2")
assert "Cursor is already closed." in str(excinfo.value)

with pytest.raises(Exception) as excinfo:
cursor.fetchone()
assert "Cursor is already closed." in str(excinfo.value)

with pytest.raises(Exception) as excinfo:
cursor.fetchmany(5)
assert "Cursor is already closed." in str(excinfo.value)

with pytest.raises(Exception) as excinfo:
cursor.fetchall()
assert "Cursor is already closed." in str(excinfo.value)

conn.close()


def test_mixed_cursor_cleanup_scenarios(conn_str, tmp_path):
"""Test various mixed cleanup scenarios in one script"""
code = f"""
from mssql_python import connect
from mssql_python.exceptions import ProgrammingError

# Test 1: Normal cursor close
conn1 = connect(\"\"\"{conn_str}\"\"\")
cursor1 = conn1.cursor()
cursor1.execute("SELECT 1")
cursor1.fetchall()
cursor1.close()

# Test 2: Double close raises error
try:
cursor1.close()
print("ERROR: Double close should have raised ProgrammingError")
except ProgrammingError as e:
print("PASS: Double close raised ProgrammingError")

# Test 3: Cursor cleanup via __del__
cursor2 = conn1.cursor()
cursor2.execute("SELECT 2")
cursor2.fetchall()
# Don't close cursor2, let __del__ handle it

# Test 4: Connection close cleans up cursors
conn2 = connect(\"\"\"{conn_str}\"\"\")
cursor3 = conn2.cursor()
cursor4 = conn2.cursor()
cursor3.execute("SELECT 3")
cursor3.fetchall()
cursor4.execute("SELECT 4")
cursor4.fetchall()
conn2.close() # Should close both cursors

# Verify cursors are closed
assert cursor3.closed is True
assert cursor4.closed is True
print("PASS: Connection close cleaned up cursors")

# Clean up
conn1.close()
print("All tests passed")
"""

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

if result.returncode != 0:
print(f"STDOUT: {result.stdout}")
print(f"STDERR: {result.stderr}")

assert result.returncode == 0, f"Script failed: {result.stderr}"
assert "PASS: Double close raised ProgrammingError" in result.stdout
assert "PASS: Connection close cleaned up cursors" in result.stdout
assert "All tests passed" in result.stdout
# Should not have error logs
assert "Exception during cursor cleanup" not in result.stderr