6354181 and the SystemDictionary

[Karen Kinnear]

Tom,

So, yes, a class is not added to the SystemDictionary until it is  
loaded (define_instance_class calls
add_to_hierarchy which sets_init_state to loaded before  
update_dictionary. So a find() call will return
with a state >= loaded. This is all done at define class time by the  
defining loader.

And yes, a klass is added to the loader constraints potentially long  
before it is loaded. Loader constraints
allow us to lazily ensure that any referenced class loaded by any of  
its initiating loaders will always
reference the same underlying class object as if it were loaded by its  
defining loader. Basically if
class A method X calls class B method Y, A's class loader and B's  
class loader must agree on
Y's parameters and return value. So this can happen when linking A  
which can be long before
B gets loaded.

So I have only walked through the  
find_constrained_instance_or_array_klass once and I
believe it is only called in ciEnv::get_klass_by_name_impl, and only  
if "require_local" is false.
I was unable to figure out the real meaning behind require_local.

find_constrained_instance_or_array_klass first does the  
find_instance_or_array_klass that is
a SystemDictionary lookup, so that should cover loaded classes. I'm  
wondering if the point is
to find classes for which the actual class is loaded by the defining  
loader, but you haven't yet
gone through the exercise of loading the class for the current, i.e. a  
different initiating loader,
and the compiler doesn't/can't initiate loading since that might  
require calling out to java and
the compiler thread can't do that... So instead we look to see if we  
already know due to
constraints what actual instance or array class this has to resolve  
to.  If that is what we are doing,
then it makes sense to look in the loader constraints for this  
initiating loader/class name and
if the class is loaded go ahead and use it.

So I think the modification would be in  
find_constrained_instance_or_array_klass to only return
classes in the loaded state, or arrays for which the element class is  
in the loaded state,
as you suggest.

hope this helps,
Karen

On Jan 26, 2011, at 4:09 PM, Tom Rodriguez wrote:

> I've been debugging 6354181 where we attempt to initialize a class  
> which is only in the allocated state and die.  After heading off  
> into the weeds for a while I discovered that the problem is that the  
> compiler is finding an instanceKlass which is only in the allocated  
> state instead of in the loaded state and generating code using that  
> klass.  We manage to run the code before the klass has been moved to  
> the loaded state and we end up dying.
>
> I can see several ways to guard against this but I can't decide  
> whether this is something that should be guarded against within  
> SystemDictionary itself or whether users of SystemDictionary should  
> be guarding against it.  It appears that we're finding the class  
> through   SystemDictionary::find_constrained_instance_or_array_klass  
> by going through the loader constraints.  I'm assuming that the  
> klass gets added to the loader constraints early and then gets added  
> into the dictionary proper and marked as loaded which creates the  
> window where things can go wrong.  I also assume that other  
> interfaces that perform lookups would never return a class which was  
> still in the allocated state.
>
> It isn't that hard to reproduce this failure but it's very hard to  
> capture a snap shot of exactly how the dictionary is being  
> manipulated at the time we find the klass which leaves me a little  
> unsure how many places really need to guard against this.  Does this  
> seem plausible to anyone who understands the SystemDictionary?  I  
> believe the proper fix is to add a  guard to  
> SystemDictionary::find_constrained_instance_or_array_klass to only  
> return classes which are really in the loaded state.
>
> Part of the problem here is that ciObject has a base method  
> is_loaded() which returns true if the ciObject is backed by a real  
> Java object.  This means that is_loaded on a ciInstanceKlass doesn't  
> mean the same thing as instanceKlass::is_loaded unless we disallow  
> the allocated state completely.  I could simply guard against it in  
> the CI but that doesn't really feel right.  I will definitely put  
> some asserts about in there though since that would have made the  
> bug much more obvious.
>
> tom