On Fri, 12 Nov 2021 21:43:42 GMT, Roman Kennke <rken...@openjdk.org> wrote:

> The caches in ObjectStreamClass basically map WeakReference<Class> to 
> SoftReference<ObjectStreamClass>, where the ObjectStreamClass also references 
> the same Class. That means that the cache entry, and thus the class and its 
> class-loader, will not get reclaimed, unless the GC determines that memory 
> pressure is very high.
> 
> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
> all its classes alive much longer than necessary: as soon as a ClassLoader 
> (and all its classes) become unreachable, there is no point in retaining the 
> stuff in OSC's caches.
> 
> The proposed change is to use WeakReference instead of SoftReference for the 
> values in caches.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2
>  - [x] tier3
>  - [ ] tier4

test/jdk/java/io/ObjectStreamClass/TestOSCClassLoaderLeak.java line 55:

> 53:         con = null;
> 54:         assert myOwnClassLoaderWeakReference.get() != null;
> 55: 

It is preferable is to write (new) tests using TestNG.
Relying on Assert to be enabled is not reliable.
It is preferable to make the checks explicit and throw RuntimeExceptions on 
failure.

test/jdk/java/io/ObjectStreamClass/TestOSCClassLoaderLeak.java line 57:

> 55: 
> 56:         gc();
> 57: 

Is the dependency on ParallelGC necessary?
To may understanding invoking System.gc() is only a request to gc and does not 
reflect any idea that it has completed.  
There is a function in the test library util/ForceGC to ensure gc has completed.

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

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

Reply via email to