RFR (S + L test) : 8016839 : JSR292: AME instead of IAE when calling a method

[David Holmes]

Hi Peter,

On 27/11/2013 7:20 PM, Peter Levart wrote:
> On 11/27/2013 08:40 AM, David Holmes wrote:
>> On 27/11/2013 2:16 AM, David Chase wrote:
>>> On 2013-11-26, at 8:12 AM, David Chase <david.r.chase at oracle.com> wrote:
>>>> On 2013-11-26, at 7:32 AM, David Holmes <david.holmes at oracle.com>
>>>> wrote:
>>>>> On 26/11/2013 10:16 PM, David Chase wrote:
>>>>>> On 2013-11-26, at 7:12 AM, David Holmes <david.holmes at oracle.com>
>>>>>> wrote:
>>>>>>> On 26/11/2013 9:56 PM, David Chase wrote:
>>>>>>>> On 2013-11-25, at 9:18 PM, David Holmes
>>>>>>>> <david.holmes at oracle.com> wrote:
>>>>>>>>> We do have the jdk.internal namespace. But I think Unsafe is as
>>>>>>>>> good a place as any - though maybe sun.misc.VM is marginally
>>>>>>>>> better?
>>>>>>>>
>>>>>>>> Does anyone have any problems with sun.misc.VM as a choice?
>>>>>>>> I have to do a minor revision to the hotspot commit anyway.
>>>>>>>> Is sun.misc.VM also loaded very early anyway?
>>>>>>>
>>>>>>> No you would have to add it as for Unsafe.
>>>>>>
>>>>>> But it's loaded early anyway as a normal consequence of other
>>>>>> class loading, right?
>>>>>
>>>>> What do you mean by "early"? It isn't a pre-loaded class but it
>>>>> will be loaded during system initialization. It is approx the 120th
>>>>> class to be loaded. Unsafe is about 135th.
>>>>
>>>> 120 is earlier than 135, so by that measure it is superior.
>>>> Do you see any other problems with the change?
>>>> The method's not at all "Unsafe" in the technical sense of the word,
>>>> so it is just a matter of choosing a good home.
>>>
>>> On further investigation, change to sun.misc.VM would be the first
>>> time that hotspot knows of the existence of sun.misc.VM;
>>> sun.misc.Unsafe is already filled with methods that the runtime knows
>>> about (intrinsics, etc).  I think Unsafe is better.
>>
>> Okay.
>>
>> David
>
> Hi David(s),
>
> Excuse me for my ignorance, but does pre-loading the class involve it's
> initialization? Is static initializer called at that time?

No, pre-load is simply loading not initialization. Static initialization 
gets triggerred via a controlled process as it is very delicate.

Even if it is
> not at that time, it surely should be called before first invocation of
> a method on that class (the throwIllegalAccessError() method in this
> case). You need a reference to this method very early - even before
> system initialization starts. How early do you expect first "faulty
> invocations" could occur that need to call this method? What if calling
> that method triggers non-trivial initialization which in turn encounters
> another faulty invocation?

These faults should only appear in application classes so generally 
everything should be initialized well before you need to use this 
method. If a core library class has a bug that triggered this need to 
call this method then yes it is possible that it might happen too early 
to succeed - but that is quite normal for the core classes, there are a 
number of things that can happen too early in the initialization process 
to work (eg throwing exceptions, using assertions).

That said I'm not sure how this could fail in that all we need is a 
reference to the method to put in the vtable and we have that after 
loading. Then all that can go wrong is that we can't actually throw the 
exception.

> sun.misc.Unsafe has a non-trivial static initialization which involves
> "registerNatives()" native method invocation, and it also calls:

registerNative is not an issue

> sun.reflect.Reflection.registerMethodsToFilter(Unsafe.class, "getUnsafe");
>
> ...which initializes hell lot of things (like java.util.HashMap for
> example, which in who knows which incarnation included random hash seed
> initialization which triggered random number generator initialization
> with gathering of random seed from various sources, ...)

> sun.misc.VM on the other hand, only has the following static
> initialization:
>
>      private static final Object lock = new Object();
>
>      static {
>          initialize();
>      }
>      private native static void initialize();
>
>
> Are you okay with all possible interactions of this initialization on
> all platforms?

The only time the initialization order would be changed by this fix is 
if one of the classes initialized before-hand had a bug that required 
this fix to come into affect. That is obviously a JDK bug and is 
extremely unlikely to happen.

Note that sun.misc.VM is currently initialized 44th while Unsafe is 61st.

I don't see any issues with this fix in that regard.

> I think a new class with only this method would be a safer choice.
> Regarding back-porting, even if sun.misc.Unsafe is used as the holder
> for that method, this new method will have to be added to
> sun.misc.Unsafe on those legacy platforms in order to obtain this new
> Method *. If the method is not found, the VM would have to behave as
> before. Couldn't the pre-loading of classes be made such that some of
> them are optional?

It already supports optional classes.

David H.
--------

>
>
> Regard, Peter
>
>>
>>> David
>>>
>