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

Missing equals/hashCode in HttpVersionSelection class generates a memory leak #9994

Closed
jpbempel opened this issue Oct 18, 2023 · 1 comment · Fixed by #9999
Closed

Missing equals/hashCode in HttpVersionSelection class generates a memory leak #9994

jpbempel opened this issue Oct 18, 2023 · 1 comment · Fixed by #9999
Assignees
Labels
type: bug Something isn't working

Comments

@jpbempel
Copy link

jpbempel commented Oct 18, 2023

Expected Behavior

no memory leak

Actual Behaviour

io.micronaut.http.client.netty.DefaultHttpClient instances are leaking and are retained in unbalancedClients map in io.micronaut.http.client.netty.DefaultNettyHttpClientRegistry

Screenshot 2023-10-17 at 15 13 36

Steps To Reproduce

In our application we have defined a http client like this:

@Client(
    id = "rc-api",
    path = "/api/unstable/remote_config/products/product1",
    alpnModes = [HttpVersionSelection.ALPN_HTTP_1],
)
interface RcApiClient {
}

We have noticed after a certain time, that the Java Heap was filling up to the maximum.
By analysis of the heap dump we noticed a large amount of DefaultHttpClient instances stored into the unbalancedClients map in DefaultNettyHttpClientRegistry.

Insertion into the map is done by this:

private DefaultHttpClient getClient(ClientKey key, BeanContext beanContext, AnnotationMetadata annotationMetadata) {
        return unbalancedClients.computeIfAbsent(key, clientKey -> {

and the key of this unbalancedClients map is io.micronaut.http.client.netty.DefaultNettyHttpClientRegistry.ClientKey

ClientKey class is made from the following fields:

        final HttpVersionSelection httpVersion;
        final String clientId;
        final List<String> filterAnnotations;
        final String path;
        final Class<?> configurationClass;
        final JsonFeatures jsonFeatures;

and equals and hashCode:

       @Override
        public boolean equals(Object o) {
            if (this == o) {
                return true;
            }
            if (o == null || getClass() != o.getClass()) {
                return false;
            }
            ClientKey clientKey = (ClientKey) o;
            return httpVersion == clientKey.httpVersion &&
                    Objects.equals(clientId, clientKey.clientId) &&
                    Objects.equals(filterAnnotations, clientKey.filterAnnotations) &&
                    Objects.equals(path, clientKey.path) &&
                    Objects.equals(configurationClass, clientKey.configurationClass) &&
                    Objects.equals(jsonFeatures, clientKey.jsonFeatures);
        }

        @Override
        public int hashCode() {
            return Objects.hash(httpVersion, clientId, filterAnnotations, path, configurationClass, jsonFeatures);
        }

The problem is that httpVersion field when not null is using the type io.micronaut.http.client.HttpVersionSelection that does not provide any equals and hashCode methods.
So in the case of the ClientKey::hashCode it means that we are using an identityHashCode and using @client annotation with alpn will create a new instance of HttpVersionSelection (through DefaultNettyHttpClientRegistry#getClientKey -> HttpVersionSelection#forClientAnnotation.
Then every http client have a different hashCode that will spread across the unbalancedClients map.
Moreover the ClientKey::equals use reference equality (==) for the httpVersion field which will be always false.

It ends up creating a new http client for every request and stored into the map.

Environment Information

No response

Example Application

No response

Version

4.1.5

@graemerocher graemerocher added the type: bug Something isn't working label Oct 20, 2023
yawkat added a commit that referenced this issue Oct 20, 2023
@yawkat
Copy link
Member

yawkat commented Oct 20, 2023

thanks for the report and figuring out the cause.

@yawkat yawkat linked a pull request Oct 23, 2023 that will close this issue
graemerocher pushed a commit that referenced this issue Oct 23, 2023
* Add eq/hc for HttpVersionSelection

Fixes #9994
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants