On Thu, 18 May 2023 15:44:15 GMT, Alan Bateman <al...@openjdk.org> wrote:
> ThreadContainers is an internal class used to make thread pools and other > groupings of threads discoverable for observability. Some refactoring in 2021 > (in the loom repo, and before integration) accidentally changed the creation > of a weak reference so that it no longer associated with the reference queue. > The result is that stale refs aren't expunged from a CHM, leading to a memory > leak. The change to fix the issue is trivial. > > Tests for memory leaks can be problematic, often more trouble than they are > worth. I started with a test that polls the size of the internal CHM but > decided to ditch it. Instead, the test is simple. It just runs for a few > seconds creating ExecuorService implementations (including TPE, TPPE, and > FJP), unreferencing them without shutdown (so they don't terminate and > unregister). This is enough to causes OOME with product builds a small heap. The change looks OK to me. The `QUEUE` (of `WeakReference`) and thus the registry, gets cleaned up whenever new `ThreadContainer`s are registered with the registry. Would it be worth calling `expungeStaleEntries()` to do this cleanup even when a specific `ThreadContainer` is deregistered, in the implementation of method `deregisterContainer(...)`? ------------- PR Comment: https://git.openjdk.org/jdk/pull/14047#issuecomment-1554246479
