RFR: 8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs [v4]

[Mandy Chung]

On 12/15/20 5:50 PM, David Holmes wrote:
> On 16/12/2020 11:35 am, Serguei Spitsyn wrote:
>> On Wed, 16 Dec 2020 00:50:04 GMT, David Holmes <dholmes at openjdk.org> 
>> wrote:
>>> The agent class doesn't have to be public it just has to be accessible.
>>>
>>> The premain method should be queried for public modifier rather than 
>>> just relying on a failed invocation request.
>>
>> David, thank you for catching this. I'm probably missing something here.
>> If the agent class is not public then the `m.canAccess(null)` check 
>> is not passed and IAE is thrown with the message:
>>    `Exception in thread "main" java.lang.IllegalAccessException: 
>> method NonPublicAgent.premain must be declared public`
>>
>> But the `NonPublicAgent.premain` is declared public as below:
>>      public static void premain(String agentArgs, Instrumentation 
>> inst) {
>>          System.out.println("premain: NonPublicAgent was loaded");
>>      }
>> It seems, the IAE is thrown because the agent class is not public.
>> Does it mean the `m.canAccess(null)` check is not fully correct?
>
> canAccess will check both the type accessibility and the method 
> accessibility.
>
> The premain class is not required to be public 

(I think you meant "exported").


The premain method must be public as this issue about.   I expect the 
agent module must export the package of the Premain-Class (or 
Agent-Class) premain method (qualifiedly or unconditionally) for 
java.instrument to access.  The spec does not state clearly if the agent 
class is public but as it requires the premain method public, it is 
sensible to say it's accessible (otherwise, premain method does not have 
to be public), i.e. public premain method in a public (accessible) 
class).    So in the modular world, the public method in a public class 
in a package exported at least to java.instrument.

Are you expecting differently?

The spec should clarify.  FWIW.  I just realized that 
https://bugs.openjdk.java.net/browse/JDK-6932391 is not resolved.


> so it may not be right to checks its accessibility as that may fail 
> even though the premain method is public and should be invoked. In 
> that regard I think the setAccessible call has to remain to deal with 
> this situation. So the process would be:
>
> - load premain class via appLoader
> - lookup premain method using getDeclaredMethod()
> - check premain method is public and reject if not
> - call setAccessible(true) in case premain class is not accessible
> - do invoke
>
> David

Mandy


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20201215/abe5d946/attachment.htm>