Skip to content

Commit 0ce4021

Browse files
committed
Fix for ODBC resource leakage
1 parent e970e34 commit 0ce4021

File tree

4 files changed

+66
-10
lines changed

4 files changed

+66
-10
lines changed

mssql_python/__init__.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
This module initializes the mssql_python package.
55
"""
66

7+
import atexit
78
import sys
89
import types
10+
import weakref
911
from typing import Dict
1012

1113
# Import settings from helpers to avoid circular imports
@@ -67,6 +69,40 @@
6769
# Pooling
6870
from .pooling import PoolingManager
6971

72+
# Global registry for tracking active connections (using weak references)
73+
_active_connections = weakref.WeakSet()
74+
75+
76+
def _register_connection(conn):
77+
"""Register a connection for cleanup before shutdown."""
78+
_active_connections.add(conn)
79+
80+
81+
def _cleanup_connections():
82+
"""
83+
Cleanup function called by atexit to close all active connections.
84+
85+
This prevents resource leaks during interpreter shutdown by ensuring
86+
all ODBC handles are freed in the correct order before Python finalizes.
87+
"""
88+
# Make a copy of the connections to avoid modification during iteration
89+
connections_to_close = list(_active_connections)
90+
91+
for conn in connections_to_close:
92+
try:
93+
# Check if connection is still valid and not closed
94+
if hasattr(conn, "_closed") and not conn._closed:
95+
# Close will handle both cursors and the connection
96+
conn.close()
97+
except Exception:
98+
# Silently ignore errors during shutdown cleanup
99+
# We're prioritizing crash prevention over error reporting
100+
pass
101+
102+
103+
# Register cleanup function to run before Python exits
104+
atexit.register(_cleanup_connections)
105+
70106
# GLOBALS
71107
# Read-Only
72108
apilevel: str = "2.0"

mssql_python/connection.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,17 @@ def __init__(
239239
)
240240
self.setautocommit(autocommit)
241241

242+
# Register this connection for cleanup before Python shutdown
243+
# This ensures ODBC handles are freed in correct order, preventing leaks
244+
try:
245+
import mssql_python
246+
247+
if hasattr(mssql_python, "_register_connection"):
248+
mssql_python._register_connection(self)
249+
except (ImportError, AttributeError):
250+
# If registration fails, continue - cleanup will still happen via __del__
251+
pass
252+
242253
def _construct_connection_string(self, connection_str: str = "", **kwargs: Any) -> str:
243254
"""
244255
Construct the connection string by parsing, validating, and merging parameters.

