On Wed, 10 Jan 2024 15:15:36 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>> test/jdk/javax/management/Introspector/ClassLeakTest.java line 55:
>>
>>> 53: ObjectName testName = new ObjectName("x:name=Test");
>>> 54: Test mbean = new Test();
>>> 55: mbs.registerMBean(mbean, testName);
>>
>> I suspect this test should first register an MBean which is a ClassLoader
>> implementing PrivateClassLoader, through which the TestMBean would be loaded
>> in order to preserve the semantics of the test.
>>
>> In other words, replace the MLet with a regular MBean that extends
>> URLClassLoader and implements PrivateClassLoader and do the same thing than
>> the original test was doing.
>
> I didn't see great value in it, as we no longer provide an MBean which is a
> ClassLoader.
> Having a test MBean that extends URLClassLoader, and calling its
> loadClass()... is basically testing URLClassLoader? 8-)
> Implementing PrivateClassLoader affects ClassLoaderRepository behaviour, but
> that's not tested here.
> If you think this has value, we can do it.
My thinking is that this tests for a leak when an MBean is created using a
ClassLoader which is also an MBean. MLet was such a ready-to-use ClassLoader.
We no longer have MLets, but it's still possible to register an MBean that is a
class loader and use that to create another MBean whose concrete class gets
loaded by that ClassLoader MBean. We're testing that the Introspector doesn't
create a leak in this case. And I believe the fact that the ClassLoader MBean
is a PrivateClassLoader may also be of importance in the tested scenario
(maybe).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16363#discussion_r1447605593
