You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Restore the wait-handle pool's previous behavior: admitted requests can finish on a retired pool, and their connections are destroyed when returned. Remove the post-wait shutdown short-circuit and synthetic semaphore releases together. Keep the cached error and its expiry timer available to remaining waiters.
Leave clearing to the factory's explicit-clear or deferred-pruning path rather than calling Clear() inside wait-handle Shutdown(). Shutdown can run under the pool-group lock before the pool is queued for release. An inline code comment records why reclamation and disposal remain outside that path.
Remove the earlier retry exception and SqlConnection.OpenAsync retry changes. New requests still use the factory's existing replacement-pool routing. Channel-pool behavior is unchanged.
Deterministic coverage
Hold the first physical login behind a gate, confirm a second request is pending on the same pool, clear the pool, then release the login. Exercise Open and OpenAsync with both ClearPool and ClearAllPools against the simulated TDS server.
Assert exact login counts, retired-pool ownership, disposal on close, and replacement-pool selection.
Gate mocked physical creation to force cancellation before a pending acquisition completes and verify its connection is returned and destroyed.
Advance a fake clock to verify blocking-period expiry after shutdown.
Verify shutdown leaves idle connections untouched until the subsequent Clear() drains them.
Removed the repeated-open stress workload and wall-clock timeout threshold test. No test relies on repeated attempts to encounter the race. Bounded waits only guard against hangs.
Validation
All eight added acquisition/error-state regression cases failed when the removed shutdown short-circuit and error-state disposal were temporarily restored. The async public-API cases reproduced the reported pool-timeout exception.
The updated deferred-clear regression failed with Clear() still inside shutdown and passed after its removal.
With the latest changes, 365 pooling tests passed on each of net8.0 and net9.0.
Validation used installed SDK 10.0.300 from outside the checkout because pinned SDK 10.0.401 is unavailable. The deterministic public-API tests use the in-process TDS server; the customer's SQL Server workload remains unverified locally.
Deferred behavior
The late-return/clear race and saturated retired-pool waiter behavior identified in review also predate #4302 and remain outside this compatibility restoration. Factory-owned async retry is tracked in #4720. This PR does not guarantee that every admitted request succeeds after clearing.
Checklist
Tests added or updated
Public API changes documented: N/A, no API changes
Verified against customer repro (SQL Server unavailable locally)
Ensure no breaking changes introduced: restore pre-hardening wait-handle behavior
Route pending async opens on a cleared wait-handle pool back through the connection factory so they can complete from the replacement pool instead of surfacing a misleading pool timeout.
Add a regression test that parks an async pending open, clears the pool group, and verifies completion from a replacement pool.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove shutdown interruption and retry routing. Let admitted requests finish on the retired pool and dispose their connections on return. Preserve error expiry for remaining waiters.
Cover sync and async clearing, cancellation, timeouts, and error expiry. Add a SQL Server regression workload for dotnet#4714.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove the concurrent stress workload and elapsed-time assertions. Gate physical creation and assert exact request ordering and creation counts around pool shutdown.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The repository's test documentation policy requires <param> and <returns> tags for helper methods where applicable (.github/instructions/testing.instructions.md:181-185). This new helper should document its three parameters and returned acquisition task.
This issue also appears on line 249 of the same file.
Document helper connection, async, and return values
The repository's test documentation policy requires <param> and <returns> documentation for helper methods where applicable (.github/instructions/testing.instructions.md:181-185). Please document connection, async, and the returned task.
The test guide requires helper methods to document parameters and return values (.github/instructions/testing.instructions.md:181-185). Please document the pool, owner, optional completion source, and the acquisition task returned here.
This issue also appears on line 249 of the same file.
The test guide requires helper methods to document their parameters and return values (.github/instructions/testing.instructions.md:181-185). This new helper only has a summary, so its sync/async selector and returned task contract are undocumented.
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
Hotfix 7.1.1PRs targeting main that should be backported to release/7.1 for 7.1.1.
4 participants
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.
Summary
Fixes #4714.
Restore the wait-handle pool's previous behavior: admitted requests can finish on a retired pool, and their connections are destroyed when returned. Remove the post-wait shutdown short-circuit and synthetic semaphore releases together. Keep the cached error and its expiry timer available to remaining waiters.
Leave clearing to the factory's explicit-clear or deferred-pruning path rather than calling
Clear()inside wait-handleShutdown(). Shutdown can run under the pool-group lock before the pool is queued for release. An inline code comment records why reclamation and disposal remain outside that path.Remove the earlier retry exception and
SqlConnection.OpenAsyncretry changes. New requests still use the factory's existing replacement-pool routing. Channel-pool behavior is unchanged.Deterministic coverage
OpenandOpenAsyncwith bothClearPoolandClearAllPoolsagainst the simulated TDS server.Clear()drains them.Removed the repeated-open stress workload and wall-clock timeout threshold test. No test relies on repeated attempts to encounter the race. Bounded waits only guard against hangs.
Validation
Clear()still inside shutdown and passed after its removal.Validation used installed SDK 10.0.300 from outside the checkout because pinned SDK 10.0.401 is unavailable. The deterministic public-API tests use the in-process TDS server; the customer's SQL Server workload remains unverified locally.
Deferred behavior
The late-return/clear race and saturated retired-pool waiter behavior identified in review also predate #4302 and remain outside this compatibility restoration. Factory-owned async retry is tracked in #4720. This PR does not guarantee that every admitted request succeeds after clearing.
Checklist