Skip to content

Commit cd071c6

Browse files
committed
Fixed the SQLBatch token-stream desync where an INFO token sharing a response with a result set hung SqlClient until command timeout.
1 parent 931674f commit cd071c6

5 files changed

Lines changed: 107 additions & 19 deletions

File tree

SqlServerSimulator.Tests.SqlClient/InfoMessageTests.cs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,67 @@ public async Task RaiseError_LowSeverity_ArrivesAsInfoMessage()
5151

5252
Contains("warn", messages);
5353
}
54+
55+
// The go-sqlcmd shakedown (2026-07-14) exposed a token-stream desync when
56+
// an INFO token shares a SQLBatch response with a result set: SqlClient
57+
// hung until command timeout on every mixed shape below (go-mssqldb
58+
// instead silently dropped the message). Real SQL Server gives the
59+
// message-producing statement its own DONE and never emits INFO after
60+
// the final DONE. CommandTimeout is set so a regression fails the test
61+
// instead of stalling the suite.
62+
63+
private async Task<(List<string> Messages, List<int> Values)> DrainBatch(string sql)
64+
{
65+
var simulation = new Simulation();
66+
_ = Wire.ReadAllInProc(simulation, "create table t (id int); insert t values (1)");
67+
await using var listener = await simulation.ListenAsync(0, TestContext.CancellationToken);
68+
await using var connection = await Wire.OpenAsync(listener, TestContext.CancellationToken);
69+
var messages = Subscribe(connection);
70+
71+
await using var command = new SqlCommand(sql, connection) { CommandTimeout = 15 };
72+
var values = new List<int>();
73+
await using (var reader = await command.ExecuteReaderAsync(TestContext.CancellationToken))
74+
{
75+
do
76+
{
77+
while (await reader.ReadAsync(TestContext.CancellationToken))
78+
values.Add(reader.GetInt32(0));
79+
}
80+
while (await reader.NextResultAsync(TestContext.CancellationToken));
81+
}
82+
83+
return (messages, values);
84+
}
85+
86+
[TestMethod]
87+
public async Task Print_ThenSelect_DeliversInfoAndRows()
88+
{
89+
var (messages, values) = await DrainBatch("print 'before select'; select 42");
90+
Contains("before select", messages);
91+
AreEqual(42, values.Single());
92+
}
93+
94+
[TestMethod]
95+
public async Task Select_ThenPrint_DeliversRowsAndInfo()
96+
{
97+
var (messages, values) = await DrainBatch("select 42; print 'after select'");
98+
Contains("after select", messages);
99+
AreEqual(42, values.Single());
100+
}
101+
102+
[TestMethod]
103+
public async Task RaiseError_LowSeverity_ThenSelect_DeliversInfoAndRows()
104+
{
105+
var (messages, values) = await DrainBatch("raiserror('warn', 10, 1); select 42");
106+
Contains("warn", messages);
107+
AreEqual(42, values.Single());
108+
}
109+
110+
[TestMethod]
111+
public async Task Print_BetweenOutcomeStatements_DeliversAll()
112+
{
113+
var (messages, values) = await DrainBatch("insert t values (2); print 'mid'; select id from t order by id");
114+
Contains("mid", messages);
115+
CollectionAssert.AreEqual(new[] { 1, 2 }, values);
116+
}
54117
}

SqlServerSimulator/Network/TdsSession.Rpc.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ private async ValueTask ExecuteRpcMessageAsync(TdsMessage message, TdsTokenWrite
5656
}
5757
catch (SimulatedSqlException ex)
5858
{
59-
this.FlushInfoMessages(writer);
59+
_ = this.FlushInfoMessages(writer);
6060
WriteErrors(writer, ex);
6161
writer.WriteDoneToken(Tds.TokenDoneProc, (ushort)(Tds.DoneError | (moreRequests ? Tds.DoneMore : Tds.DoneFinal)), 0);
6262
}
6363
catch (NotSupportedException ex)
6464
{
65-
this.FlushInfoMessages(writer);
65+
_ = this.FlushInfoMessages(writer);
6666
writer.WriteErrorOrInfo(Tds.TokenError, 50000, 1, 16, $"SqlServerSimulator: {ex.Message}", "SIMULATED", "", 1);
6767
writer.WriteDoneToken(Tds.TokenDoneProc, (ushort)(Tds.DoneError | (moreRequests ? Tds.DoneMore : Tds.DoneFinal)), 0);
6868
}

