Skip to content

Commit a906837

Browse files
authored
Merge branch 'main' into jahnvi/250_encoding_decoding
2 parents ca40f5e + 5c77e3f commit a906837

File tree

2 files changed

+96
-0
lines changed

2 files changed

+96
-0
lines changed

mssql_python/pybind/connection/connection.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,19 @@ bool Connection::reset() {
308308
disconnect();
309309
return false;
310310
}
311+
312+
// SQL_ATTR_RESET_CONNECTION does NOT reset the transaction isolation level.
313+
// Explicitly reset it to the default (SQL_TXN_READ_COMMITTED) to prevent
314+
// isolation level settings from leaking between pooled connection usages.
315+
LOG("Resetting transaction isolation level to READ COMMITTED");
316+
ret = SQLSetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_TXN_ISOLATION,
317+
(SQLPOINTER)SQL_TXN_READ_COMMITTED, SQL_IS_INTEGER);
318+
if (!SQL_SUCCEEDED(ret)) {
319+
LOG("Failed to reset transaction isolation level (ret=%d). Marking as dead.", ret);
320+
disconnect();
321+
return false;
322+
}
323+
311324
updateLastUsed();
312325
return true;
313326
}

tests/test_009_pooling.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import statistics
2222
from mssql_python import connect, pooling
2323
from mssql_python.pooling import PoolingManager
24+
import mssql_python
2425

2526

2627
@pytest.fixture(autouse=True)
@@ -84,6 +85,85 @@ def test_connection_pooling_reuse_spid(conn_str):
8485
assert spid1 == spid2, "Connections not reused - different SPIDs"
8586

8687

88+
def test_connection_pooling_isolation_level_reset(conn_str):
89+
"""Test that pooling correctly resets session state for isolation level.
90+
91+
This test verifies that when a connection is returned to the pool and then
92+
reused, the isolation level setting is reset to the default (READ COMMITTED)
93+
to prevent session state from leaking between connection usages.
94+
95+
Bug Fix: Previously, SQL_ATTR_RESET_CONNECTION was used which does NOT reset
96+
the isolation level. Now we explicitly reset it to prevent state leakage.
97+
"""
98+
# Enable pooling with small pool to ensure connection reuse
99+
pooling(enabled=True, max_size=1, idle_timeout=30)
100+
101+
# Create first connection and set isolation level to SERIALIZABLE
102+
conn1 = connect(conn_str)
103+
104+
# Set isolation level to SERIALIZABLE (non-default)
105+
conn1.set_attr(mssql_python.SQL_ATTR_TXN_ISOLATION, mssql_python.SQL_TXN_SERIALIZABLE)
106+
107+
# Verify the isolation level was set
108+
cursor1 = conn1.cursor()
109+
cursor1.execute(
110+
"SELECT CASE transaction_isolation_level "
111+
"WHEN 0 THEN 'Unspecified' "
112+
"WHEN 1 THEN 'ReadUncommitted' "
113+
"WHEN 2 THEN 'ReadCommitted' "
114+
"WHEN 3 THEN 'RepeatableRead' "
115+
"WHEN 4 THEN 'Serializable' "
116+
"WHEN 5 THEN 'Snapshot' END AS isolation_level "
117+
"FROM sys.dm_exec_sessions WHERE session_id = @@SPID"
118+
)
119+
isolation_level_1 = cursor1.fetchone()[0]
120+
assert isolation_level_1 == "Serializable", f"Expected Serializable, got {isolation_level_1}"
121+
122+
# Get SPID for verification of connection reuse
123+
cursor1.execute("SELECT @@SPID")
124+
spid1 = cursor1.fetchone()[0]
125+
126+
# Close connection (return to pool)
127+
cursor1.close()
128+
conn1.close()
129+
130+
# Get second connection from pool (should reuse the same connection)
131+
conn2 = connect(conn_str)
132+
133+
# Check if it's the same connection (same SPID)
134+
cursor2 = conn2.cursor()
135+
cursor2.execute("SELECT @@SPID")
136+
spid2 = cursor2.fetchone()[0]
137+
138+
# Verify connection was reused
139+
assert spid1 == spid2, "Connection was not reused from pool"
140+
141+
# Check if isolation level is reset to default
142+
cursor2.execute(
143+
"SELECT CASE transaction_isolation_level "
144+
"WHEN 0 THEN 'Unspecified' "
145+
"WHEN 1 THEN 'ReadUncommitted' "
146+
"WHEN 2 THEN 'ReadCommitted' "
147+
"WHEN 3 THEN 'RepeatableRead' "
148+
"WHEN 4 THEN 'Serializable' "
149+
"WHEN 5 THEN 'Snapshot' END AS isolation_level "
150+
"FROM sys.dm_exec_sessions WHERE session_id = @@SPID"
151+
)
152+
isolation_level_2 = cursor2.fetchone()[0]
153+
154+
# Verify isolation level is reset to default (READ COMMITTED)
155+
# This is the CORRECT behavior for connection pooling - we should reset
156+
# session state to prevent settings from one usage affecting the next
157+
assert isolation_level_2 == "ReadCommitted", (
158+
f"Isolation level was not reset! Expected 'ReadCommitted', got '{isolation_level_2}'. "
159+
f"This indicates session state leaked from the previous connection usage."
160+
)
161+
162+
# Clean up
163+
cursor2.close()
164+
conn2.close()
165+
166+
87167
def test_connection_pooling_speed(conn_str):
88168
"""Test that connection pooling provides performance benefits over multiple iterations."""
89169
# Warm up to eliminate cold start effects
@@ -229,6 +309,9 @@ def test_pool_idle_timeout_removes_connections(conn_str):
229309
# =============================================================================
230310

231311

312+
@pytest.mark.skip(
313+
"Test causes fatal crash - forcibly closing underlying connection leads to undefined behavior"
314+
)
232315
def test_pool_removes_invalid_connections(conn_str):
233316
"""Test that the pool removes connections that become invalid (simulate by closing underlying connection)."""
234317
pooling(max_size=1, idle_timeout=30)

0 commit comments

Comments
 (0)