SQLite: do not report success when a later statement or a busy commit fails

[HuzaifaChaudary]

Fixes #32740

there are two places where the error is lost , and the reporter's program goes through both .

a statement after a query

ExecuteNonQuery is ExecuteReader() then Dispose() . the reader stops at the first statement that returns columns , and Dispose runs the rest inside a catch { } . so this returns with no error , and because the COMMIT never runs the row for 2 is rolled back :

BEGIN; SELECT 1; INSERT INTO Data VALUES (2); INSERT INTO Data VALUES (1); COMMIT;

ExecuteScalar has the same shape . both now run the remaining statements through NextResult() before the reader is disposed , so the error comes out . Dispose itself is left alone .

a busy commit after RETURNING

that alone did not fix the reporter's program , it still lost a row in 2 runs of 3 with no error . the insert has RETURNING , so it is left unfinished and it commits when NextResult resets it in DisposeWithBusyHandling . when that commit is busy the loop resets again , and sqlite3_reset only reports the error once . i logged it : the reset codes were 5,0 , sqlite3_get_autocommit was 1 afterwards , and the row was not there .

this is not only the reporter's shape . on main , ExecuteReader on an INSERT ... RETURNING , Read() , then NextResult() while another connection holds a read lock returns false after about half a second with no error , and the row is not in the table .

so instead of resetting again , it steps the statement to the end while it is busy , which retries the commit , and then resets once . that keeps the wait #36657 added , and when the timeout runs out the busy error is thrown instead of lost . it only applies to a write that is still unfinished , and not to the plain Dispose() path .

with both changes the reporter's program kept all 1000 rows in 20 runs of 20 . the commit was busy 5 times across those runs and each time it waited and committed .

tests

the busy tests hold a reader open on one connection , so the insert on the other one can run but cannot commit . no race , and each one uses its own file .

• ExecuteNonQuery_throws_when_statement_after_query_fails , ExecuteNonQuery_does_not_lose_rows_silently_when_statement_after_query_fails , ExecuteScalar_throws_when_statement_after_query_fails : no exception on main , fixed by the SqliteCommand change
• NextResult_throws_instead_of_losing_the_write_when_commit_stays_busy_with_returning , NextResult_waits_for_the_commit_when_busy_with_returning : no exception and the row gone on main , still failing with only the SqliteCommand change , fixed by the SqliteDataRecord change
• ExecuteNonQuery_waits_for_the_commit_when_busy_with_returning : on main this throws busy at once instead of waiting . not data loss , but it is a change in behaviour , ExecuteNonQuery now waits up to the command timeout here like it does for other busy statements

Microsoft.Data.Sqlite.sqlite3.Tests 710 pass , sqlite3mc 711 pass , EFCore.Sqlite.Tests 896 pass , EFCore.Sqlite.FunctionalTests 38042 pass . the 177 functional failures here are all mod_spatialite.dylib not being installed on this mac .

because of that last point , ExecuteNonQuery_throws_when_busy_with_returning and ExecuteScalar_throws_when_busy_with_returning , which are skipped under #35585 , would now fail if they were turned back on , since they expect a throw while the other connection holds the lock for 5 seconds . i left them as they are . if you would rather take the two halves separately , the busy part is only the SqliteDataRecord change and the two NextResult tests .

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01VfnFMKDUZWENKk7Vpe6GS7

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

🟡 Changes recommended

The busy-wait tests are race-prone, and existing skipped tests retain obsolete expectations.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[AndriySvyryd]

Thanks for your contribution!