SqlConnectionInternal starts a fresh ConnectTimeout for in-open round trips instead of honoring the caller's remaining timeout

[priyankatiwari08]

Summary

Several call sites in SqlConnectionInternal issue a TDS round trip during connection open using a fresh ConnectionOptions.ConnectTimeout, rather than the TimeoutTimer already counting down for the in-flight Open(). Time already spent (pool wait, login) therefore does not count against the caller's budget, so a connection string with Connect Timeout=15 can block substantially longer than 15 seconds.

Raised by @mdaigle during review of #4335: #4335 (comment)

Affected call sites

All in src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs:

Line	Call
757	ChangeDatabase
2128	GetDTCAddress
2154	PropagateTransactionCookie
2419	ReassertSessionIsolationLevel (added by #4335)
2795	Transaction manager request

Each passes ConnectionOptions.ConnectTimeout. Downstream, TdsParser.TdsExecuteSQLBatch calls TdsParserStateObject.SetTimeoutSeconds, which restarts _timeoutTime from the current time.

Example

With Connect Timeout=15:

• 10s waiting for a pooled connection
• 4s login
• the in-open batch then starts a new 15s budget

Worst case is roughly 29s against a documented 15s limit.

Why this is not a one-line fix

Activate(Transaction) is an abstract override on DbConnectionInternal with no timeout parameter. It is reached through DbConnectionInternal.ActivateConnection(Transaction) from both WaitHandleDbConnectionPool and ChannelDbConnectionPool. Threading a TimeoutTimer down requires changing the base signature and both pool implementations.

Existing precedent

SqlConnectionInternal.ResolveLoginTimeout already models exactly this choice for the login phase:

internal static TimeoutTimer ResolveLoginTimeout(TimeoutTimer callerTimeout, int connectTimeoutSeconds)
    => LocalAppContextSwitches.UseOverallConnectTimeoutForPoolWait
        ? callerTimeout
        : TimeoutTimer.StartNew(TimeSpan.FromSeconds(connectTimeoutSeconds));

Switch.Microsoft.Data.SqlClient.UseOverallConnectTimeoutForPoolWait defaults to false, i.e. the driver currently restarts the clock by default. Any fix here should decide whether these five call sites follow that switch or change unconditionally.

Suggested scope

1. Thread the live TimeoutTimer through ActivateConnection / Activate and both pool implementations.
2. Apply the change consistently across all five call sites.
3. Decide switch-gated vs. unconditional, consistent with UseOverallConnectTimeoutForPoolWait.

Note: the connection pool implementations are under active rewrite, so this likely wants to sequence after that work.

[github-actions]

🔍 Triage Summary

Check	Result
Issue type	Bug (behavioral correctness — documented Connect Timeout may be substantially exceeded)
Environment	N/A — this is an internal code correctness issue identified during code review; no user-supplied repro required
Area	Area\Connection Pooling
Duplicates	None found
Regression	Not a regression — this is a long-standing design gap; related to existing UseOverallConnectTimeoutForPoolWait switch precedent

Analysis

Five call sites in SqlConnectionInternal (lines 757, 2128, 2154, 2419, 2795) issue TDS round trips during connection activation using a fresh ConnectTimeout budget rather than the already-running TimeoutTimer from Open(). This means pool wait time and login time do not count against the caller's budget, potentially extending a Connect Timeout=15 connection up to ~29 seconds in the worst case. The root cause is that Activate(Transaction) has no timeout parameter in the DbConnectionInternal base class, so threading the live timer requires signature changes to both WaitHandleDbConnectionPool and ChannelDbConnectionPool. A precedent for this pattern already exists in ResolveLoginTimeout / UseOverallConnectTimeoutForPoolWait. Severity: P2 — correctness violation of documented behavior with a measurable user-visible impact.

Next Steps

• Assign to Copilot coding agent to investigate and implement a fix. Specifically: (1) thread a TimeoutTimer parameter through DbConnectionInternal.ActivateConnection → Activate and update both pool implementations (WaitHandleDbConnectionPool, ChannelDbConnectionPool); (2) update all five call sites in SqlConnectionInternal to pass the live timer; (3) decide whether the fix should be unconditional or gated on UseOverallConnectTimeoutForPoolWait — given both switches share the same semantic intent, gating is preferred for consistency and rollback safety.
• Sequence this work after the in-progress connection pool rewrite (see PR Re-issue session isolation level on TransactionScope re-enlistment (fixes #146) #4335 and related work) to avoid merge conflicts with the base-class changes.

Note: This triage summary is auto-generated by an AI agent. The analysis and suggestions above have not been verified by a human maintainer. Please treat as preliminary guidance only.

Generated by SqlClient Issue Auto-Triage for issue #4582 · 31.6 AIC · ⌖ 9.01 AIC · ⊞ 8.6K · ◷