diff --git a/docs/odbc-connection-pooling-plan.md b/docs/odbc-connection-pooling-plan.md index cea68f37..dc226564 100644 --- a/docs/odbc-connection-pooling-plan.md +++ b/docs/odbc-connection-pooling-plan.md @@ -127,6 +127,14 @@ Reviewed the real driver: `Sql/Ntdbms/sqlncli/odbc/{sqlcmisc.cpp, sqlcfunc.cpp, - concurrent checkout; - token-identity separation + near-expiry reconnect (new login, not re-auth). - Diagnose any behavior differences vs. `msodbcsql18`. +- **No in-repo `mssql-python` harness exists** (adding one would pull a Python + + `mssql-python` toolchain into `mssql-rs`, which does not fit the layout and is + the "no second pool" non-goal). The supported hook is the existing driver + registration the e2e runners perform, so `mssql-python` selects this driver by + name. The exact manual/CI steps, env, and expected results for each scenario + above are documented in + [`odbc-connection-pooling-python-e2e.md`](./odbc-connection-pooling-python-e2e.md), + backed by the in-repo unit / TDS / live-C++ coverage listed there. --- diff --git a/docs/odbc-connection-pooling-python-e2e.md b/docs/odbc-connection-pooling-python-e2e.md new file mode 100644 index 00000000..b5eb610e --- /dev/null +++ b/docs/odbc-connection-pooling-python-e2e.md @@ -0,0 +1,215 @@ + + +# mssql-python ↔ mssql-odbc Connection-Pool E2E Verification Plan + +Companion to [`odbc-connection-pooling-plan.md`](./odbc-connection-pooling-plan.md) +(ADO #47317). That plan delivers the ODBC/TDS reset, liveness, transaction, and +isolation semantics that let **mssql-python's** existing client-side pool safely +reuse a physical Rust ODBC connection. This document is the **Workstream C** +verification plan: the exact steps, environment, and expected results for +exercising that pool against this driver end to end. + +## Why this is a documented plan, not an in-repo test + +There is **no in-repo hook that loads mssql-python against the Rust driver**, and +adding one would mean pulling heavy new infrastructure (a Python toolchain plus +the `mssql-python` package and its own C++ pool) into `mssql-rs`, which does not +fit the current repo layout. `mssql-python` is a separate product with its own +pool implementation (`pybind/connection/connection_pool.cpp`); the non-goal is +explicitly "no second pool in Rust." The supported, already-existing hook is the +ODBC **driver registration** the e2e runners perform, so mssql-python can select +this driver by name in a connection string. This plan documents how to drive +that combination manually or in CI, per the plan's guidance to avoid fabricating +new infrastructure. + +The in-repo coverage that backs these scenarios already exists and is exercised +by `cargo btest` / the ODBC e2e suite: + +- **Unit (mock, no server):** `mssql-odbc/src/api/txn.rs`, + `mssql-odbc/src/api/get_connect_attr.rs`, + `mssql-odbc/src/api/set_connect_attr.rs` — `CONNECTION_DEAD` polarity, + `RESET_CONNECTION` value validation / busy rejection / poison-on-failure, + isolation re-apply after reset, checkout lifecycle, access-token-after-connect + rejection. +- **TDS (mock):** `mssql-tds/src/connection/tds_client.rs` — reset ack clears + session-bound caches and restores login defaults; fatal-error death marking. +- **Live C++ e2e:** `mssql-odbc/tests/e2e/tests/connection_pool_test.cpp` — + same-physical-connection reuse, clean state for the next borrower, isolation + reset each checkout, prepared-statement survival, `CONNECTION_DEAD` on a + healthy connection. + +This plan covers the remaining, cross-product parity: mssql-python's **own** pool +driving those same primitives. + +## Prerequisites + +| Requirement | Notes | +|---|---| +| A live SQL Server | e.g. `./dev/dev-launchsql.sh`, or any reachable instance. | +| The Rust ODBC driver, registered | Build with `cargo build` in `mssql-odbc/`, then register it (below). | +| `mssql-python` from GitHub `main` | Authoritative `reset()` / isolation behavior is on GitHub `microsoft/mssql-python` `main` (PR #343). A local ADO-mirror clone may predate #343 — do not rely on it. | +| Python 3.10+ | To run the verification snippets. | + +### Registering the Rust driver for mssql-python + +Reuse the exact registration the e2e runners already perform — do **not** invent +a new mechanism. See `mssql-odbc/tests/e2e/README.md` §"Driver Registration". + +- **Windows** (`mssql-odbc/tests/e2e/run_e2e.ps1`, requires Administrator) + registers the driver under `HKLM\Software\ODBC\ODBCINST.INI\ODBC Driver 18 for + SQL Server (Rust)`, alongside any installed `msodbcsql18`. +- **Linux / macOS** (`mssql-odbc/tests/e2e/run_e2e.sh`) writes an `odbcinst.ini` + registering `[ODBC Driver 18 for SQL Server (Rust)]` and points `ODBCSYSINI` + at it. + +mssql-python selects the driver by name in its connection string, so target the +Rust registration explicitly: + +``` +Driver={ODBC Driver 18 for SQL Server (Rust)};Server=localhost;Database=tempdb;UID=sa;PWD=...;TrustServerCertificate=Yes; +``` + +Run each scenario twice — once against this string, once against +`Driver={ODBC Driver 18 for SQL Server}` (msodbcsql18) — and compare. Parity with +msodbcsql18 is the pass criterion, mirroring the `-CompareWithMsodbcsql` gate the +C++ e2e already uses. + +## Scenarios + +Each maps to the plan's Definition of Done. "Expected" is the behavior that must +match msodbcsql18. + +### (a) Same-physical-connection reuse after reset + +- **Steps:** From a pool of size 1, acquire a connection, record `@@SPID`, create + a `#temp` table, `USE` another database, raise isolation to `SERIALIZABLE`, open + a transaction, then return the connection to the pool. Acquire again from the + same pool. +- **Expected:** Same `@@SPID` (no new physical login); `@@TRANCOUNT = 0`; the + `#temp` table is gone; `transaction_isolation_level` is READ COMMITTED (2); the + database is back to the login default. +- **Backing in-repo:** `connection_pool_test.cpp::ResetRestoresCleanStateForNextBorrower`, + `checkout_cycle_reuses_one_physical_connection` (unit). + +### (b) Dead idle-connection discard + +- **Steps:** Acquire a connection and record its `@@SPID`. From a *separate* + admin connection, `KILL` that SPID. Return the connection to the pool and + acquire again. +- **Expected:** The pool's checkout liveness probe + (`SQLGetConnectAttr(SQL_ATTR_CONNECTION_DEAD)`) reports the killed connection + dead (or its reset fails with `08S01`), so the pool **discards** it and hands + out a fresh physical connection (new `@@SPID`). No query is ever run on the dead + session. A never-connected / disconnected handle reports `SQL_CD_TRUE`. +- **Backing in-repo:** `get_connect_attr.rs` unit tests + (`connection_dead_reports_true_when_*`), `reset_connection_poisons_client_on_failure`; + live death-marking in `session_recovery_test.cpp`. + +### (c) Prepared statements invalidated across reset + +- **Steps:** Acquire, prepare and execute a statement (server-side prepared + handle created), return to the pool (reset), acquire again, and execute the + same logical statement. +- **Expected:** The second execution returns correct results. The reset ack + cleared the client's session-bound prepared-handle cache, so the driver + re-prepares against the fresh session instead of aliasing a dropped or unrelated + server handle. No `08S01` / invalid-handle error. +- **Backing in-repo:** `connection_pool_test.cpp::PreparedStatementSurvivesResetViaReprepare`; + `reset_connection_ack_clears_caches_and_restores_login_defaults` (TDS unit). + +### (d) Concurrent checkout + +- **Steps:** With a pool of size N, run N worker threads that each acquire, run a + short query (e.g. `SELECT @@SPID`), and release, in a loop. +- **Expected:** No panics, no cross-talk, no "connection busy" errors from the + driver; each borrower sees a clean session; `@@SPID`s are stable per physical + connection and never interleave results. A busy connection (open cursor / + active statement) is never handed out as idle, and a reset is never run on a + busy connection (rejected with `HY000`/`08S01`). +- **Backing in-repo:** `reset_connection_rejects_busy_connection` (unit); the + "no DBC mutex across network I/O" invariant in `txn.rs`. + +### (e) Token-identity separation + near-expiry reconnect + +- **Steps:** Configure the pool for AAD access-token auth (mssql-python sets the + token pre-connect). Acquire/reset/reuse within one token's validity. Then + present a **new/rotated** token and acquire. +- **Expected:** Reuse-with-reset never re-authenticates — it is the same physical + login, and the auth/recovery context is preserved (see B6). A rotated token + cannot be applied to a live session: setting `SQL_COPT_SS_ACCESS_TOKEN` after + connect is rejected (`HY011`), so a new token forces a **new physical login** + (fresh `SQLDriverConnect`), not a re-auth of the pooled session. Two connections + authenticated with different token identities never share a physical session. +- **Backing in-repo:** `access_token_after_connect_is_rejected` (unit); the B6 + auth-preservation note on `TdsClient::reset_connection`. + +### (f) Isolation-reset scenario (mssql-python #343) + +- **Steps:** Acquire, set isolation to `SERIALIZABLE` via the connection + attribute, return to the pool, re-acquire. +- **Expected:** The pool's checkout re-applies `READ COMMITTED` + (`SQLSetConnectAttr(SQL_ATTR_TXN_ISOLATION, SQL_TXN_READ_COMMITTED)`), which our + handler emits as a real `SET TRANSACTION ISOLATION LEVEL READ COMMITTED` batch + carrying the armed reset bit. `transaction_isolation_level` is 2 for the next + borrower. **Documented shared limitation:** isolation changed via *raw T-SQL* + (`SET TRANSACTION ISOLATION LEVEL ...`, bypassing the attribute) can leak, + because `sp_reset_connection` does not reset isolation (D9) and mssql-python's + #343 workaround only tracks the attribute path — this matches msodbcsql18 and is + a Non-goal to "fix." +- **Backing in-repo:** `reset_then_checkout_isolation_reapplies_read_committed` + (unit); `connection_pool_test.cpp::IsolationReturnsToReadCommittedEachCheckout`. + +## Reference verification snippet + +A minimal driver for scenarios (a) and (f), assuming `mssql-python` from GitHub +`main` and the Rust driver registered as above: + +```python +import mssql_python + +CONN = ( + "Driver={ODBC Driver 18 for SQL Server (Rust)};" + "Server=localhost;Database=tempdb;UID=sa;PWD=your-password;" + "TrustServerCertificate=Yes;" +) + +def isolation(cur): + cur.execute( + "SELECT CAST(transaction_isolation_level AS int) " + "FROM sys.dm_exec_sessions WHERE session_id = @@SPID" + ) + return cur.fetchone()[0] + +# Pool of size 1 so both acquisitions land on the same physical connection. +mssql_python.pooling(max_size=1, idle_timeout=30) + +c1 = mssql_python.connect(CONN) +cur1 = c1.cursor() +cur1.execute("SELECT @@SPID"); spid1 = cur1.fetchone()[0] +cur1.execute("SET TRANSACTION ISOLATION LEVEL SERIALIZABLE") # via T-SQL: see (f) caveat +c1.close() # returns to pool + +c2 = mssql_python.connect(CONN) +cur2 = c2.cursor() +cur2.execute("SELECT @@SPID"); spid2 = cur2.fetchone()[0] +assert spid1 == spid2, "same physical connection expected" +assert isolation(cur2) == 2, "checkout must restore READ COMMITTED" +c2.close() +``` + +Run the same script against `Driver={ODBC Driver 18 for SQL Server}` and confirm +identical results. Diverging behavior between the two drivers is the signal to +investigate (per Workstream C's "diagnose any behavior differences vs. +msodbcsql18"). + +## CI wiring (future) + +If this cross-product check is later automated in CI, prefer extending the +existing containerized ODBC e2e job (`.pipeline/scripts/containerized-odbc-e2e.sh`, +which already owns a SQL Server and registers the driver) with a Python step that +`pip install`s `mssql-python` and runs the snippets above against both driver +names, rather than adding a standalone harness. Until then, this document is the +manual verification procedure. diff --git a/mssql-odbc/src/api/get_connect_attr.rs b/mssql-odbc/src/api/get_connect_attr.rs index 29aa5f27..1478b3e9 100644 --- a/mssql-odbc/src/api/get_connect_attr.rs +++ b/mssql-odbc/src/api/get_connect_attr.rs @@ -416,6 +416,41 @@ mod tests { dbc.inner.lock().unwrap().client = None; } + #[test] + fn connection_dead_reports_true_when_client_marked_dead() { + use crate::handles::DbcHandle; + use crate::handles::handle_from_raw; + use mssql_tds::test_client_support::tds_client_from_tokens; + + let h = TestHandles::with_env_dbc(); + h.mark_dbc_connected(); + let dbc = unsafe { handle_from_raw::(h.dbc) }; + // A connected client whose session was left unusable (e.g. a reset whose + // round trip failed poisons it) must read DEAD so the pool discards it, + // even though the DBC is still marked Connected. + { + let mut state = dbc.inner.lock().unwrap(); + let mut client = tds_client_from_tokens(vec![]); + client.mark_connection_dead(); + state.client = Some(client); + } + + let mut out: u32 = 12345; + let get = unsafe { + sql_get_connect_attr_w( + h.dbc, + SQL_ATTR_CONNECTION_DEAD, + &mut out as *mut u32 as SqlPointer, + 0, + std::ptr::null_mut(), + ) + }; + assert_eq!(get, SQL_SUCCESS); + assert_eq!(out, SQL_CD_TRUE); + + dbc.inner.lock().unwrap().client = None; + } + #[test] fn connection_dead_null_pointer_is_rejected() { let h = TestHandles::with_env_dbc(); diff --git a/mssql-odbc/src/api/set_connect_attr.rs b/mssql-odbc/src/api/set_connect_attr.rs index 16d251c2..b19b6863 100644 --- a/mssql-odbc/src/api/set_connect_attr.rs +++ b/mssql-odbc/src/api/set_connect_attr.rs @@ -325,6 +325,34 @@ mod tests { assert_eq!(state.access_token.as_deref(), Some(jwt)); } + #[test] + fn access_token_after_connect_is_rejected() { + // B6: an access token is a pre-connect credential. A reset is the same + // physical login and never re-authenticates, so a rotated token cannot be + // applied to a live session — it must drive a fresh SQLDriverConnect (new + // physical login). Setting it after connect is rejected with HY011, + // locking in that there is no live-session token-refresh path. + let h = TestHandles::with_env_dbc(); + h.mark_dbc_connected(); + let buf = make_token_struct("rotated.jwt.value"); + let ret = unsafe { + sql_set_connect_attr_w( + h.dbc, + SQL_COPT_SS_ACCESS_TOKEN, + buf.as_ptr() as SqlPointer, + SQL_IS_POINTER, + ) + }; + assert_eq!(ret, SQL_ERROR); + let dbc = unsafe { handle_from_raw::(h.dbc) }; + let state = dbc.inner.lock().unwrap(); + assert_eq!(state.diag_records()[0].sql_state, SQLSTATE_HY011); + assert!( + state.access_token.is_none(), + "a post-connect token must not overwrite live-session credentials" + ); + } + #[test] fn null_token_pointer_is_rejected() { let h = TestHandles::with_env_dbc(); diff --git a/mssql-odbc/tests/e2e/CMakeLists.txt b/mssql-odbc/tests/e2e/CMakeLists.txt index e93fe0c3..cd56be6a 100644 --- a/mssql-odbc/tests/e2e/CMakeLists.txt +++ b/mssql-odbc/tests/e2e/CMakeLists.txt @@ -128,3 +128,4 @@ add_odbc_test(get_type_info_test tests/get_type_info_test.cpp) add_odbc_test(row_count_test tests/row_count_test.cpp) add_odbc_test(transaction_test tests/transaction_test.cpp) add_odbc_test(session_recovery_test tests/session_recovery_test.cpp) +add_odbc_test(connection_pool_test tests/connection_pool_test.cpp) diff --git a/mssql-odbc/tests/e2e/tests/connection_pool_test.cpp b/mssql-odbc/tests/e2e/tests/connection_pool_test.cpp new file mode 100644 index 00000000..320ecd5e --- /dev/null +++ b/mssql-odbc/tests/e2e/tests/connection_pool_test.cpp @@ -0,0 +1,298 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// connection_pool_test.cpp – E2E tests for the connection-pool check-in reset +// (SQL_ATTR_RESET_CONNECTION), SQL_ATTR_CONNECTION_DEAD liveness, and the +// same-physical-connection reuse a client-side pool (e.g. mssql-python) depends +// on. +// +// Structure mirrors transaction_test.cpp: a live SQL Server is required and the +// suite skips cleanly when none is configured. Every assertion here must also +// hold for msodbcsql 18, because the same binary runs against both drivers under +// `run_e2e.ps1 -CompareWithMsodbcsql`. +// +// The scenario reproduces one physical connection serving two consecutive +// borrowers: borrower A mutates session state (isolation, temp table, open +// transaction, current database), the pool checks the connection in and resets +// it, then borrower B — on the SAME @@SPID — must observe clean login defaults. + +#include "odbc_test_fixture.h" + +#include + +// SQL_ATTR_RESET_CONNECTION / SQL_RESET_CONNECTION_YES arrived in ODBC 3.8. +// Older Driver Manager headers may lack them. +#ifndef SQL_ATTR_RESET_CONNECTION +#define SQL_ATTR_RESET_CONNECTION 116 +#endif +#ifndef SQL_RESET_CONNECTION_YES +#define SQL_RESET_CONNECTION_YES 1 +#endif + +// SQL_ATTR_CONNECTION_DEAD and its SQL_CD_* values. +#ifndef SQL_ATTR_CONNECTION_DEAD +#define SQL_ATTR_CONNECTION_DEAD 1209 +#endif +#ifndef SQL_CD_TRUE +#define SQL_CD_TRUE 1 +#endif +#ifndef SQL_CD_FALSE +#define SQL_CD_FALSE 0 +#endif + +// =================================================================== +// Tests that require a live SQL Server +// =================================================================== + +class ConnectionPoolLiveTest : public ODBCTest { +protected: + void SetUp() override { + ODBCTest::SetUp(); + if (!ODBCTestConfig::Instance().HasConnection()) { + GTEST_SKIP() << "No connection configured – set ODBC_TEST_SERVER or ODBC_TEST_CONNSTR"; + } + Connect(); + } + + void TearDown() override { + // Leave the connection idle and in autocommit so SQLDisconnect never + // trips the "transaction still open" (25000) guard during teardown. + if (dbc_ != SQL_NULL_HDBC) { + SQLEndTran(SQL_HANDLE_DBC, dbc_, SQL_ROLLBACK); + SetAutocommit(dbc_, SQL_AUTOCOMMIT_ON); + } + ODBCTest::TearDown(); + } + + // --- Attribute helpers ------------------------------------------------- + + static SQLRETURN SetAutocommit(SQLHDBC dbc, SQLUINTEGER mode) { + return SQLSetConnectAttr(dbc, SQL_ATTR_AUTOCOMMIT, + reinterpret_cast(static_cast(mode)), + SQL_IS_UINTEGER); + } + + static SQLRETURN SetIsolation(SQLHDBC dbc, SQLUINTEGER level) { + return SQLSetConnectAttr(dbc, SQL_ATTR_TXN_ISOLATION, + reinterpret_cast(static_cast(level)), + SQL_IS_UINTEGER); + } + + static SQLRETURN ResetConnection(SQLHDBC dbc) { + return SQLSetConnectAttr( + dbc, SQL_ATTR_RESET_CONNECTION, + reinterpret_cast(static_cast(SQL_RESET_CONNECTION_YES)), + SQL_IS_UINTEGER); + } + + static SQLUINTEGER GetConnectionDead(SQLHDBC dbc) { + SQLUINTEGER value = 0xDEAD; + SQLRETURN rc = + SQLGetConnectAttr(dbc, SQL_ATTR_CONNECTION_DEAD, &value, SQL_IS_UINTEGER, nullptr); + EXPECT_SQL_OK(rc, SQL_HANDLE_DBC, dbc); + return value; + } + + // --- SQL helpers ------------------------------------------------------- + + static SQLRETURN Run(SQLHSTMT hstmt, const std::string& sql) { + SqlTString text = ODBCTestUtils::ToSqlTStr(sql); + return SQLExecDirect(hstmt, const_cast(text.c_str()), SQL_NTS); + } + + void Exec(const std::string& sql) { + SQLRETURN rc = Run(stmt_, sql); + ASSERT_SQL_OK(rc, SQL_HANDLE_STMT, stmt_); + } + + /// Execute a single-column, single-row integer query and return the value. + SQLINTEGER Scalar(const std::string& sql) { + SQLRETURN rc = Run(stmt_, sql); + EXPECT_SQL_OK(rc, SQL_HANDLE_STMT, stmt_); + rc = SQLFetch(stmt_); + EXPECT_SQL_OK(rc, SQL_HANDLE_STMT, stmt_); + + SQLINTEGER value = -1; + SQLLEN indicator = 0; + rc = SQLGetData(stmt_, 1, SQL_C_SLONG, &value, sizeof(value), &indicator); + EXPECT_SQL_OK(rc, SQL_HANDLE_STMT, stmt_); + SQLCloseCursor(stmt_); + return value; + } + + SQLINTEGER Spid() { return Scalar("SELECT @@SPID"); } + SQLINTEGER TranCount() { return Scalar("SELECT @@TRANCOUNT"); } + SQLINTEGER DatabaseId() { return Scalar("SELECT DB_ID()"); } + + /// Server-side view of the session isolation level (1..5), which is what + /// SET TRANSACTION ISOLATION LEVEL actually changed. + SQLINTEGER ServerIsolation() { + return Scalar( + "SELECT CAST(transaction_isolation_level AS int) FROM sys.dm_exec_sessions " + "WHERE session_id = @@SPID"); + } + + /// True if |name| (e.g. "#probe") resolves in tempdb for this session. + bool TempTableExists(const std::string& name) { + return Scalar("SELECT CASE WHEN OBJECT_ID('tempdb.." + name + + "') IS NULL THEN 0 ELSE 1 END") != 0; + } + + /// Model the pool check-in + next-acquire the way mssql-python does: roll + /// back the borrower's transaction and return to autocommit (Connection + /// close), then arm the reset and re-apply READ COMMITTED (Connection reset + /// on the next acquire — the isolation SET is deliberate because + /// sp_reset_connection does not restore isolation, D9/#343). + void CheckInAndReset() { + ASSERT_SQL_OK(SQLEndTran(SQL_HANDLE_DBC, dbc_, SQL_ROLLBACK), SQL_HANDLE_DBC, dbc_); + ASSERT_SQL_OK(SetAutocommit(dbc_, SQL_AUTOCOMMIT_ON), SQL_HANDLE_DBC, dbc_); + ASSERT_SQL_OK(ResetConnection(dbc_), SQL_HANDLE_DBC, dbc_); + ASSERT_SQL_OK(SetIsolation(dbc_, SQL_TXN_READ_COMMITTED), SQL_HANDLE_DBC, dbc_); + } +}; + +// The full check-in reset: borrower A raises isolation, creates a temp table, +// opens a transaction and switches database; after the pool resets the +// connection, borrower B on the SAME physical connection sees login defaults — +// no leaked temp table, no open transaction, isolation back to READ COMMITTED, +// and the original database. This is the pooling analogue of +// transaction_test.cpp's cross-connection checks. +TEST_F(ConnectionPoolLiveTest, ResetRestoresCleanStateForNextBorrower) { + const SQLINTEGER spid_before = Spid(); + const SQLINTEGER login_db = DatabaseId(); + + // Borrower A dirties the session. + ASSERT_SQL_OK(SetIsolation(dbc_, SQL_TXN_SERIALIZABLE), SQL_HANDLE_DBC, dbc_); + Exec("CREATE TABLE #pool_probe(i int)"); + Exec("INSERT INTO #pool_probe VALUES (1), (2)"); + ASSERT_TRUE(TempTableExists("#pool_probe")); + Exec("USE master"); + EXPECT_EQ(4, ServerIsolation()) << "A raised isolation to SERIALIZABLE"; + ASSERT_SQL_OK(SetAutocommit(dbc_, SQL_AUTOCOMMIT_OFF), SQL_HANDLE_DBC, dbc_); + Exec("INSERT INTO #pool_probe VALUES (3)"); + EXPECT_GE(TranCount(), 1) << "A left a transaction open"; + + // Pool check-in + next acquire. + CheckInAndReset(); + + // Borrower B, same physical connection, sees a clean session. + EXPECT_EQ(spid_before, Spid()) << "reset must reuse the same physical connection"; + EXPECT_EQ(0, TranCount()) << "no transaction may leak across check-in"; + EXPECT_FALSE(TempTableExists("#pool_probe")) << "the temp table must not survive the reset"; + EXPECT_EQ(2, ServerIsolation()) << "isolation must return to READ COMMITTED"; + EXPECT_EQ(login_db, DatabaseId()) << "database must return to the login default"; +} + +// The reset itself keeps the connection usable: CONNECTION_DEAD reads FALSE on a +// healthy connection both before and after the reset, so a pool does not discard +// a perfectly good connection. +TEST_F(ConnectionPoolLiveTest, ConnectionDeadStaysFalseAcrossReset) { + EXPECT_EQ(static_cast(SQL_CD_FALSE), GetConnectionDead(dbc_)); + ASSERT_SQL_OK(ResetConnection(dbc_), SQL_HANDLE_DBC, dbc_); + EXPECT_EQ(static_cast(SQL_CD_FALSE), GetConnectionDead(dbc_)) + << "a successful reset leaves the connection alive and reusable"; + EXPECT_EQ(0, TranCount()); +} + +// mssql-python re-applies READ COMMITTED on every checkout because +// sp_reset_connection does not reset isolation (D9). A borrower that raised +// isolation via the attribute must see it restored after the reset+re-apply, +// and the same physical connection stays reusable across repeated cycles. +TEST_F(ConnectionPoolLiveTest, IsolationReturnsToReadCommittedEachCheckout) { + const SQLINTEGER spid_before = Spid(); + + for (int cycle = 0; cycle < 2; ++cycle) { + ASSERT_SQL_OK(SetIsolation(dbc_, SQL_TXN_SERIALIZABLE), SQL_HANDLE_DBC, dbc_); + EXPECT_EQ(4, ServerIsolation()) << "cycle " << cycle << ": raised to SERIALIZABLE"; + + CheckInAndReset(); + + EXPECT_EQ(2, ServerIsolation()) << "cycle " << cycle << ": back to READ COMMITTED"; + EXPECT_EQ(spid_before, Spid()) << "cycle " << cycle << ": same physical connection"; + } +} + +// Parity-safe portion: both drivers must keep the connection usable across a pool +// reset for newly prepared statements. A statement prepared and executed before +// the reset works, and after the reset a freshly prepared statement (a new +// prepare, not a reuse of the old server-side handle) also works. This holds on +// both mssql-odbc and msodbcsql, so it runs on both legs with no skip. +TEST_F(ConnectionPoolLiveTest, PreparedStatementUsableAcrossReset) { + SqlTString sql = ODBCTestUtils::ToSqlTStr("SELECT 42"); + ASSERT_SQL_OK(SQLPrepare(stmt_, const_cast(sql.c_str()), SQL_NTS), SQL_HANDLE_STMT, + stmt_); + + ASSERT_SQL_OK(SQLExecute(stmt_), SQL_HANDLE_STMT, stmt_); + SQLINTEGER value = 0; + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_SLONG, &value, sizeof(value), nullptr), SQL_HANDLE_STMT, + stmt_); + EXPECT_EQ(42, value); + SQLCloseCursor(stmt_); + + CheckInAndReset(); + + // Freshly prepare the statement again after the reset (a new prepare, not a + // reuse of the old server-side handle): the connection stays usable on both + // drivers. + ASSERT_SQL_OK(SQLPrepare(stmt_, const_cast(sql.c_str()), SQL_NTS), SQL_HANDLE_STMT, + stmt_); + ASSERT_SQL_OK(SQLExecute(stmt_), SQL_HANDLE_STMT, stmt_); + value = 0; + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_SLONG, &value, sizeof(value), nullptr), SQL_HANDLE_STMT, + stmt_); + EXPECT_EQ(42, value) << "a freshly prepared statement must work after the pool reset"; + SQLCloseCursor(stmt_); +} + +// Isolates ONLY the mssql-odbc-specific transparent-re-prepare divergence: after +// the reset, re-executing the SAME already-prepared handle without re-preparing +// must succeed. Stage 1 clears session-bound prepared-statement handles on the +// reset ack, so the driver transparently re-prepares against the fresh session +// rather than aliasing a stale or dropped handle. sp_reset_connection drops the +// server-side prepared handles, so msodbcsql blindly re-executes the dropped +// handle and fails with native error 8179 ("Could not find prepared statement +// with handle N"). SKIP_IF_COMPARING_MSODBCSQL() keeps the parity comparison +// honest (see transaction_test.cpp / get_data_test.cpp); the shared "usable +// across reset" behavior is covered by PreparedStatementUsableAcrossReset, which +// runs on both legs. +TEST_F(ConnectionPoolLiveTest, PreparedStatementSurvivesResetViaReprepare) { + SKIP_IF_COMPARING_MSODBCSQL(); + + SqlTString sql = ODBCTestUtils::ToSqlTStr("SELECT 42"); + ASSERT_SQL_OK(SQLPrepare(stmt_, const_cast(sql.c_str()), SQL_NTS), SQL_HANDLE_STMT, + stmt_); + + ASSERT_SQL_OK(SQLExecute(stmt_), SQL_HANDLE_STMT, stmt_); + SQLINTEGER value = 0; + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_SLONG, &value, sizeof(value), nullptr), SQL_HANDLE_STMT, + stmt_); + EXPECT_EQ(42, value); + SQLCloseCursor(stmt_); + + CheckInAndReset(); + + // Re-execute the same prepared handle after the reset without re-preparing: it + // must transparently re-prepare and return the correct result, not fail on a + // dropped server-side handle. + ASSERT_SQL_OK(SQLExecute(stmt_), SQL_HANDLE_STMT, stmt_); + value = 0; + ASSERT_SQL_OK(SQLFetch(stmt_), SQL_HANDLE_STMT, stmt_); + ASSERT_SQL_OK(SQLGetData(stmt_, 1, SQL_C_SLONG, &value, sizeof(value), nullptr), SQL_HANDLE_STMT, + stmt_); + EXPECT_EQ(42, value) << "the statement must survive the reset via transparent re-prepare"; + SQLCloseCursor(stmt_); +} + +// Value validation is enforced at the driver: only SQL_RESET_CONNECTION_YES is +// accepted; any other value is rejected with HY024 and leaves the connection +// untouched (D7). No server round trip is needed, so this holds without a live +// connection too, but it runs here alongside the rest of the pool surface. +TEST_F(ConnectionPoolLiveTest, ResetRejectsNonYesValue) { + EXPECT_SQL_ERROR(SQLSetConnectAttr(dbc_, SQL_ATTR_RESET_CONNECTION, + reinterpret_cast(static_cast(2)), + SQL_IS_UINTEGER)); + EXPECT_SQLSTATE(SQL_HANDLE_DBC, dbc_, "HY024"); + // The connection is still healthy and usable after the rejected set. + EXPECT_EQ(static_cast(SQL_CD_FALSE), GetConnectionDead(dbc_)); +} diff --git a/mssql-tds/src/connection/tds_client.rs b/mssql-tds/src/connection/tds_client.rs index a281e349..e44d0842 100644 --- a/mssql-tds/src/connection/tds_client.rs +++ b/mssql-tds/src/connection/tds_client.rs @@ -851,6 +851,14 @@ impl TdsClient { /// /// `SELECT 1` is the cheapest batch that reliably carries the reset bit and /// forces the acknowledging ENVCHANGE. + /// + /// The reset is the *same physical login*: it never re-runs the LOGIN7 / + /// fedauth handshake and never re-sends the access token, so the + /// authentication and session-recovery context (the `client_context` held in + /// [`recovery_context`](Self::recovery_context)) is preserved untouched. A + /// new or rotated access token is only ever consumed at connect time, so + /// rotating credentials means a fresh connection (new LOGIN7), not a re-auth + /// of this live session. pub async fn reset_connection(&mut self) -> TdsResult<()> { self.prepare_reset_connection(false); self.execute("SELECT 1".to_string(), ()).await?;