SqlServerSimulator/Network/TdsSession.cs

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -211,18 +211,18 @@ private async ValueTask ExecuteBatchAsync(TdsMessage message, TdsTokenWriter wri
211211
}
212212
catch (SimulatedSqlException ex)
213213
{
214-
this.FlushInfoMessages(writer);
214+
_ = this.FlushInfoMessages(writer);
215215
WriteErrors(writer, ex);
216216
writer.WriteDone(Tds.DoneError, 0);
217217
}
218218
catch (NotSupportedException ex)
219219
{
220-
this.FlushInfoMessages(writer);
220+
_ = this.FlushInfoMessages(writer);
221221
writer.WriteErrorOrInfo(Tds.TokenError, 50000, 1, 16, $"SqlServerSimulator: {ex.Message}", "SIMULATED", "", 1);
222222
writer.WriteDone(Tds.DoneError, 0);
223223
}
224224

225-
this.FlushInfoMessages(writer);
225+
_ = this.FlushInfoMessages(writer);
226226
var databaseAfter = this.connection.Database;
227227
if (!string.Equals(databaseAfter, databaseBefore, StringComparison.Ordinal))
228228
writer.WriteEnvChange(Tds.EnvDatabase, databaseAfter, databaseBefore);
@@ -241,16 +241,19 @@ private async ValueTask StreamOutcomesAsync(SimulatedDbCommand command, TdsToken
241241
using var outcomes = simulation.CreateResultSetsForCommand(command).GetEnumerator();
242242

243243
var hasOutcome = outcomes.MoveNext();
244-
if (!hasOutcome && !trailingTokensFollow)
245-
{
246-
this.FlushInfoMessages(writer);
247-
writer.WriteDone(Tds.DoneFinal, 0);
248-
}
249-
244+
var anyOutcome = hasOutcome;
250245
while (hasOutcome)
251246
{
252247
var outcome = outcomes.Current;
253-
this.FlushInfoMessages(writer);
248+
// Messages from a preceding no-outcome statement (PRINT,
249+
// severity<=10 RAISERROR): real SQL Server gives that statement
250+
// its own DONE after the INFO tokens. Without it SqlClient's
251+
// token reader stalls on the INFO/COLMETADATA adjacency until
252+
// command timeout (go-sqlcmd shakedown, 2026-07-14). RPC
253+
// responses skip the extra DONEINPROC — their per-statement
254+
// DONEINPROC stream is already well-formed for proc-body PRINT.
255+
if (this.FlushInfoMessages(writer) && !trailingTokensFollow)
256+
writer.WriteDoneToken(doneToken, Tds.DoneMore, 0);
254257
if (outcome is SimulatedQueryResult query)
255258
{
256259
TdsTypeCodec.ValidateSchema(query.Schema);
@@ -267,24 +270,37 @@ private async ValueTask StreamOutcomesAsync(SimulatedDbCommand command, TdsToken
267270
}
268271

269272
hasOutcome = outcomes.MoveNext();
270-
var status = hasOutcome || trailingTokensFollow ? Tds.DoneMore : Tds.DoneFinal;
271-
writer.WriteDoneToken(doneToken, (ushort)(status | Tds.DoneCount), rows);
273+
writer.WriteDoneToken(doneToken, (ushort)(this.OutcomeDoneStatus(hasOutcome, trailingTokensFollow) | Tds.DoneCount), rows);
272274
}
273275
else
274276
{
275277
var affected = outcome.RecordsAffected;
276278
hasOutcome = outcomes.MoveNext();
277-
var status = hasOutcome || trailingTokensFollow ? Tds.DoneMore : Tds.DoneFinal;
279+
var status = this.OutcomeDoneStatus(hasOutcome, trailingTokensFollow);
278280
if (affected >= 0)
279281
status |= Tds.DoneCount;
280282

281283
writer.WriteDoneToken(doneToken, status, Math.Max(affected, 0));
282284
}
283285
}
284286

285-
this.FlushInfoMessages(writer);
287+
// Trailing messages (batch ends in PRINT): INFO may never follow the
288+
// final DONE, so the last outcome's DONE stayed DONE_MORE (see
289+
// OutcomeDoneStatus) and the batch closes with its own final DONE.
290+
var flushedTrailing = this.FlushInfoMessages(writer);
291+
if (!trailingTokensFollow && (flushedTrailing || !anyOutcome))
292+
writer.WriteDoneToken(doneToken, Tds.DoneFinal, 0);
286293
}
287294

