igniterealtime
Excessive numbers of NonInjectionManagers pile up in JVM heap
As background, I investigated a periodical OutOfMemoryError encountered in a server application I am working on. This used REST-API-Client built from the 'master' branch, for JDK 17 support. The main symptoms were:
Symptom 1:
Lots of messages like:
org.glassfish.jersey.client.innate.inject.NonInjectionManager.<init> Falling back to injection-less client.
Symptom 2:
Periodical crashes even when the server application saw no usage, apart from health checks that performed a request using org.igniterealtime.restclient.RestApiClient.
A heap dump showed a lot of org.glassfish.jersey.client.innate.inject.NonInjectionManagers having piled up:
This is from a server application that has not seen much use apart from a health check every few seconds for roughly half a day. Each health check created a new RestApiClient and performed a single request with it.
After analyzing these symptoms, I found various problems and solutions to many of them. This approach uses jersey-injectless-client. I also tried using jersey-hk2, but it suffered from piling up of various hk2-related classes, caused OutOfMemoryErrors in more active usage and was more difficult to analyze, so I abandoned that research.
Problem 1: MessageBodyWriter error messages
I also found messages like in the logs of the server application I am developing:
MessageBodyWriter not found for media type=application/xml, type=class org.igniterealtime.restclient.entity.UserEntity, genericType=class org.igniterealtime.restclient.entity.UserEntity.
This is fixed by adding jersey-media-jaxb as a dependency.
Problem 2: Warnings about NonInjectionManager
Every time a request is made with RestApiClient, the following message appears in the logs:
org.glassfish.jersey.client.innate.inject.NonInjectionManager.<init> Falling back to injection-less client.
These disappear after adding either jersey-hk2 or jersey-injectless-client as a dependency. I tried both and ended up with jersey-injectless-client due to the other problems being simpler to analyze with it.
Problem 3: New NonInjectionManager is created for each request
I noticed NonInjectionManagers piling up in the heap dump.
I set a debugger breakpoint at NonInjectionManager constructors, and performed requests with the RestApiClient. A new NonInjectionManager is created every time.
To solve this problem, I changed RestClient to store the created Jersey client and return it for future use:
- private Client createRestClient() throws KeyManagementException, NoSuchAlgorithmException {
+ private Client getOrCreateRestClient() throws KeyManagementException, NoSuchAlgorithmException {
+ if(this.client != null)
+ return this.client;
+
ClientConfig clientConfig = new ClientConfig();
// Set connection timeout
if (this.connectionTimeout != 0) {
@@ -296,6 +301,8 @@ public final class RestClient {
client = ClientBuilder.newClient(clientConfig);
}
+ this.client = client;
+
return client;
}
This solves NonInjectionManagers piling up. This requires also that the application using the RestApiClient does not create multiple RestApiClients.
This code is not as thread-safe as the previous code, but I am unsure if the RestApiClient is aiming for thread-safety.
Problem 4: Allocated resources are not closed
The clients allocated by createRestClient are never explicitly closed. Some of the calls that end up calling it return closeable resources, but not all of them do. I am also unsure if the returned closeable resources free the resources allocated by the client created by createRestClient or not.
A partial fix would be to implement AutoCloseable interface along with the above Problem 3 fix, with the close method calling this.client.close(). However, this did not seem to be sufficient to clean everything based on my tests. When I analyzed this approach, heap dumps still showed a mixture of NonInjectionManagers with 'shutdown' values of both false and true.
Problem 5: NonInjectionManager has a bug that has been patched in a newer Jersey version
Jersey version 3.1.8 should contain a patch to fix this issue: https://github.com/eclipse-ee4j/jersey/issues/5710
This issue may have prevented the piled-up NonInjectionManagers from being garbage collected properly, which would make this the cause of the OutOfMemoryErrors I have observed. I have not been able to validate this, and the approach I eventually picked for the server application I am developing ensures that no excessive amounts of NonInjectionManagers are created in the first place.
Problem 6: NonInjectionManagers still keep piling up over time
Even with the fixes above, I did not manage to find a way to make NonInjectionManagers not keep piling up in the JVM heap. I am unsure if they would now get collected by the GC more properly with the problem 5 fix.
As a workaround, I ended up making the application I am working on create RestApiClients under a ThreadLocal, which limits the number of created instances to the application's thread pool size. This avoids potential concurrency issues.
Results
By patching org.igniterealtime.restclient.RestClient.createRestClient() and updating the dependencies as above, and by ensuring my own application does not create too many RestApiClients, I managed to solve the problem with heap getting filled with NonInjectionManagers. This seems sufficient for my own use case, but solving the resource buildup and cleanup problems even if lots of RestApiClients are created and discarded would be better.
I am unfortunately unable to share the server application code due to it being proprietary.
A reproduction could perhaps be created by just using a single RestApiClient, making api requests in a loop, and placing a breakpoint in NonInjectionManager constructor. This should show that one NonInjectionManager is created for every request.
This same reproduction could be run for a while, and a heap dump taken and analyzed to confirm that they have piled up.
If the heap size is set with -Xmx to a low-ish value, this might even trigger the OutOfMemoryError.
A reproduction that triggers an OutOfMemoryError:
This package includes:
-
ReproductionTest.java: The reproduction, described below -
pom.xml: Project Maven configuration for building and running the test class. This limits the used memory to 100 megabytes, to trigger the OutOfMemory error faster -
Dockerfile: Docker build script to create a full reproduction -
build.sh: Script that performs the Docker build, including running the test and reproducing the error
The main component here is the test class that triggers repeatedly creating NonInjectionManagers by performing requests using the client. The errors caused by the server not existing are simply ignored:
import org.igniterealtime.restclient.RestApiClient;
import org.igniterealtime.restclient.entity.AuthenticationToken;
import org.igniterealtime.restclient.entity.UserEntity;
import org.junit.Test;
import jakarta.ws.rs.core.Response;
public class ReproductionTest
{
@Test
public void test() throws Throwable
{
RestApiClient client =
new RestApiClient("localhost",
12345,
new AuthenticationToken("invalidpassword"));
UserEntity user = new UserEntity("test","test","test","test");
while(true)
{
try(Response response = client.createUser(user))
{
response.getStatus();
}
catch(Exception e)
{
continue; // Keep creating NonInjectionManagers
}
}
}
}
Running build.sh will run the test, which prints lots of Falling back to injection-less client warnings and ends up with this error:
[ERROR] Errors:
[ERROR] ReproductionTest.test:22 ? OutOfMemory Java heap space
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
The OutOfMemory error in the reproduction no longer occurs if the REST-API-Client code is patched like this:
diff --git a/src/main/java/org/igniterealtime/restclient/RestClient.java b/src/main/java/org/igniterealtime/restclient/RestClient.java
index 100c2f7..2e9ca5b 100644
--- a/src/main/java/org/igniterealtime/restclient/RestClient.java
+++ b/src/main/java/org/igniterealtime/restclient/RestClient.java
@@ -84,6 +84,8 @@ public final class RestClient {
*/
private SupportedMediaType mediaType;
+ private Client client;
+
/**
* Gets the.
*
@@ -237,7 +239,7 @@ public final class RestClient {
try {
URI u = new URI(this.baseURI + "/plugins/restapi/v1/" + restPath);
LOG.debug("Connecting to:" + u);
- Client client = createRestClient();
+ Client client = getOrCreateRestClient();
webTarget = client.target(u);
if (queryParams != null && !queryParams.isEmpty()) {
@@ -276,7 +278,10 @@ public final class RestClient {
* @throws KeyManagementException the key management exception
* @throws NoSuchAlgorithmException the no such algorithm exception
*/
- private Client createRestClient() throws KeyManagementException, NoSuchAlgorithmException {
+ private Client getOrCreateRestClient() throws KeyManagementException, NoSuchAlgorithmException {
+ if(this.client != null)
+ return this.client;
+
ClientConfig clientConfig = new ClientConfig();
// Set connection timeout
if (this.connectionTimeout != 0) {
@@ -296,6 +301,8 @@ public final class RestClient {
client = ClientBuilder.newClient(clientConfig);
}
+ this.client = client;
+
return client;
}
This matches my earlier Problem 3 fix idea. The Problem 4 findings of the client never getting closed would still need to be fixed.
I created a pull request that contains fixes for the problems 1-5. This is likely not a complete fix to all resource leakage as I described in the problem descriptions, but it fixes the main issues found here.