On Mon, 14 Dec 2020 22:45:26 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> Changes requested by dholmes (Reviewer).
>
> Alan, David and Many, thank you for the comments!
> I'll prepare an update according to the recent requests from you.
> One question is if I need to clone this PR to the JDK 16 fork:
>   https://github.com/openjdk/jdk16
> It depends on our chances to finalize this before RDP2.
> An alternate approach would be to continue reviewing this PR until all 
> comment are resolved and then clone it to jdk16.

Mandy, thank you for the suggestion.
I'll retarget the bug and CSR to jdk 17 as nobody is objecting.

Also, wanted to make it clear about Exception messages that are provided for 
two different cases.

The test java/lang/instrument/NonPublicPremainAgent.java defines a non-public 
premain method:
`    static void premain(String agentArgs, Instrumentation inst) {`

With the m.canAccess check the following message is provided (it looks right):
`  Exception in thread "main" java.lang.IllegalAccessException: method 
NonPublicPremainAgent.premain must be declared public `

Without this check the message was (not sure, it is good enough):
`  Exception in thread "main" java.lang.IllegalAccessException: class 
sun.instrument.InstrumentationImpl (in module java.instrument) cannot access a 
member of class NonPublicPremainAgent with modifiers "static"`

Also, I've added a new test java/lang/instrument/NonPublicAgent.java which 
defines a non-public agent class with a public premain method.

With the m.canAccess check the following message is provided (it does not look 
right):
`  Exception in thread "main" java.lang.IllegalAccessException: method 
NonPublicPremainAgent.premain must be declared public `

Without this check the message is (it looks pretty confusing):
`  Exception in thread "main" java.lang.IllegalAccessException: class 
sun.instrument.InstrumentationImpl (in module java.instrument) cannot access a 
member of class NonPublicAgent with modifiers "public static"`

So, it seems we also need an explicit check for agent class being public with a 
right message provided.
Please, let me know what you think.

I've done some tests refactoring motivated by some private requests from Mandy. 
Will publish the update as soon as it is ready. Hopefully, today.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1694

Reply via email to