Stage 2: ODBC pooling primitives (ADO #47338)

mssql-odbc/src/api/get_connect_attr.rs

@@ -14,13 +14,15 @@ use tracing::{debug, error};
 
 use super::sqlstate::*;
 use crate::api::odbc_types::{
-    SQL_ATTR_ACCESS_MODE, SQL_ATTR_AUTOCOMMIT, SQL_ATTR_CONNECTION_TIMEOUT, SQL_ATTR_LOGIN_TIMEOUT,
-    SQL_ATTR_PACKET_SIZE, SQL_ATTR_TXN_ISOLATION, SQL_AUTOCOMMIT_OFF, SQL_AUTOCOMMIT_ON,
+    SQL_ATTR_ACCESS_MODE, SQL_ATTR_AUTOCOMMIT, SQL_ATTR_CONNECTION_DEAD,
+    SQL_ATTR_CONNECTION_TIMEOUT, SQL_ATTR_LOGIN_TIMEOUT, SQL_ATTR_PACKET_SIZE,
+    SQL_ATTR_TXN_ISOLATION, SQL_AUTOCOMMIT_OFF, SQL_AUTOCOMMIT_ON, SQL_CD_FALSE, SQL_CD_TRUE,
     SQL_COPT_SS_TXN_ISOLATION, SQL_ERROR, SQL_INVALID_HANDLE, SQL_SUCCESS, SqlHandle, SqlInteger,
     SqlPointer, SqlReturn,
 };
 use crate::api::util::write_if_some;
 use crate::error::{free_errors, post_sql_error};
+use crate::handles::dbc::ConnectionState;
 use crate::handles::{DbcHandle, HandleType, handle_from_raw};
 
 /// Login timeout reported when the application has not set
@@ -144,6 +146,29 @@ fn sql_get_connect_attr_w_safe(
             debug!(attribute, value, "SQLGetConnectAttrW: attribute returned");
             SQL_SUCCESS
         }
+        SQL_ATTR_CONNECTION_DEAD => {
+            if value_ptr.is_null() {
+                error!("SQLGetConnectAttrW: SQL_ATTR_CONNECTION_DEAD value pointer is null");
+                post_diag(&mut state, ERR_INVALID_NULL_POINTER);
+                return SQL_ERROR;
+            }
+            // Cached, never-probe liveness (D1/D2): report DEAD unless the DBC is
+            // connected and its client has not been observed dead. Peek the
+            // client under the DBC mutex — never `take` it (D8) and never touch
+            // the socket. `SQL_CD_FALSE` means "not known dead", not "proven
+            // healthy"; a connection that failed silently while idle is
+            // recovered on the next operation. Disconnected/never-connected
+            // reads DEAD so a pool discards it.
+            let alive = state.connection_state == ConnectionState::Connected
+                && state
+                    .client
+                    .as_ref()
+                    .is_some_and(|client| !client.is_connection_dead());
+            let value = if alive { SQL_CD_FALSE } else { SQL_CD_TRUE };
+            unsafe { write_if_some(value_ptr as *mut u32, value) };
+            debug!(value, "SQLGetConnectAttrW: connection-dead returned");
+            SQL_SUCCESS
+        }
         // Any other attribute is genuinely unsupported: surface HYC00 instead of
         // claiming success while leaving the caller's buffer untouched.
         // `SQL_ATTR_ANSI_APP` lands here deliberately — the Driver Manager sets
@@ -168,8 +193,8 @@ fn sql_get_connect_attr_w_safe(
 mod tests {
     use super::*;
     use crate::api::odbc_types::{
-        DEFAULT_PACKET_SIZE, SQL_ATTR_ANSI_APP, SQL_MODE_READ_WRITE, SQL_TXN_READ_COMMITTED,
-        SQL_TXN_SS_SNAPSHOT,
+        DEFAULT_PACKET_SIZE, SQL_ATTR_ANSI_APP, SQL_ATTR_CONNECTION_DEAD, SQL_CD_FALSE,
+        SQL_CD_TRUE, SQL_MODE_READ_WRITE, SQL_TXN_READ_COMMITTED, SQL_TXN_SS_SNAPSHOT,
     };
     use crate::api::set_connect_attr::sql_set_connect_attr_w;
     use crate::test_support::TestHandles;
@@ -320,4 +345,89 @@ mod tests {
         };
         assert_eq!(get, SQL_ERROR);
     }
+
+    #[test]
+    fn connection_dead_reports_true_when_never_connected() {
+        // D1: msodbcsql defaults the attribute to DEAD until a token read
+        // succeeds; a freshly allocated DBC has no client, so a pool must
+        // discard it.
+        let h = TestHandles::with_env_dbc();
+        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);
+    }
+
+    #[test]
+    fn connection_dead_reports_true_when_marked_connected_without_client() {
+        // `Connected` state but no client (mid-operation, taken) still reads
+        // DEAD: liveness is a property of the client, not the state flag.
+        let h = TestHandles::with_env_dbc();
+        h.mark_dbc_connected();
+        let mut out: u32 = 0;
+        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);
+    }
+
+    #[test]
+    fn connection_dead_reports_false_when_connected_and_alive() {
+        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::<DbcHandle>(h.dbc) };
+        // A replay-transport client has not been observed dead, so the cached
+        // liveness read reports ALIVE.
+        dbc.inner.lock().unwrap().client = Some(tds_client_from_tokens(vec![]));
+
+        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_FALSE);
+
+        // Clear the client so TestHandles drop does not try to disconnect it.
+        dbc.inner.lock().unwrap().client = None;
+    }
+
+    #[test]
+    fn connection_dead_null_pointer_is_rejected() {
+        let h = TestHandles::with_env_dbc();
+        let get = unsafe {
+            sql_get_connect_attr_w(
+                h.dbc,
+                SQL_ATTR_CONNECTION_DEAD,
+                std::ptr::null_mut(),
+                0,
+                std::ptr::null_mut(),
+            )
+        };
+        assert_eq!(get, SQL_ERROR);
+    }
 }

