Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025

When context is canceled while a session is being created asynchronously, the session was added to the pool index but never properly cleaned up, causing the pool to grow beyond configured limits.

Root Cause

In makeAsyncCreateItemFunc, the background goroutine adds newly created items to p.index immediately after creation. When the caller's context is canceled before receiving the item:

  1. The item remains in p.index
  2. The goroutine attempts putItem() on an item the caller never received
  3. Pool metrics show items in index but createInProgress is decremented, allowing limit violations

Changes

  • internal/pool/pool.go: In the <-done case, remove item from index and close it directly instead of calling putItem()
  • internal/pool/pool_test.go: Add ContextCanceledDuringCreate test covering both race conditions:
    • Item created before context check (returned to caller)
    • Context canceled before item creation (removed and closed)
case <-done:
    if newItem == nil {
        return
    }
    // Remove from index and close directly
    p.mu.WithLock(func() {
        delete(p.index, newItem)
    })
    closeCtx := createCtx
    if t := p.config.closeTimeout; t > 0 {
        var cancel context.CancelFunc
        closeCtx, cancel = context.WithTimeout(closeCtx, t)
        defer cancel()
    }
    p.config.closeItemFunc(closeCtx, newItem)
Original prompt

This section details on the original issue you should resolve

<issue_title>bug: Session leak. Growth beyond the configured limits in the pool.</issue_title>
<issue_description># Bug Report

v3.118.1

Environment

go1.25.4 linux/amd64

Current behavior:

If the context is canceled early, the created session is not counted in the limits and is not closed on the server.

Expected behavior:

The total number of sessions does not exceed the specified limits.

Related code:

Example playback repository

Other information:

Screencast demo
</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits December 16, 2025 16:37
When context is canceled while a session is being created, the session
was being added to the pool index but then attempted to be put back
via putItem(). This caused issues because:
1. The session would be in the index but not properly tracked
2. In some cases it could exceed pool limits

The fix removes the session from the index and closes it directly
when the caller has canceled, rather than trying to put it back
in the pool.

Added test to validate the fix handles both race conditions:
- Item created before context check (item returned to caller)
- Context canceled before item creation (item closed, not added to pool)

Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com>
Use separate closeCtx variable instead of reassigning createCtx
to avoid potential resource leaks from not calling the previous
context's cancel function.

Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix session leak when context is canceled early Fix session leak when context canceled during async creation Dec 16, 2025
Copilot AI requested a review from asmyasnikov December 16, 2025 16:47
@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.14%. Comparing base (3c5460f) to head (64885fd).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1982   +/-   ##
=======================================
  Coverage   74.14%   74.14%           
=======================================
  Files         395      395           
  Lines       34598    34606    +8     
=======================================
+ Hits        25654    25660    +6     
+ Misses       7825     7820    -5     
- Partials     1119     1126    +7     
Flag Coverage Δ
experiment 73.82% <100.00%> (-0.04%) ⬇️
go-1.21.x 72.99% <100.00%> (-0.06%) ⬇️
go-1.25.x 74.14% <100.00%> (+<0.01%) ⬆️
integration 55.07% <0.00%> (-0.33%) ⬇️
macOS 47.77% <100.00%> (-0.02%) ⬇️
ubuntu 74.14% <100.00%> (+<0.01%) ⬆️
unit 47.79% <100.00%> (+<0.01%) ⬆️
windows 47.77% <100.00%> (-0.02%) ⬇️
ydb-24.4 54.25% <0.00%> (-0.30%) ⬇️
ydb-25.2 54.98% <0.00%> (-0.34%) ⬇️
ydb-latest 54.66% <0.00%> (-0.31%) ⬇️
ydb-nightly 73.82% <100.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

summary

Inferred base version: v3.123.1
Suggested version: v3.123.2

@polRk polRk added the SLO label Dec 16, 2025
@github-actions github-actions bot removed the SLO label Dec 16, 2025
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.

bug: Session leak. Growth beyond the configured limits in the pool.

4 participants