HTTP Connection Pooling May Not Work Properly with Explicitly Configured Client SSL

[kzander91]

Expected Behavior

Identically configured HttpClient instances should share HTTP connection pool instances.

Actual Behavior

A new pool is created for every connection acquiry, because Netty's SslContext does not produce stable hash codes. This causes HttpClientConfig#channelHash to be unstable as well, in turn causing PooledConnectionProvider#acquire to always create a new pool instance for every request.

In my application this eventually leads to OOME crashes because the pools don't seem to be disposed within a reasonable amount of time.

Steps to Reproduce

1. Create multiple, identically configured HttpClient and SslContext instances.
2. Call some HTTPS endpoint repeatedly.
3. Observe logs, for each request, we get Creating a new [http] client pool [PoolFactory{evictionInterval=PT0S, leasingStrategy=fifo, maxConnections=500, maxIdleTime=-1, maxLifeTime=-1, metricsEnabled=false, pendingAcquireMaxCount=1000, pendingAcquireTimeout=45000}] for [httpbin.org/<unresolved>:443].

Reproducer (unzip and run mvnw spring-boot:run): repro.zip
Code:

@SpringBootApplication
public class ReproApplication {

    public static void main(String[] args) {
        try (var ignored = SpringApplication.run(ReproApplication.class, args)) {
            Mono.defer(() -> HttpClient.create()
                            .secure(ssl -> ssl.sslContext(createSslContext()))
                            .get()
                            .uri("https://httpbin.org/get")
                            .response())
                    .repeat(10)
                    .blockLast();
        }
    }

    private static SslContext createSslContext() {
        try {
            KeyStore cacerts = KeyStore.getInstance("PKCS12");
            try (var in = Files.newInputStream(Path.of(System.getProperty("java.home"))
                    .resolve("lib/security/cacerts"))) {
                cacerts.load(in, "changeit".toCharArray());
            }

            TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
            tmf.init(cacerts);
            return SslContextBuilder.forClient()
                    .trustManager(tmf)
                    .build();
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

}

The issue doesn't arise if I create and reuse a single SslContext instance by instantiating it outside the defer().

This reproducer is a bit of a contrived example, please see spring-projects/spring-framework#33093 for how I noticed this behaviour in my real-world application.

Possible Solution

This would most likely be fixed if Netty's SslContext implemented hashCode(), but I was unsure if this is actually a "bug" on Netty's side...
Additionally, implementing a proper hashCode() method may not be that simple or even possible, because it wraps a javax.net.ssl.SSLContext which doesn't implement that either.
Thus, one could argue that there isn't anything to "fix" on Netty's side, in turn suggesting a bug on Reactor's side in including the SslContext in the channelHash().

Your Environment

• Reactor version(s) used: Reactor Netty 1.1.20
• Other relevant libraries versions: Netty 4.1.111.Final
• JVM version (java -version):

openjdk version "21.0.3" 2024-04-16 LTS
OpenJDK Runtime Environment Temurin-21.0.3+9 (build 21.0.3+9-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.3+9 (build 21.0.3+9-LTS, mixed mode, sharing)

[violetagg]

@kzander91 From the sample that you provided, it seems that you recreate the client and the ssl context every time. I would say that this is not a very good pattern and easily can lead to memory problems (see #1054).

By design if you do not share the SslContext, a new pool will be created. We guarantee that all connections in one connection pool are with absolutely equal configurations.

[kzander91]

@violetagg Yes, the memory problems is exactly why I noticed this issue. Recreating the ssl context every time is not intended and a bug in Spring, see spring-projects/spring-framework#33093.

Thanks for the feedback, so not sharing pools across different SslContext instances is by design then, ultimately that's fine by me (I suppose it must be this way to ensure that separate SSL configurations for the same target domain get dedicated pools).

However, could it make sense to limit the number of pools (configurable of course) and maybe log warnings if additional pools are being created? Something like this would have saved me a lot of time when debugging this issue.

[violetagg]

However, could it make sense to limit the number of pools (configurable of course) and maybe log warnings if additional pools are being created? Something like this would have saved me a lot of time when debugging this issue.

@kzander91 Would you like to provide a PR?

[violetagg]

You can also configure the pool to be disposed if not used for a specific time - disposeInactivePoolsInBackground (see all available configurations here https://projectreactor.io/docs/netty/release/reference/index.html#_connection_pool_2)

[kzander91]

Would you like to provide a PR?

No, I don't feel comfortable enough with the code base to attempt an implementation of that myself.

You can also configure the pool to be disposed if not used for a specific time

Thank you for the hint, I'll keep that in mind as a possible workaround.

[violetagg]

Let me close this one. I opened a new issue for limiting the number of the connection pools #3318