mssql-odbc/src/api/odbc_types.rs

@@ -69,6 +69,24 @@ pub const SQL_ATTR_PACKET_SIZE: SqlInteger = 112;
 pub const SQL_ATTR_CONNECTION_TIMEOUT: SqlInteger = 113;
 pub const SQL_ATTR_ANSI_APP: SqlInteger = 115;
 
+// Connection-pooling attributes.
+//
+// `SQL_ATTR_RESET_CONNECTION` is armed by the pool at check-in so the next
+// request resets the session to its login defaults; `SQL_ATTR_CONNECTION_DEAD`
+// is a read-only liveness flag the pool consults before handing a connection
+// out.
+pub const SQL_ATTR_RESET_CONNECTION: SqlInteger = 116;
+pub const SQL_ATTR_CONNECTION_DEAD: SqlInteger = 1209;
+
+// `SQL_ATTR_RESET_CONNECTION` value. Only `SQL_RESET_CONNECTION_YES` is defined;
+// any other value is HY024.
+pub const SQL_RESET_CONNECTION_YES: u32 = 1;
+
+// `SQL_ATTR_CONNECTION_DEAD` values. msodbcsql reports DEAD until a token read
+// succeeds, so disconnected/never-connected reads DEAD.
+pub const SQL_CD_TRUE: u32 = 1;
+pub const SQL_CD_FALSE: u32 = 0;
+
 // `SQL_ATTR_ACCESS_MODE` values.
 pub const SQL_MODE_READ_WRITE: u32 = 0;
 

mssql-odbc/src/api/set_connect_attr.rs

@@ -11,12 +11,13 @@
 use tracing::{debug, error};
 
 use super::sqlstate::*;
-use super::txn::{set_autocommit, set_txn_isolation};
+use super::txn::{reset_connection, set_autocommit, set_txn_isolation};
 use crate::api::odbc_types::{
     SQL_ATTR_ACCESS_MODE, SQL_ATTR_ANSI_APP, SQL_ATTR_AUTOCOMMIT, SQL_ATTR_CONNECTION_TIMEOUT,
-    SQL_ATTR_LOGIN_TIMEOUT, SQL_ATTR_PACKET_SIZE, SQL_ATTR_TXN_ISOLATION, SQL_COPT_SS_ACCESS_TOKEN,
-    SQL_COPT_SS_TXN_ISOLATION, SQL_ERROR, SQL_INVALID_HANDLE, SQL_SUCCESS, SQL_SUCCESS_WITH_INFO,
-    SqlHandle, SqlInteger, SqlPointer, SqlReturn,
+    SQL_ATTR_LOGIN_TIMEOUT, SQL_ATTR_PACKET_SIZE, SQL_ATTR_RESET_CONNECTION,
+    SQL_ATTR_TXN_ISOLATION, SQL_COPT_SS_ACCESS_TOKEN, SQL_COPT_SS_TXN_ISOLATION, SQL_ERROR,
+    SQL_INVALID_HANDLE, SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SqlHandle, SqlInteger, SqlPointer,
+    SqlReturn,
 };
 use crate::error::{free_errors, post_sql_error};
 use crate::handles::dbc::ConnectionState;
@@ -86,6 +87,9 @@ unsafe fn sql_set_connect_attr_w_impl(
         SQL_ATTR_TXN_ISOLATION | SQL_COPT_SS_TXN_ISOLATION => {
             return set_txn_isolation(dbc, value_ptr as usize as u64);
         }
+        // Pooling check-in reset: rolls back any live local transaction and arms
+        // the RESETCONNECTION bit, so it must not run under the DBC mutex either.
+        SQL_ATTR_RESET_CONNECTION => return reset_connection(dbc, value_ptr as usize as u64),
         _ => {}
     }
 

mssql-odbc/src/api/txn.rs

@@ -17,9 +17,9 @@ use tracing::{debug, error};
 
 use super::close_cursor::close_cursor_for_connection_op;
 use super::odbc_types::{
-    SQL_AUTOCOMMIT_OFF, SQL_AUTOCOMMIT_ON, SQL_ERROR, SQL_SUCCESS, SQL_SUCCESS_WITH_INFO,
-    SQL_TXN_READ_COMMITTED, SQL_TXN_READ_UNCOMMITTED, SQL_TXN_REPEATABLE_READ,
-    SQL_TXN_SERIALIZABLE, SQL_TXN_SS_SNAPSHOT, SqlReturn,
+    SQL_AUTOCOMMIT_OFF, SQL_AUTOCOMMIT_ON, SQL_ERROR, SQL_RESET_CONNECTION_YES, SQL_SUCCESS,
+    SQL_SUCCESS_WITH_INFO, SQL_TXN_READ_COMMITTED, SQL_TXN_READ_UNCOMMITTED,
+    SQL_TXN_REPEATABLE_READ, SQL_TXN_SERIALIZABLE, SQL_TXN_SS_SNAPSHOT, SqlReturn,
 };
 use super::sqlstate::{
     ERR_ATTRIBUTE_CANNOT_BE_SET_NOW, ERR_CONNECTION_BUSY, ERR_CONNECTION_DOES_NOT_EXIST,
@@ -358,6 +358,73 @@ pub(super) fn set_autocommit(dbc: &DbcHandle, value: u64) -> SqlReturn {
     }
 }
 
+/// Applies `SQL_ATTR_RESET_CONNECTION` — the connection-pool check-in reset.
+///
+/// Mirrors msodbcsql's `SQL_COPT_SS_RESET_CONNECTION` handler
+/// (`sqlcmisc.cpp:2373-2461`): reject any value but `SQL_RESET_CONNECTION_YES`
+/// with HY024 (D7), roll back a live local transaction first (D4), then arm the
+/// full RESETCONNECTION bit so the next request resets the session to its login
+/// defaults. Pool checkout does not preserve transactions, so this never uses
+/// RESETCONNECTIONSKIPTRAN.
+///
+/// The bit only rides the next request here; making the reset self-acking with
+/// its own round trip is a later stage.
+pub(super) fn reset_connection(dbc: &DbcHandle, value: u64) -> SqlReturn {
+    const OP: &str = "SQLSetConnectAttrW(SQL_ATTR_RESET_CONNECTION)";
+
+    {
+        let Ok(mut state) = dbc.inner.lock() else {
+            error!("{OP}: dbc mutex poisoned");
+            return SQL_ERROR;
+        };
+        free_errors(&mut state);
+        if value != u64::from(SQL_RESET_CONNECTION_YES) {
+            error!(value, "{OP}: invalid value");
+            post_diag(&mut state, ERR_INVALID_ATTRIBUTE_VALUE);
+            return SQL_ERROR;
+        }
+    }
+
+    // Claim the idle client: this rejects a busy connection (open cursor /
+    // `active_stmt`) with ERR_CONNECTION_BUSY and a disconnected DBC with
+    // ERR_CONNECTION_DOES_NOT_EXIST (08003, D7).
+    let mut client = match claim_dbc_client(dbc, OP) {
+        Ok(c) => c,
+        Err(ret) => return ret,
+    };
+
+    // D4: roll back a live local transaction before the reset so the next
+    // borrower cannot inherit it. Guard on the server actually having one, the
+    // same way `end_transaction` does — the flag can be stale.
+    let started = match dbc.inner.lock() {
+        Ok(state) => state.local_tran_started,
+        Err(_) => false,
+    };
+    let result = if started && client.has_active_transaction() {
+        debug!("{OP}: rolling back live local transaction before reset");
+        dbc.runtime
+            .block_on(client.rollback_transaction(None, None))
+    } else {
+        Ok(())
+    };
+
+    client.prepare_reset_connection(false);
+    release_dbc_client(dbc, client);
+
+    let Ok(mut state) = dbc.inner.lock() else {
+        error!("{OP}: dbc mutex poisoned");
+        return SQL_ERROR;
+    };
+    state.local_tran_started = false;
+    if let Err(e) = result {
+        error!(%e, "{OP}: rollback before reset failed");
+        post_tds_error(&mut state, &e, SQLSTATE_HY000);
+        return SQL_ERROR;
+    }
+    debug!("{OP}: reset armed for next request");
+    SQL_SUCCESS
+}
+
 /// Manual-commit → autocommit. Any transaction holding user work is **committed**
 /// with a `01000` warning; a driver-begun piggyback transaction is rolled back
 /// silently (`sqlcconn.cpp:3692-3741`).
