15 RFR(XS): 8165276: Spec states that invoke the premain method in an agent class if it's public but implementation differs

[serguei.spitsyn at oracle.com]

Hi Larry,

Thank you for looking at this!


On 6/23/20 21:32, Laurence Cable wrote:
> should we not consider some form of depreciation here, and continue to 
> support non-public pre-main invocation for some time while issuing a 
> warning???

I'm not sure what form of deprecation we can use as it has to be a 
deprecation of a spec non-compliant implementation. :)

>
> while we have a sample of agents that will not be affected there may 
> be some agent that will fail terminally with this change

There is more important problem now.
A big number or j.l.instrument started to fail with my fix with messages 
like this:
  Exception in thread "main" java.lang.IllegalAccessException: class 
sun.instrument.InstrumentationImpl
  (in module java.instrument) cannot access a member of class 
SimpleAgent with modifiers "public static"

I'm not sure if there can be a version of the 
Method.setAccessible(boolean flag) api that works for public methods only.
One alternate approach is to relax the current spec to allow premain 
methods to be non-public.

Thanks,
Serguei


>
> just a thought
>
> - Larry
>
> On 6/23/20 8:42 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Mandy,
>>
>> Thank you for looking at this!
>>
>>
>> On 6/23/20 20:21, Mandy Chung wrote:
>>> Hi Serguei,
>>>
>>> I'm glad that you have a patch for this.
>>>
>>> On 6/23/20 7:05 PM, serguei.spitsyn at oracle.com wrote:
>>>> Please, review a fix for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8165276
>>>>
>>>>
>>>> CSR draft (one CSR reviewer is needed before finalizing it):
>>>> https://bugs.openjdk.java.net/browse/JDK-8248189
>>>>
>>>
>>> The compatibility risk should be low (rather than minimal).
>>
>> I was not sure if it has to be minimal or low.
>> Made it low now.
>>
>>
>>> It says "All known Java agents define the premain method as public". 
>>> It'd be useful to add a comment in the JBS issue to list the Java 
>>> agents you have checked.
>>
>> I'm relying on the Alan's comments posted in the bug report:
>>  "I checked a number of popular java agents and their premain methods 
>> are public, I haven't found any where the premain was not public."
>>  "I think we should just bite the bullet on this so that the premain 
>> must be public as originally intended."
>>
>> Probably, my statement in the CSR is too strong.
>> I've changed it to:
>>  "No popular Java agent that defines the premain method as a 
>> non-public was found."
>>
>> Does it looks better or you think we have to investigate existing 
>> popular Java agents?
>>
>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.1/ 
>>>>
>>>>
>>>>
>>>
>>> Looks okay.  Can you add a test to verify this fix?
>>
>> Yes, I can add a test but it will be trivial.
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>> Mandy
>>
>