Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport - Fix a potential connection pool leak. #140

Open
wants to merge 1 commit into
base: release-19.0-github
Choose a base branch
from

Conversation

arthurschreiber
Copy link
Member

Description

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@Copilot Copilot bot review requested due to automatic review settings February 17, 2025 12:25

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR backports a fix for a potential connection pool leak by ensuring that the stack node’s "next" field is cleared when it is popped.

  • Adds a test in stack_test.go to verify that the popped node's "next" pointer is nil.
  • Updates stack.go so that after a successful pop, the "next" pointer of the popped node is set to nil to prevent connection leaks.

Changes

File Description
go/pools/smartconnpool/stack_test.go Adds a test case to verify that the popped node's "next" pointer is nil.
go/pools/smartconnpool/stack.go Clears the "next" pointer after a successful pop to prevent leaks.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

go/pools/smartconnpool/stack_test.go:13

  • Consider adding a separate test case for the scenario where Pop() is called on an empty stack to validate that it correctly handles the edge case without errors.
func TestStackPop(t *testing.T) {

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

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.

1 participant