RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v7]

[Roman Kennke]

On Tue, 7 Dec 2021 06:23:22 GMT, Peter Levart <plevart at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix cast; add testcase to cover memory pressure
>
> test/jdk/java/io/ObjectStreamClass/TestOSCClassLoaderLeak.java line 84:
> 
>> 82: 
>> 83:         assertNotNull(ObjectStreamClass.lookup(TestClass.class).getFields());
>> 84:     }
> 
> I don't quite get this test. It loads ObjectStreamClass_MemoryLeakExample class from child class loader, constructs an instance from it and calls .toString() on an instance. This is just to indicate that the class initializer of that class did lookup an ObjectStreamClass instance for Test class loaded by the same child loader. OK so far...
> Then there is this loop that tries to exhibit some memory pressure while constantly looking up OSC for another Test class (this time loaded by parent class loader) presumably to trigger clearing the SoftReference(s) of both classes loaded by child ClassLoader.... Is this what the loop was supposed to do?
> And finally there is an assertNotNull that does another lookup for OSC of Test class loaded by parent class loader, retrive its fields and check that the returned OSC instance as well as the field array are not null. This will always succeed regardless of what you do before the assertion.
> 
> I don't think you need any custom class loading to verify the correctness of caching. The following two tests pass on old implementation of OSC. Do they pass on the new one too?
> 
> 
> public class ObjectStreamClassCaching {
> 
>     @Test
>     public void testCachingEffectiveness() throws Exception {
>         var ref = lookupObjectStreamClass(TestClass.class);
>         System.gc();
>         Thread.sleep(100L);
>         // to trigger any ReferenceQueue processing...
>         lookupObjectStreamClass(AnotherTestClass.class);
>         Assertions.assertFalse(ref.refersTo(null),
>                                "Cache lost entry although memory was not under pressure");
>     }
> 
>     @Test
>     public void testCacheReleaseUnderMemoryPressure() throws Exception {
>         var ref = lookupObjectStreamClass(TestClass.class);
>         pressMemoryHard(ref);
>         System.gc();
>         Thread.sleep(100L);
>         Assertions.assertTrue(ref.refersTo(null),
>                               "Cache still has entry although memory was pressed hard");
>     }
> 
>     // separate method so that the looked-up ObjectStreamClass is not kept on stack
>     private static WeakReference<?> lookupObjectStreamClass(Class<?> cl) {
>         return new WeakReference<>(ObjectStreamClass.lookup(cl));
>     }
> 
>     private static void pressMemoryHard(Reference<?> ref) {
>         try {
>             var list = new ArrayList<>();
>             while (!ref.refersTo(null)) {
>                 list.add(new byte[1024 * 1024 * 64]); // 64 MiB chunks
>             }
>         } catch (OutOfMemoryError e) {
>             // release
>         }
>     }
> }
> 
> class TestClass implements Serializable {
> }
> 
> class AnotherTestClass implements Serializable {
> }

The test was a rather crude (but successful) attempt to demonstrate the ClassCastException. Thanks for providing the better testcase. I verified that it succeeds with this PR, and also demonstrates the ClassCastException if I revert my previous change in ClassCache. I pushed this new test, and removed my old one.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6375