On Wed, 10 Jan 2024 17:06:01 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Checking on the original problem noted in the test, looks like it wasn't >> really about ClassLoaders and MLets: >> JDK-4909536: Standard MBean introspector keeps reference to last class >> introspected >> This is what the test still tests. >> Do we see value in having the MBean be a ClassLoader and checking if such >> MBeans are held on to...? 8-) > > Yes, that's the whole point of the test. Without using an MBean which is a > ClassLoader the fix cannot be tested. > 1. you need to create an MBean which is an URLClassLoader, and it needs to be > a PrivateClassLoader so that it's not added to the ClassLoaderRepository - > let's call it LoaderMBean. > 2. you need to use that LoaderMBean to load another MBean - let's call it > TestMBean. The concrete class of TestMBean needs to be loaded by the > LoaderMBean - that is - LoaderMBean needs to be the defining ClassLoader for > that class (the TestMBean class must not be loaded by the System/Application > ClassLoader). > 3. In the MBeanServer you then end up with two MBeans, one is the > LoaderMBean, the other is the TestMBean whose class has been loaded by the > LoaderMBean. > 4. you then keep a weak reference on the _LoaderMBean_, and unregister both > MBeans. > 5. at that point - if the Introspector still has a strong reference to the > class of the TestMBean, then the _LoaderMBean WeakReference_ will never be > enqueued, because the class of the TestMBean will have a strong reference to > its ClassLoader, which is _LoaderMBean_. > > So you can only test that if you have an MBean which is loaded by a custom > ClassLoader, because you want to verify that the Introspector doesn't hold on > that ClassLoader (through the TestMBean class). Hi Daniel - I am missing see how being a ClassLoader relates to the leak where the Introspector holds on to a reference to the most recent MBean. I didn't locate the jdk5 change, I only read a short synopsis. I can make this test create an MBean which is a ClassLoader, and the test can do its check that having the loader MBean load a class, creates a distinct class. I can add that update here. But it looks like an MLet test which leaked into the Introspector test. 8-) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16363#discussion_r1448708811