295+
/// <summary>
296+
/// DONE status for a completed outcome: more tokens follow when another
297+
/// outcome exists, the response is an RPC (RETURNSTATUS / DONEPROC still
298+
/// come), or queued info messages remain to be written — the trailing-
299+
/// PRINT case, whose INFO must precede the batch's final DONE.
300+
/// </summary>
301+
private ushort OutcomeDoneStatus(bool hasOutcome, bool trailingTokensFollow) =>
302+
hasOutcome || trailingTokensFollow || this.pendingInfoMessages.Count > 0 ? Tds.DoneMore : Tds.DoneFinal;
303+
288304
private void ResetConnection()
289305
{
290306
var previous = this.connection!;
@@ -302,10 +318,17 @@ private void ResetConnection()
302318
this.connection = fresh;
303319
}
304320

305-
private void FlushInfoMessages(TdsTokenWriter writer)
321+
/// <summary>Writes all queued info messages as INFO tokens; true when any were written.</summary>
322+
private bool FlushInfoMessages(TdsTokenWriter writer)
306323
{
324+
var any = false;
307325
while (this.pendingInfoMessages.TryDequeue(out var error))
326+
{
308327
writer.WriteErrorOrInfo(Tds.TokenInfo, error.Number, error.State, error.Class, error.Message, error.Server, error.Procedure, error.LineNumber);
328+
any = true;
329+
}
330+
331+
return any;
309332
}
310333

311334
private static void WriteErrors(TdsTokenWriter writer, SimulatedSqlException exception)

docs/claude/backlog.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ This file is the home for net-new non-function feature proposals too. CLAUDE.md'
2222

2323
The endpoint ships with SQLBatch + RPC + Transaction Manager support and credential enforcement via the `CREATE LOGIN` registry (see [`tds-endpoint.md`](tds-endpoint.md)); EF Core runs over the wire through vanilla `UseSqlServer`. Remaining phases, roughly in value order:
2424

25-
- **Tool shakedown** — point sqlcmd/SSMS/DBeaver at the endpoint and harvest their exotic catalog queries / SET shapes into this backlog. SSMS is the boss fight. Likely to want `sys.server_principals` / `sys.sql_logins` projected over the login registry.
25+
- **Tool shakedown** — point sqlcmd/SSMS/DBeaver at the endpoint and harvest their exotic catalog queries / SET shapes into this backlog. SSMS is the boss fight. Likely to want `sys.server_principals` / `sys.sql_logins` projected over the login registry. **go-sqlcmd leg done (2026-07-14)**, first non-SqlClient client (go-mssqldb): connect/auth/batches/GO files/`:setvar`/`:r`/transactions/output modes all clean; headline find was the INFO-token/result-set DONE desync (fixed — see [`tds-endpoint.md`](tds-endpoint.md) batch-loop section); engine-level harvest landed under Fidelity gaps. SSMS + DBeaver legs remain (GUI, need a host-exposed port).
2626
- **TVP parameters over the wire** (TYPE_INFO 0xF3) — the in-process TVP surface ships; the RPC reader rejects the wire form. Needed for `SqlDbType.Structured` and EF bulk patterns.
2727
- **Wire forms deferred by the codec**: `text`/`ntext`/`image` (textptr ROW form + table-name-bearing COLMETADATA), `hierarchyid`/`geography`/`geometry` (UDT 0xF0), `sql_variant` (no simulator type at all).
2828
- **Smaller fidelity items**: Msg 4060 (not 911) for login to a missing database; mid-stream attention (cancel during a large result requires a concurrent reader); `SqlBulkCopy` (BulkLoadBCP); MARS; TDS 8.0 / `Encrypt=Strict` (ALPN via `SslApplicationProtocol`); user-supplied `X509Certificate2`; off-loopback binding — the last two are add-on-demand public-surface expansions.
@@ -53,6 +53,8 @@ Low priority / niche — simulatable (as placeholder constants or a small model)
5353

