Repository navigation
Conversation
…nses are outstanding Fixes npgsql#6662
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Immediate cancellation does not interrupt a synchronous read blocked on a prepended response.
Review effort: Balanced
Findings: 1
What changed in this PR
Bounds deferred cancellation while prepended responses remain unread.
Changes:
- Arms Cancellation Timeout for deferred cancellation.
- Prevents PostgreSQL cancellation of prepended queries.
- Adds an asynchronous unresponsive-server regression test.
| File | Description |
|---|---|
NpgsqlConnector.cs |
Implements deferred cancellation deadlines. |
NpgsqlReadBuffer.cs |
Suppresses unsafe PostgreSQL cancellation. |
CommandTests.cs |
Tests asynchronous deferred cancellation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ion Timeout -1 A zero Socket.ReceiveTimeout disables the timeout, so synchronous reads after the deferred cancellation were unbounded. Add a synchronous prepended-query cancellation test.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #6662
When the user's token fires while a prepended query (
DISCARD ALLon a pooled connection) is still unanswered,PerformImmediateUserCancellationdefers the cancellation and returns without arming a deadline. If the server never answers, the token has no effect until Command Timeout.What this changes:
Socket.ReceiveTimeoutdisables the timeout for sync reads. With 0 nothing changes.OperationCanceledExceptioninstead.PerformDelayedUserCancellationruns as before and re-arms its own Cancellation Timeout. Nothing blocks, so Npgsql 7.0.2 blocks threads NpgsqlConnector.PerformUserCancellation causing connection pool exhaustion #5032 does not come back.Limitation: a synchronous receive that is already blocked isn't interrupted, because a
ReceiveTimeoutchange doesn't affect it. That read still ends at Command Timeout. It then breaks without sending a cancel request, and the sync reads after it are bounded by Cancellation Timeout. Main has the same limit for sync hard cancellation of non-prepended commands. Interrupting the blocked receive is #5070.How I tested it:
CommandTests.Cancel_async_with_prepended_query_server_unresponsive(PgPostmasterMock, Command Timeout=30, Cancellation Timeout=1000). It fails on main after 31 s and passes with this change in about 1 s. It also checks that no cancel request reaches the server.CommandTests.Cancel_sync_with_prepended_query_server_unresponsive(1000, -1): syncCancel(). The server sends part of theDISCARD ALLresponse and then stalls. It takes 30–31 s on main and about 1 s with this change. It also checks that no cancel request is sent.Batched_small_then_big_statements_do_not_deadlock_in_sync_iohangs in my local setup (Windows + Docker), and it hangs on unmodified main too, so I excluded it.