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

KeepAlive Caching stops working in 2.41 #5671

Closed
johannesherr opened this issue Jun 4, 2024 · 1 comment · Fixed by #5677
Closed

KeepAlive Caching stops working in 2.41 #5671

johannesherr opened this issue Jun 4, 2024 · 1 comment · Fixed by #5677

Comments

@johannesherr
Copy link

johannesherr commented Jun 4, 2024

When using the same client instance and making requests to the same URL the TCP connection should be reused via the Java KeepAliveCache. This worked until version 2.40 and does no longer work in version 2.41-2.43. Now for every request a new TCP connection is created (but still put into the cache), which leads to a large number of established connections.

To reproduce you can run this snippet:

public static void main(String[] args) throws Exception {
    Client client = ClientBuilder.newClient();

    for (int i = 0; i < 2; i++) {
        try (Response response = client.target("https://www.spiegel.de")
                .request()
                .get()) {

            response.readEntity(String.class);
            System.out.println(String.format("response = %s", response));
            Thread.sleep(1000);
        }
    }

    Thread.sleep(Long.MAX_VALUE);
}

With version 2.40 only one connection will be created, from 2.41 on it will be two.

The cause seems to be that when looking for a previous connection in the KeepAliveCache:

/**
 * Check to see if this URL has a cached HttpClient
 */
public HttpClient get(URL url, Object obj) {
    cacheLock.lock();
    try {
        KeepAliveKey key = new KeepAliveKey(url, obj);
        ClientVector v = super.get(key);
        if (v == null) { // nothing in cache yet
            return null;
        }
        return v.get();
    } finally {
        cacheLock.unlock();
    }
}

The second parameter obj is a sun.security.ssl.SSLSocketFactoryImpl instance. It is part of the cache key. So only if the same instance is used for a lookup a previous connection will be found. This is the case for older versions (I added an Intellij breakpoint to print the arguments):

get https://www.spiegel.de sun.security.ssl.SSLSocketFactoryImpl@5bfa8cc5
get https://www.spiegel.de sun.security.ssl.SSLSocketFactoryImpl@5bfa8cc5

But not for new versions:

get https://www.spiegel.de sun.security.ssl.SSLSocketFactoryImpl@13e3c1c7
get https://www.spiegel.de sun.security.ssl.SSLSocketFactoryImpl@20312893

Since different SSLSocketFactoryImpl instances are used, the cache lookup always fail and always a new connection is created. So we see the number of established connections explode in production after the update.

I assume some change lead to new instances of SSLSocketFactoryImpl being created for new requests, even though the client is reused.

(The Java version is irrelevant. I tested with 17 and 21.)

@jansupol
Copy link
Contributor

The cause is that

        SSLContext ctx = ...;
        System.out.println(ctx.getSocketFactory());
        System.out.println(ctx.getSocketFactory());

gets 2 factories

sun.security.ssl.SSLSocketFactoryImpl@35cabb2a
sun.security.ssl.SSLSocketFactoryImpl@7e07db1f

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 a pull request may close this issue.

2 participants