Skip to content

Memoize reopened pools while holding the mutex - #313

Merged
byroot merged 1 commit into
redis-rb:masterfrom
OskarEichler:codex/recheck-pool-reopening
Aug 27, 2026
Merged

Memoize reopened pools while holding the mutex#313
byroot merged 1 commit into
redis-rb:masterfrom
OskarEichler:codex/recheck-pool-reopening

Conversation

@OskarEichler

Copy link
Copy Markdown
Contributor

Summary

  • Recheck and assign @pool inside the reopening mutex.
  • Keep concurrent callers after close on a single pool, preserving its configured capacity.

Reproduction

Create RedisClient.config.new_pool(size: 1), close it, hold its @mutex, and start two threads invoking its private pool accessor. Wait until both threads block on that mutex, then release it and join both with deadlines. Shut down both returned pools during cleanup.

Before: the two callers receive different ConnectionPool objects.
After: both receive the same object.

The outer ||= evaluates the nil value before either caller acquires the mutex. Serializing only construction still creates a second pool and overwrites the first one. Memoizing inside the lock avoids that race without adding locking to the normal non-nil path.

Verification

  • Ruby 4.0.6 / redis-client 0.30.1 / connection_pool 3.0.2.
  • Deterministic bounded two-thread before/after reproduction.
  • Existing circuit-breaker and pooled suites: 65 cases / 1,210 assertions, zero failures, errors, retries or skips on both baseline and patch.
  • Targeted RuboCop, Ruby syntax and whitespace checks pass.

Compatibility and limitations

No intended breaking changes, API or dependency changes. Reopening after close remains supported. This does not redefine concurrent shutdown/check-out semantics. Other Ruby implementations and upstream CI are not yet locally verified. No test files added or modified, per the originating project's policy; the focused reproduction was run outside the repository.

Prepared with Codex assistance; findings and checks above were reproduced locally.

@byroot
byroot merged commit d17fe10 into redis-rb:master Aug 27, 2026
15 checks passed
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