@@ -759,4 +826,103 @@ mod tests {
             "the no-transaction path must still sweep cursors"
         );
     }
+
+    #[test]
+    fn reset_connection_rejects_non_yes_value() {
+        // D7: only SQL_RESET_CONNECTION_YES(1) is valid; anything else is HY024.
+        // Value validation runs before the connection is claimed.
+        use crate::api::odbc_types::SQL_ATTR_RESET_CONNECTION;
+        use crate::api::set_connect_attr::sql_set_connect_attr_w;
+        use crate::api::sqlstate::SQLSTATE_HY024;
+        use crate::error::HasDiagnostics;
+        use crate::test_support::TestHandles;
+
+        let h = TestHandles::with_env_dbc();
+        let ret =
+            unsafe { sql_set_connect_attr_w(h.dbc, SQL_ATTR_RESET_CONNECTION, 2usize as _, 0) };
+        assert_eq!(ret, SQL_ERROR);
+        let dbc = unsafe { handle_from_raw::<DbcHandle>(h.dbc) };
+        assert_eq!(
+            dbc.inner.lock().unwrap().diag_records()[0].sql_state,
+            SQLSTATE_HY024
+        );
+    }
+
+    #[test]
+    fn reset_connection_on_disconnected_dbc_is_08003() {
+        // D7: reset on a connection that does not exist surfaces 08003, the
+        // diagnostic `claim_dbc_client` posts for the disconnected case.
+        use crate::api::odbc_types::{SQL_ATTR_RESET_CONNECTION, SQL_RESET_CONNECTION_YES};
+        use crate::api::set_connect_attr::sql_set_connect_attr_w;
+        use crate::api::sqlstate::SQLSTATE_08003;
+        use crate::error::HasDiagnostics;
+        use crate::test_support::TestHandles;
+
+        let h = TestHandles::with_env_dbc();
+        let ret = unsafe {
+            sql_set_connect_attr_w(
+                h.dbc,
+                SQL_ATTR_RESET_CONNECTION,
+                SQL_RESET_CONNECTION_YES as usize as _,
+                0,
+            )
+        };
+        assert_eq!(ret, SQL_ERROR);
+        let dbc = unsafe { handle_from_raw::<DbcHandle>(h.dbc) };
+        assert_eq!(
+            dbc.inner.lock().unwrap().diag_records()[0].sql_state,
+            SQLSTATE_08003
+        );
+    }
+
+    #[test]
+    fn reset_connection_rejects_busy_connection() {
+        // An open cursor / in-progress result set pins `active_stmt`; the reset
+        // must not touch a busy connection.
+        use std::ffi::c_void;
+
+        use crate::error::HasDiagnostics;
+        use crate::test_support::TestHandles;
+        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::<DbcHandle>(h.dbc) };
+        {
+            let mut state = dbc.inner.lock().unwrap();
+            state.client = Some(tds_client_from_tokens(vec![]));
+            state.active_stmt = Some(std::ptr::dangling_mut::<c_void>());
+        }
+
+        assert_eq!(reset_connection(dbc, 1), SQL_ERROR);
+        let state = dbc.inner.lock().unwrap();
+        assert_eq!(state.diag_records()[0].sql_state, SQLSTATE_HY000);
+        // The busy connection was rejected without taking the client.
+        assert!(state.client.is_some());
+    }
+
+    #[test]
+    fn reset_connection_arms_and_clears_local_tran() {
+        // A successful reset leaves the idle client in place and clears the
+        // driver-side transaction flag (D4). The RESETCONNECTION bit riding the
+        // next request is covered by the mssql-tds transport test.
+        use crate::error::HasDiagnostics;
+        use crate::test_support::TestHandles;
+        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::<DbcHandle>(h.dbc) };
+        {
+            let mut state = dbc.inner.lock().unwrap();
+            state.client = Some(tds_client_from_tokens(vec![]));
+            state.local_tran_started = true;
+        }
+
+        assert_eq!(reset_connection(dbc, 1), SQL_SUCCESS);
+        let state = dbc.inner.lock().unwrap();
+        assert!(!state.local_tran_started);
+        assert!(state.client.is_some());
+        assert!(state.diag_records().is_empty());
+    }
 }