Skip to content

Remove DnsCacheListener from DnsCache when resolver is closed - #6174

Merged
trustin merged 2 commits into
line:mainfrom
yzfeng2020:remove-listener-when-resolver-close
Mar 27, 2025
Merged

Remove DnsCacheListener from DnsCache when resolver is closed#6174
trustin merged 2 commits into
line:mainfrom
yzfeng2020:remove-listener-when-resolver-close

Conversation

@yzfeng2020

@yzfeng2020 yzfeng2020 commented Mar 26, 2025

Copy link
Copy Markdown
Contributor

Motivation:

The RefreshingAddressResolver is added to the DnsCache as listener but is never removed from it when the resolver is closed. This makes the DnsCache hold a strong reference (internally, in the CopyOnWriteArrayList that stores all the listeners) of it even when the resolver is closed, which may cause memory leak if the DnsCache is a global one and lives for the lifetime of the JVM.

Modifications:

  • Removes the DnsCacheListener from the DnsCache to make it eligible for GC.

Result:

@yzfeng2020 yzfeng2020 changed the title Remove DnsCacheListener when resolver is closed Remove DnsCacheListener from DnsCache when resolver is closed Mar 26, 2025

@trustin trustin 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.

Thanks a lot for fixing this, @yzfeng2020! I left one nit, but it looks great! 🙇

Comment thread core/src/main/java/com/linecorp/armeria/client/DefaultDnsCache.java Outdated
@trustin trustin added the defect label Mar 27, 2025
@trustin trustin added this to the 1.33.0 milestone Mar 27, 2025

@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.

Thanks!

@trustin
trustin merged commit 3ef8c77 into line:main Mar 27, 2025
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.

Possible memory leak wrt RefreshingAddressResolver

5 participants