RFR: 8308235: ThreadContainer registry accumulates weak refs

[Jaikiran Pai]

On Thu, 18 May 2023 15:44:15 GMT, Alan Bateman <alanb at 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