5454
Real bugs / limitations against shipped behavior — fixes are concrete work, not design decisions.
5555

56+
- **`SimulatedSqlException` ERROR tokens carry `Server=''` / `Line 0`** — real SQL Server fills the server name and a 1-based line number on every error; the simulator's query errors surface blank/0 both in-process and over the wire (sqlcmd renders `Server , Line 0`). Inconsistently, `NotSupportedException`-derived wire errors *do* carry `Server=SIMULATED, Line 1`. Engine-level: the fix is stamping `SimulatedError.Server` (from `SERVERPROPERTY('ServerName')` = `SIMULATED`) at construction and threading real line numbers from the tokenizer — the latter is the large part. Harvested from the go-sqlcmd shakedown (2026-07-14).
57+
- **String literals typed at container width, not value width**`SELECT 'included'` advertises `varchar(8000)` (and `@@VERSION` `nvarchar(4000)`) in projection schema; real types literals at exact length (`varchar(8)`). Correct data, but clients that size display columns from COLMETADATA (sqlcmd) render absurdly wide output, and `GetSchemaTable`-driven consumers see wrong sizes. Fix lives in literal `SqlType` inference (`Literal` → length-parameterized type), with CONCAT/CASE/set-op width-combination ripple effects — probe real's width algebra before starting. Harvested from the go-sqlcmd shakedown (2026-07-14).
5658
- **Trailing-space MIN/MAX representative** — for a group of values differing only in trailing spaces (sort-equal under SQL Server), MIN/MAX returns a different byte-variant than the live server's scan-order representative. Surfaced by the AdventureWorks crosscheck on synthetic XML data (`vJobCandidateEducation._max_Edu_Loc_CountryRegion`). Needs trailing-space-insensitive compare + SQL Server's unspecified MAX-tie scan-order. See [`collations.md`](collations.md) "byte-exact sort" trailing-space note. **Deferred** — synthetic data, and the representative is unspecified scan-order on the live side.
5759
- **Leaked-connection session cleanup** — a `SimulatedDbConnection` that's never `Dispose`d never reclaims its session state: an open transaction holds its locks and pins the MVCC version store, `##temp` tables linger, session-owned app locks stay held, and the SPID accumulates. Real SqlClient's GC-finalization eventually closes a leaked connection and the server resets the session, so this is a genuine fidelity divergence. **Scope correction (2026-07-11): the fix is bigger than "weaken the registry."** Investigation found *three* global strong-reference cycles that pin exactly the connections that hold session state, so GC can't collect them and a finalizer never fires: (1) `LockResource.Hold.Owner` is a strong `SimulatedDbConnection` (reachable Database → table → lock → hold) — pins any lock- or session-app-lock-holding connection; (2) `HeapTable.OwnerConnection` is strong and `Simulation.GlobalTempTables` holds the table — pins any `##temp` owner; (3) `Database.ActiveSnapshotTxs` holds the transaction, which strongly refs its connection — pins any open-snapshot session. Weakening `Simulation.Connections` alone accomplishes nothing because the resource *is* the pin. A correct fix must break all three cycles — cleanest via a one-way `SessionToken` indirection (resources reference a lightweight token identity; the connection references the token, not vice versa) plus a finalizer that enqueues a **deferred teardown** drained on a normal worker thread (next `CreateDbConnection` / version-store GC) so transaction rollback stays off the finalizer thread. This is a broad, mechanical owner-indirection refactor landing on the most regression-sensitive subsystem (lock manager × GC timing × threading). Payoff is bounded (EF disposes scrupulously; only buggy consumer code leaks), so it's **deliberately deferred** as high-risk / low-frequency. Eventual home: [`locking.md`](locking.md).
5860
- **Workload-harness divergence reporting quirks** (`.vs/workload/Program.cs`, local-only) — the parity report's example line rebuilds parameters from the op seed and can mismatch the actual divergent instance, and divergent instances aren't re-run single-threaded to classify transient-vs-stable. Both made the 2026-07-10 shared-plan-state hunt slower than it needed to be (the fixed bug class itself — instance-bound aggregate/window results, baked TOP/OFFSET counts, frozen RAND, unstamped replay clock — is documented in [`plan-cache.md`](plan-cache.md)'s shared-plan contract section).

docs/claude/tds-endpoint.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Everything lives in `Network/` (internal) except the public `SimulatedNetworkLis
2727

2828
- **SQLBatch** (type 1): ALL_HEADERS skipped via its leading length DWORD; UCS-2 text executed on the session connection. Per result set: COLMETADATA + ROW stream + DONE (`DONE_COUNT` + `DONE_MORE` when more outcomes follow); per non-query statement: DONE with `DONE_COUNT` only when `RecordsAffected >= 0`. Zero outcomes → single final DONE. All tokens are `0xFD DONE` (DONEPROC/DONEINPROC are proc-scoped and RPC isn't shipped).
2929
- **Errors**: `SimulatedSqlException.Errors` map field-for-field onto ERROR tokens (number/state/class/message/server/procedure/line) + DONE with `DONE_ERROR`; the session survives and keeps serving. `NotSupportedException` becomes a synthetic ERROR number 50000 class 16 prefixed `SqlServerSimulator:`.
30-
- **PRINT / low-severity RAISERROR**: the session subscribes to `SimulatedDbConnection.InfoMessage` and drains the queue as INFO tokens between statements and at batch end.
30+
- **PRINT / low-severity RAISERROR**: the session subscribes to `SimulatedDbConnection.InfoMessage` and drains the queue as INFO tokens between statements and at batch end. **Each INFO flush in a SQLBatch response gets its own DONE**, mirroring real per-statement DONEs: info preceding an outcome is followed by `DONE_MORE` count 0 before the outcome's tokens, and trailing info (batch ends in PRINT) forces the last outcome's DONE to `DONE_MORE` with a closing `DONE_FINAL` count 0 after the INFO — an INFO token must never follow the final DONE. Without both, SqlClient's token reader stalls until command timeout on any batch mixing PRINT with a result set, and go-mssqldb silently drops the message (go-sqlcmd shakedown, 2026-07-14; the pre-fix oracle only covered PRINT-without-result-set). RPC responses are unaffected — every DONEINPROC already carries `DONE_MORE`.
3131
- **`USE`**: database change detected by before/after comparison and emitted as ENVCHANGE type 1 (after the DONEs, before flush — SqlClient processes it anywhere pre-EOM).
3232
- **Reset-connection status bit** (pooled-connection recycle): backing connection disposed and recreated on the same database, acked with the empty ENVCHANGE type 18 before the batch's tokens.
3333
- **Attention** (type 6): acked with DONE `DONE_ATTN`. Execution is synchronous per message, so attention is only observed between messages — a cancel never interrupts a running statement server-side, it just gets acked when the stream drains. In-process execution is fast enough that this matches observable SqlClient behavior.

0 commit comments

Comments
 (0)