mssql_python/pybind/ddbc_bindings.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,15 @@ void SqlHandle::free() {
11301130
// Type 3 = SQL_HANDLE_STMT (parent: DBC)
11311131
// Type 2 = SQL_HANDLE_DBC (parent: ENV, which is static and may destruct first)
11321132
// Type 1 = SQL_HANDLE_ENV (no parent)
1133+
//
1134+
// RESOURCE LEAK MITIGATION:
1135+
// When handles are skipped during shutdown, they are not freed, which could
1136+
// cause resource leaks. However, this is mitigated by:
1137+
// 1. Python-side atexit cleanup (in __init__.py) that explicitly closes all
1138+
// connections before shutdown, ensuring handles are freed in correct order
1139+
// 2. OS-level cleanup at process termination recovers any remaining resources
1140+
// 3. This tradeoff prioritizes crash prevention over resource cleanup, which
1141+
// is appropriate since we're already in shutdown sequence
11331142
if (pythonShuttingDown && (_type == 3 || _type == 2)) {
11341143
_handle = nullptr; // Mark as freed to prevent double-free attempts
11351144
return;

tests/test_013_SqlHandle_free_shutdown.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ def test_stmt_handle_cleanup_at_shutdown(self, conn_str):
259259
assert result.returncode == 0, f"Process crashed. stderr: {result.stderr}"
260260
assert "STMT handle cleanup test: Exiting without explicit cleanup" in result.stdout
261261
assert "Query result: [(1,)]" in result.stdout
262-
print(f" STMT handle (Type 3) cleanup during shutdown: PASSED")
262+
print(f"PASS: STMT handle (Type 3) cleanup during shutdown")
263263

264264
def test_dbc_handle_cleanup_at_shutdown(self, conn_str):
265265
"""
@@ -308,7 +308,7 @@ def test_dbc_handle_cleanup_at_shutdown(self, conn_str):
308308
assert "Connection 0: created and cursor closed" in result.stdout
309309
assert "Connection 1: created and cursor closed" in result.stdout
310310
assert "Connection 2: created and cursor closed" in result.stdout
311-
print(f" DBC handle (Type 2) cleanup during shutdown: PASSED")
311+
print(f"PASS: DBC handle (Type 2) cleanup during shutdown")
312312

313313
def test_env_handle_cleanup_at_shutdown(self, conn_str):
314314
"""
@@ -356,7 +356,7 @@ def test_env_handle_cleanup_at_shutdown(self, conn_str):
356356
assert "Connection 0: properly closed" in result.stdout
357357
assert "Connection 1: properly closed" in result.stdout
358358
assert "Connection 2: properly closed" in result.stdout
359-
print(f" ENV handle (Type 1) cleanup during shutdown: PASSED")
359+
print(f"PASS: ENV handle (Type 1) cleanup during shutdown")
360360

361361
def test_mixed_handle_cleanup_at_shutdown(self, conn_str):
362362
"""
@@ -430,7 +430,7 @@ def test_mixed_handle_cleanup_at_shutdown(self, conn_str):
430430
assert "Connection 1: cursors left open" in result.stdout
431431
assert "Connection 2: cursors closed, connection left open" in result.stdout
432432
assert "Connection 3: everything properly closed" in result.stdout
433-
print(f" Mixed handle cleanup during shutdown: PASSED")
433+
print(f"PASS: Mixed handle cleanup during shutdown")
434434

435435
def test_rapid_connection_churn_with_shutdown(self, conn_str):
436436
"""
@@ -483,7 +483,7 @@ def test_rapid_connection_churn_with_shutdown(self, conn_str):
483483
assert result.returncode == 0, f"Process crashed. stderr: {result.stderr}"
484484
assert "Created 10 connections, closed 5 explicitly" in result.stdout
485485
assert "Rapid churn test: Exiting with mixed cleanup" in result.stdout
486-
print(f" Rapid connection churn with shutdown: PASSED")
486+
print(f"PASS: Rapid connection churn with shutdown")
487487

488488
def test_exception_during_query_with_shutdown(self, conn_str):
489489
"""
@@ -524,7 +524,7 @@ def test_exception_during_query_with_shutdown(self, conn_str):
524524
assert result.returncode == 0, f"Process crashed. stderr: {result.stderr}"
525525
assert "Expected error occurred: ProgrammingError" in result.stdout
526526
assert "Exception test: Exiting after exception without cleanup" in result.stdout
527-
print(f" Exception during query with shutdown: PASSED")
527+
print(f"PASS: Exception during query with shutdown")
528528

529529
def test_weakref_cleanup_at_shutdown(self, conn_str):
530530
"""
@@ -578,7 +578,7 @@ def callback(ref):
578578

579579
assert result.returncode == 0, f"Process crashed. stderr: {result.stderr}"
580580
assert "Weakref test: Exiting with weakrefs active" in result.stdout
581-
print(f" Weakref cleanup at shutdown: PASSED")
581+
print(f"PASS: Weakref cleanup at shutdown")
582582

583583
def test_gc_during_shutdown_with_circular_refs(self, conn_str):
584584
"""
@@ -638,7 +638,7 @@ def execute_query(self):
638638

639639
assert result.returncode == 0, f"Process crashed. stderr: {result.stderr}"
640640
assert "Circular ref test: Exiting after GC with cycles" in result.stdout
641-
print(f" GC during shutdown with circular refs: PASSED")
641+
print(f"PASS: GC during shutdown with circular refs")
642642

643643
def test_all_handle_types_comprehensive(self, conn_str):
644644
"""
@@ -717,7 +717,7 @@ def test_all_handle_types_comprehensive(self, conn_str):
717717
assert "Scenario 3: Both cursor and connection left open" in result.stdout
718718
assert "Scenario 4: Multiple cursors per connection left open" in result.stdout
719719
assert "=== Exiting ===" in result.stdout
720-
print(f" Comprehensive all handle types test: PASSED")
720+
print(f"PASS: Comprehensive all handle types test")
721721

722722

723723
if __name__ == "__main__":
@@ -755,7 +755,7 @@ def test_all_handle_types_comprehensive(self, conn_str):
755755
test.test_all_handle_types_comprehensive(conn_str)
756756

757757
print("\n" + "=" * 70)
758-
print(" ALL TESTS PASSED - No segfaults detected")
758+
print("PASS: ALL TESTS PASSED - No segfaults detected")
759759
print("=" * 70 + "\n")
760760
except AssertionError as e:
761761
print(f"\n✗ TEST FAILED: {e}")

0 commit comments

Comments
 (0)