Memory leak when using Jersey with HK2

[albanf]

When using Jersey and Jackson to deserialize XML, any change to the configuration of the WebTarget causes a memory leak since hk2 3.0.4 (and up to 3.1.1).

I suspect this is caused by this change in HK2: eclipse-ee4j/glassfish-hk2@9e5b5f5 (FISH-6432 Applications Take Longer To Deploy on JDK 17) since the same occurs with the patched Payara version 3.0.1.payara-p2 (payara/patched-src-hk2@3.0.1.payara-p1...3.0.1.payara-p2).

Not sure if this is a regression in HK2 or something that Jersey is doing incorrectly.

I've tested with Jersey version 3.0.0 to 3.1.9, and the behavior is the same.

See the attached example project.
jersey-memory-leak.zip

Also posted on HK2: eclipse-ee4j/glassfish-hk2#1126

[jbescos]

I am analyzing the heap dump of that example. What instances are leaking?. There are 0 instances of org.example.jersey3.memoryleak.MyDTO.

[albanf]

Hi @jbescos, look at class org.glassfish.client.ClientRuntime, you should expect 0 instances, but you see 100 instances.

[jbescos]

One note, in your example you have:

        try (Client client = ClientBuilder.newClient()) {
            ...

            Runtime.getRuntime().gc();
            System.out.println("Ready for heap dump");
            Thread.sleep(20000);
            System.exit(0);
        }

That cannot release the clients, because it is inside the try block (#close was not invoked). It has to be in this way:

        try (Client client = ClientBuilder.newClient()) {
            ...
        }
        Runtime.getRuntime().gc();
        System.out.println("Ready for heap dump");
        Thread.sleep(20000);
        System.exit(0);

[jbescos]

After modifying your example, I don't find org.glassfish.client.ClientRuntime instances. Could you check it too, please?.

[albanf]

If you consider a long running application, where a Client is instantiated then reused for many requests over a long period, then the code in my example is representative and shows that over time, the application will run out of memory.

[jbescos]

Right, but I am not sure this is designed in that way.

From what I see, ClientRuntime is created for each request and in can be released in 2 ways:
1- Client#close
2- If there are no strong references to it (this case it seems it is not happening).

However, the point 2 is not good to happen because ClientRuntime#close is supposed to close the connector and it will be executed from GC threads. Tthis is not a good practice because it ignores exceptions, delays the GC, etc. For the default case of org.glassfish.jersey.client.internal.HttpUrlConnector it is a no-op, so it is not really a big issue.

Maybe I am wrong, but it looks it is an issue in the design. ClientRuntime should be discarded from memory after every request, and it should not have any onClose action. Connections should be released after Client#close or errors, limit of opened connections, etc.

[jbescos]

It seems this was introduced here: #4508

If I revert it, there are 0 instances of ClientRuntime

[jansupol]

This looks like a bug in Jersey. Removing HK2 by adding

        <dependency>
            <groupId>org.glassfish.jersey.incubator</groupId>
            <artifactId>jersey-injectless-client</artifactId>
            <version>${jersey.version}</version>
        </dependency>

leaves the 100 ClientRuntimes as well.
I wonder what prevents the ClientRuntime from being GCed.

[jbescos]

This looks like a bug in Jersey. Removing HK2 by adding

        <dependency>
            <groupId>org.glassfish.jersey.incubator</groupId>
            <artifactId>jersey-injectless-client</artifactId>
            <version>${jersey.version}</version>
        </dependency>

leaves the 100 ClientRuntimes as well. I wonder what prevents the ClientRuntime from being GCed.

It is this:

jersey/core-client/src/main/java/org/glassfish/jersey/client/ClientMessageBodyFactory.java

Line 62 in 382f69e

private ClientRuntime clientRuntime;

There are also 100 instances of ClientMessageBodyFactory.MessageBodyWorkersConfigurator`.

The problem is that if we remove that, we will have again this issue #4507

I think the proper fix is that ClientRuntime should not contain closable resources, so it can be GCed without any additional work. Those resources should be passed to the JerseyClient, that is closable and the user must close it. I am not sure whether this is possible right now.

Probably it is easier fix is to keep the reference to ClientRuntime instance in the same scope of life than the input/output streams. The problem is that resources will be closed by the GC threads when it invokes #finalize and that could be a performance issue. We can sort this out with PhantomReferences in our internal thread executors.

[jansupol]

The reproducer contains:

                final WebTarget webTarget = client
                        .target(wireMockServer.baseUrl());
                        .property("test", "test");

Interestingly, with the property() invoked, the instances are not GCed, without calling that, everything is GCed just fine.