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

[Peter Levart]

On 11/27/2013 11:03 AM, David Holmes wrote:
> 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 see. Thanks for explanation, David.

>
>> 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.

Ah, I have misunderstood the back-porting issue. It was not about not 
having new class but about which existing class to use as a host that 
might not exist in older version of platform...

Sorry for noise.

Regards, Peter

>
> David H.
> --------
>
>>
>>
>> Regard, Peter
>>
>>>
>>>> David
>>>>
>>