Tighten Connection.initializeFromClientConfig() visibility to package-private#4548
Merged
Merged
Conversation
Connection.Builder.build() already initializes the connection, so FailFastConnectionFactory.makeObject() was running initializeFromClientConfig() a second time. This re-registered PUBSUB_CONSUMER and re-ran the HELLO/CLIENT handshake on every MultiDbClient database connection. Split Builder.build() into buildUninitialized() + init so the existing factoryTrackedObjects set can wrap the (now sole) init — preserving the ability for forceDisconnect() to interrupt a connection mid-handshake.
🛡️ Jit Security Scan Results✅ No security findings were detected in this PR
Security scan by Jit
|
Follow-up to #4547: no out-of-package caller remains.
Base automatically changed from
topic/ggivo/fix-tracking-pool-double-init
to
master
June 8, 2026 07:37
…-connection-init-visibility # Conflicts: # src/test/java/redis/clients/jedis/mcf/TrackingConnectionPoolInitTest.java
…mock-construction
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Tightens
Connection.initializeFromClientConfig()(no-arg) frompublicto package-private. Follow-up to #4547.Why
After #4547, this method has no out-of-package caller — and importantly, no external caller could legitimately use it:
Connection.builder().build()new Connection(socketFactory, clientConfig)new Connection(HostAndPort, JedisClientConfig)new Connection()/(host,port)/(HostAndPort)etc.clientConfig == null→ callinginitializeFromClientConfig()would NPEConnection.Builder.buildUninitialized()There is no public construction path that produces an uninitialized-but-configured
Connection. Keeping the method public means: an external caller can only reach it on a fully-initialized connection (where it would re-run the handshake — the exact bug #4547 fixed).The
protectedoverloadinitializeFromClientConfig(JedisClientConfig)stays as-is —CacheConnectionoverrides it.Breaking change
This is a binary-incompatible change.
Note
Medium Risk
Binary-incompatible API change for any external caller of the no-arg initializer; runtime behavior for supported construction paths should be unchanged.
Overview
Narrows the API surface by making
Connection.initializeFromClientConfig()(no-arg) package-private instead of public, with Javadoc stating it is an internal lifecycle step forBuilder#build()and poolConnectionFactoryonly. The protectedinitializeFromClientConfig(JedisClientConfig)overload is unchanged for subclasses likeCacheConnection.Strengthens the regression test for
TrackingConnectionPooldouble-init: drops Mockito construction/verify ofinitializeFromClientConfig()and instead borrows a real connection againstTcpMockServer, assertingHELLOis sent exactly once per pooled connection.Reviewed by Cursor Bugbot for commit 8c607e2. Bugbot is set up for automated code reviews on this repo. Configure here.