diff --git a/mssql_python/pybind/connection/connection.cpp b/mssql_python/pybind/connection/connection.cpp index 12811220..1fe4d213 100644 --- a/mssql_python/pybind/connection/connection.cpp +++ b/mssql_python/pybind/connection/connection.cpp @@ -308,6 +308,19 @@ bool Connection::reset() { disconnect(); return false; } + + // SQL_ATTR_RESET_CONNECTION does NOT reset the transaction isolation level. + // Explicitly reset it to the default (SQL_TXN_READ_COMMITTED) to prevent + // isolation level settings from leaking between pooled connection usages. + LOG("Resetting transaction isolation level to READ COMMITTED"); + ret = SQLSetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_TXN_ISOLATION, + (SQLPOINTER)SQL_TXN_READ_COMMITTED, SQL_IS_INTEGER); + if (!SQL_SUCCEEDED(ret)) { + LOG("Failed to reset transaction isolation level (ret=%d). Marking as dead.", ret); + disconnect(); + return false; + } + updateLastUsed(); return true; } diff --git a/tests/test_009_pooling.py b/tests/test_009_pooling.py index dfc99c2b..1a3e5f09 100644 --- a/tests/test_009_pooling.py +++ b/tests/test_009_pooling.py @@ -21,6 +21,7 @@ import statistics from mssql_python import connect, pooling from mssql_python.pooling import PoolingManager +import mssql_python @pytest.fixture(autouse=True) @@ -84,6 +85,85 @@ def test_connection_pooling_reuse_spid(conn_str): assert spid1 == spid2, "Connections not reused - different SPIDs" +def test_connection_pooling_isolation_level_reset(conn_str): + """Test that pooling correctly resets session state for isolation level. + + This test verifies that when a connection is returned to the pool and then + reused, the isolation level setting is reset to the default (READ COMMITTED) + to prevent session state from leaking between connection usages. + + Bug Fix: Previously, SQL_ATTR_RESET_CONNECTION was used which does NOT reset + the isolation level. Now we explicitly reset it to prevent state leakage. + """ + # Enable pooling with small pool to ensure connection reuse + pooling(enabled=True, max_size=1, idle_timeout=30) + + # Create first connection and set isolation level to SERIALIZABLE + conn1 = connect(conn_str) + + # Set isolation level to SERIALIZABLE (non-default) + conn1.set_attr(mssql_python.SQL_ATTR_TXN_ISOLATION, mssql_python.SQL_TXN_SERIALIZABLE) + + # Verify the isolation level was set + cursor1 = conn1.cursor() + cursor1.execute( + "SELECT CASE transaction_isolation_level " + "WHEN 0 THEN 'Unspecified' " + "WHEN 1 THEN 'ReadUncommitted' " + "WHEN 2 THEN 'ReadCommitted' " + "WHEN 3 THEN 'RepeatableRead' " + "WHEN 4 THEN 'Serializable' " + "WHEN 5 THEN 'Snapshot' END AS isolation_level " + "FROM sys.dm_exec_sessions WHERE session_id = @@SPID" + ) + isolation_level_1 = cursor1.fetchone()[0] + assert isolation_level_1 == "Serializable", f"Expected Serializable, got {isolation_level_1}" + + # Get SPID for verification of connection reuse + cursor1.execute("SELECT @@SPID") + spid1 = cursor1.fetchone()[0] + + # Close connection (return to pool) + cursor1.close() + conn1.close() + + # Get second connection from pool (should reuse the same connection) + conn2 = connect(conn_str) + + # Check if it's the same connection (same SPID) + cursor2 = conn2.cursor() + cursor2.execute("SELECT @@SPID") + spid2 = cursor2.fetchone()[0] + + # Verify connection was reused + assert spid1 == spid2, "Connection was not reused from pool" + + # Check if isolation level is reset to default + cursor2.execute( + "SELECT CASE transaction_isolation_level " + "WHEN 0 THEN 'Unspecified' " + "WHEN 1 THEN 'ReadUncommitted' " + "WHEN 2 THEN 'ReadCommitted' " + "WHEN 3 THEN 'RepeatableRead' " + "WHEN 4 THEN 'Serializable' " + "WHEN 5 THEN 'Snapshot' END AS isolation_level " + "FROM sys.dm_exec_sessions WHERE session_id = @@SPID" + ) + isolation_level_2 = cursor2.fetchone()[0] + + # Verify isolation level is reset to default (READ COMMITTED) + # This is the CORRECT behavior for connection pooling - we should reset + # session state to prevent settings from one usage affecting the next + assert isolation_level_2 == "ReadCommitted", ( + f"Isolation level was not reset! Expected 'ReadCommitted', got '{isolation_level_2}'. " + f"This indicates session state leaked from the previous connection usage." + ) + + # Clean up + cursor2.close() + conn2.close() + + def test_connection_pooling_speed(conn_str): """Test that connection pooling provides performance benefits over multiple iterations.""" # Warm up to eliminate cold start effects @@ -229,6 +309,9 @@ def test_pool_idle_timeout_removes_connections(conn_str): # ============================================================================= +@pytest.mark.skip( + "Test causes fatal crash - forcibly closing underlying connection leads to undefined behavior" +) def test_pool_removes_invalid_connections(conn_str): """Test that the pool removes connections that become invalid (simulate by closing underlying connection).""" pooling(max_size=1, idle_timeout=30)