Skip to content

Fix ClientFactory leaks in ResteasyClient - #5833

Merged
ikhoon merged 1 commit into
line:mainfrom
ikhoon:fix-test-leak
Jul 26, 2024
Merged

Fix ClientFactory leaks in ResteasyClient#5833
ikhoon merged 1 commit into
line:mainfrom
ikhoon:fix-test-leak

Conversation

@ikhoon

@ikhoon ikhoon commented Jul 26, 2024

Copy link
Copy Markdown
Contributor

Motivation:

A ClientFactory leak was detected in CalculatorServiceClientServerTest

10:19:16.263 [Test worker] ERROR io.netty.util.ResourceLeakDetector - LEAK: ClientFactory.release() was not called before it's garbage-collected. See https://netty.io/wiki/reference-counted-objects.html for more information.
Recent access records:
Created at:
	com.linecorp.armeria.client.DefaultClientFactory.<init>(DefaultClientFactory.java:101)
	com.linecorp.armeria.client.ClientFactoryBuilder.build(ClientFactoryBuilder.java:998)
	com.linecorp.armeria.client.resteasy.ArmeriaResteasyClientBuilder.buildWebClient(ArmeriaResteasyClientBuilder.java:193)
	com.linecorp.armeria.client.resteasy.ArmeriaResteasyClientBuilder.build(ArmeriaResteasyClientBuilder.java:210)
	com.linecorp.armeria.server.resteasy.CalculatorServiceClientServerTest.newWebTarget(CalculatorServiceClientServerTest.java:124)
	com.linecorp.armeria.server.resteasy.CalculatorServiceClientServerTest.testCalc(CalculatorServiceClientServerTest.java:92)

A new ClientFactory is created when a ResteasyClient is made but the factory isn't closed.

return new ArmeriaResteasyClientBuilder(webClientBuilder, ClientFactory.builder());

Modifications:

  • Close the WebClient's ClientFactory when ArmeriaJaxrsClientEngine is closed.

Result:

ResteasyClient now correctly releases ClientFactory when it is closed.

Motivation:

A `ClientFactory` leak was detected in `CalculatorServiceClientServerTest`
```java
10:19:16.263 [Test worker] ERROR io.netty.util.ResourceLeakDetector - LEAK: ClientFactory.release() was not called before it's garbage-collected. See https://netty.io/wiki/reference-counted-objects.html for more information.
Recent access records:
Created at:
	com.linecorp.armeria.client.DefaultClientFactory.<init>(DefaultClientFactory.java:101)
	com.linecorp.armeria.client.ClientFactoryBuilder.build(ClientFactoryBuilder.java:998)
	com.linecorp.armeria.client.resteasy.ArmeriaResteasyClientBuilder.buildWebClient(ArmeriaResteasyClientBuilder.java:193)
	com.linecorp.armeria.client.resteasy.ArmeriaResteasyClientBuilder.build(ArmeriaResteasyClientBuilder.java:210)
	com.linecorp.armeria.server.resteasy.CalculatorServiceClientServerTest.newWebTarget(CalculatorServiceClientServerTest.java:124)
	com.linecorp.armeria.server.resteasy.CalculatorServiceClientServerTest.testCalc(CalculatorServiceClientServerTest.java:92)
```

The reason is a new `ClientFactory` is created  when a `ResteasyClient` is created but
the factory isn't closed.
https://github.com/line/armeria/blob/cbe744e453eb66e7a1588d2d4cab8582d446b9ce/resteasy/src/main/java/com/linecorp/armeria/client/resteasy/ArmeriaResteasyClientBuilder.java#L91

Modifications:

- Close the `WebClient`'s `ClientFactory` when
  `ArmeriaJaxrsClientEngine` is closed.

Result:

`ResteasyClient` now correctly releases `ClientFactory` when it is closed.

@jrhee17 jrhee17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

@minwoox minwoox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

@ikhoon
ikhoon merged commit 7a5a4c5 into line:main Jul 26, 2024
@ikhoon
ikhoon deleted the fix-test-leak branch July 26, 2024 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants