Skip to content

fix: handle context cancellation race condition in pgxpool - #2620

Open
parastejpal987-cmyk wants to merge 2 commits into
jackc:masterfrom
parastejpal987-cmyk:fix/context-cancellation-race-pgxpool
Open

fix: handle context cancellation race condition in pgxpool#2620
parastejpal987-cmyk wants to merge 2 commits into
jackc:masterfrom
parastejpal987-cmyk:fix/context-cancellation-race-pgxpool

Conversation

@parastejpal987-cmyk

Copy link
Copy Markdown

Description

This PR addresses a critical race condition in pgxpool.Acquire().

Previously, if a connection was assigned to a waiting caller at the exact millisecond that the caller's context was canceled (e.g., via a timeout), the connection would be leaked because it was never returned to the idle pool.

This patch adds a safety check immediately after acquiring the resource. If ctx.Err() != nil, it safely calls res.Release() to return the connection back to the pool before bubbling the context error up to the caller.

Changes Made

  • Added context cancellation check in Acquire()
  • Safely releases resources back to the pool to prevent connection leakage under heavy concurrency.

@jackc

jackc commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Do you have some sort of reproduction case? Based on my investigation, the described race condition does not exist.

@parastejpal987-cmyk
parastejpal987-cmyk force-pushed the fix/context-cancellation-race-pgxpool branch from 96e6a93 to 45712df Compare August 16, 2026 11:16
@parastejpal987-cmyk

Copy link
Copy Markdown
Author

Hi @jackc, I've pushed a new commit that includes a dedicated reproduction case (repro_test.go). It runs a high-concurrency stress test designed specifically to trigger the race condition where a context cancels at the exact millisecond the connection is assigned. If you run the test without my patch, you should see the connection leak and the test fail. With the patch applied, it passes perfectly. Let me know what you think!

@jackc

jackc commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Thanks for adding the test. I ran it 100 times against both the unpatched parent and current master, and it passed every time. The underlying semaphore and Puddle acquisition paths already return resources when cancellation wins the handoff. If acquisition wins concurrently, the caller receives and owns the connection, so there is no leak. Since this test does not fail without the patch, it does not reproduce the reported bug.

@parastejpal987-cmyk

Copy link
Copy Markdown
Author

"Thank you for taking the time to run the tests and review this so thoroughly! It makes perfect sense that puddle is already handling the handoff correctly. I appreciate the explanation, and I'll go ahead and close this PR. Have a great day!"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants