Skip to content

fix(pool): fix wantConnQueue zombie elements and add comprehensive test coverage#3680

Merged
ndyakov merged 3 commits into
redis:masterfrom
cyningsun:bugfix_zombie_elements_cleanup
Jan 24, 2026
Merged

fix(pool): fix wantConnQueue zombie elements and add comprehensive test coverage#3680
ndyakov merged 3 commits into
redis:masterfrom
cyningsun:bugfix_zombie_elements_cleanup

Conversation

@cyningsun

Copy link
Copy Markdown
Contributor

Summary

Fixes zombie wantConn elements accumulation in wantConnQueue and adds comprehensive test coverage.

This PR fixes #3678

Changes

  • Fix resource cleanup: Properly remove wantConn from queue in panic/failure scenarios
  • Optimize concurrency: Replace Mutex with RWMutex in wantConn for better read performance
  • Enhance existing tests: Add dialsQueue cleanup assertions to 8 tests
  • Add zombie cleanup tests: 4 new test cases covering:
    • High concurrency with continuous dial failures (1000 requests)
    • Request timeout + dial failure scenario
    • Intermittent failures over 5 seconds
    • Queue upper bound enforcement

@jit-ci

jit-ci Bot commented Jan 20, 2026

Copy link
Copy Markdown

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

@ndyakov

ndyakov commented Jan 20, 2026

Copy link
Copy Markdown
Member

@cyningsun thank you, i took a brief look and it looks promising, will do a proper review in the next couple of days.

@cyningsun

Copy link
Copy Markdown
Contributor Author

@ndyakov, could you please take a first look when you have a moment?

Once you think it’s functionally sound, we can then ask @jseparator to help verify if it also addresses the memory leak issue they reported.

@ndyakov ndyakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left one question, other than that the PR looks good.

Comment thread internal/pool/want_conn.go

@ndyakov ndyakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@cyningsun looks good, pinged @jseparator on the issue to verify if this solves the reported issue for him.

@ndyakov ndyakov added the bug label Jan 24, 2026
@ndyakov ndyakov merged commit a44932b into redis:master Jan 24, 2026
31 checks passed
ndyakov added a commit that referenced this pull request Jan 24, 2026
…st coverage (#3680)

* discard zombie elements in wantConnQueue

* fix lint

---------

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
ndyakov added a commit that referenced this pull request Jan 24, 2026
…st coverage (#3680)

* discard zombie elements in wantConnQueue

* fix lint

---------

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
ndyakov added a commit that referenced this pull request Jan 24, 2026
…st coverage (#3680)

* discard zombie elements in wantConnQueue

* fix lint

---------

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
mwhooker pushed a commit to mwhooker/go-redis that referenced this pull request Feb 9, 2026
…st coverage (redis#3680)

* discard zombie elements in wantConnQueue

* fix lint

---------

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak in v9.17.2 with high concurrency - massive queuedNewConn and context object accumulation

2 participants