RFR(S): 8173743: Failures during class definition can lead to memory leaks in metaspace

Karen Kinnear karen.kinnear at oracle.com
Tue Feb 7 18:06:24 UTC 2017


Volker,

Thank you so much for this fix.
The current code looks good.

Thank you for the mix-and-match flag testing for UnsyncloadClass and AllowParallelDefineClass !

A couple of notes:
1) \ I think there is also a need for a check
inside load_instance_klass for the null loader case, i.e. after the call to find_or_define_instance_class.

2) Just a note - you could also get this problem if you ran into OOME e.g. adding a class to the ClassLoader vector
in define_class. You have this covered by covering all callers of define_instance_class.

3) one clarification below

thanks,
Karen

> On Feb 6, 2017, at 1:40 PM, Volker Simonis <volker.simonis at gmail.com> wrote:
> 
> Hi,
> 
> can somebody please review the following change:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2017/8173743.v1/
> https://bugs.openjdk.java.net/browse/JDK-8173743
> 
> It fixes some problems during class definitions where instance klasses
> can leak into the metaspace in cases where the class definition fails
> after the class was successfully loaded from the bytecode stream.
> 
> There are actually two cases to consider:
> 
> 1. If we load the same class several times into the same class loader,
> we will get LinkageErrors after the first successful load. But
> nevertheless we will first construct a new instanceKlass for each new
> load attempt and this instanceKlass will never be deleted.
Just to be sure we are saying the same thing.
If we try to define a class several times in the same class loader, …

And I’m sure you already know - we do that so that we correctly throw
any class file format errors on the new byte stream passed in.
> 
> 2. If we have a parallel capable class loader, set
> -XX:-UnsyncloadClass and/or -XX:+AllowParallelDefineClass and load a
> class from several threads at the same time in parallel, it can happen
> that we create several instance klasses for the same class. At the end
> only one of them will be added to the system dictionary, but all the
> other ones will never be deleted. Notice that if we run this scenario
> without setting either of -XX:-UnsyncloadClass or
> -XX:+AllowParallelDefineClass, this scenario will degrade into the
> case above and we will get LinkageErrors for all but the first
> successful load.
> 
> The change comes with a regression test which checks for the two cases
> just describe and also for the failing class redefinition case, which
> currently doesn't produce a memory leak.
> 
> I've already committed this to the hs-demo-submit/hotspot/ forest and
> it went through without a problem. So in theory it should have passed
> the internal JPRT tests although I'm not sure if the test set of the
> "demo-submit" forest and the real hotspot repo are exactly the same
> (CC'ed Tim and Brian).
> 
> Thank you and best regards,
> Volker



More information about the hotspot-runtime-dev mailing list