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

Hi David, Thank you a lot for review and tweaking the bug title. I've re-targeted this to 16 as was suggested by Joe. Thanks, Serguei On 6/28/20 19:37, David Holmes wrote: Hi Serguei, These changes look good to me. Note that I tweaked the bug synopsis to make it slightly more

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

On 6/27/20 00:23, Alan Bateman wrote: On 27/06/2020 01:40, serguei.spit...@oracle.com wrote: The implementation has this order of lookup:     // The agent class must have a premain or agentmain method that     // has 1 or 2 arguments. We check in the following order:     //    

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

On 6/27/20 12:23 AM, Alan Bateman wrote: On 27/06/2020 01:40, serguei.spit...@oracle.com wrote: The implementation has this order of lookup:     // The agent class must have a premain or agentmain method that     // has 1 or 2 arguments. We check in the following order:     //   

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

Hi Serguei, These changes look good to me. Note that I tweaked the bug synopsis to make it slightly more grammatically correct: that invoke -> to invoke Thanks, David On 26/06/2020 2:17 am, serguei.spit...@oracle.com wrote: New wevrev version is:

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

On 27/06/2020 01:40, serguei.spit...@oracle.com wrote: The implementation has this order of lookup:     // The agent class must have a premain or agentmain method that     // has 1 or 2 arguments. We check in the following order:     //     // 1) declared with a signature of

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

On 6/25/20 11:07, Alan Bateman wrote: On 25/06/2020 17:17, serguei.spit...@oracle.com wrote: New wevrev version is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.2/ One inconsistency is that it uses getDeclaredMethod to find the 2-arg premain and getMethod to find the

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

Thanks you for reviewing, Alan! I'll check if it can be fixed. Thanks, Serguei On 6/25/20 11:07, Alan Bateman wrote: On 25/06/2020 17:17, serguei.spit...@oracle.com wrote: New wevrev version is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.2/ One inconsistency is

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

On 25/06/2020 17:17, serguei.spit...@oracle.com wrote: New wevrev version is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.2/ One inconsistency is that it uses getDeclaredMethod to find the 2-arg premain and getMethod to find the 1-arg premain. The latter will fail if

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

New wevrev version is:   http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.2/ Now the InstrumentationImpl.java has this new check to throw IAE with the meaningful error message: +// reject non-public premain or

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

On 6/24/20 12:44, Mandy Chung wrote: On 6/24/20 12:26 PM, serguei.spit...@oracle.com wrote: On 6/24/20 05:25, David Holmes wrote: Ah! The test class SimpleAgent is what is not public. That seems a bug in the test. There are many such tests. We can break some of the existing agents by

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

On 6/24/20 12:26 PM, serguei.spit...@oracle.com wrote: On 6/24/20 05:25, David Holmes wrote: Ah! The test class SimpleAgent is what is not public. That seems a bug in the test. There are many such tests. We can break some of the existing agents by rejecting non-public agent classes. I'm

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

On 6/24/20 05:25, David Holmes wrote: On 24/06/2020 8:14 pm, Alan Bateman wrote: On 24/06/2020 10:57, David Holmes wrote: But you are ignoring my next statement. If we remove the setAccessible(true) then the premain method will not be accessible as Serguei reported. Exception in thread

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

On 24/06/2020 8:14 pm, Alan Bateman wrote: On 24/06/2020 10:57, David Holmes wrote: But you are ignoring my next statement. If we remove the setAccessible(true) then the premain method will not be accessible as Serguei reported. Exception in thread "main" java.lang.IllegalAccessException:

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

On 24/06/2020 10:57, David Holmes wrote: But you are ignoring my next statement. If we remove the setAccessible(true) then the premain method will not be accessible as Serguei reported. Exception in thread "main" java.lang.IllegalAccessException: class sun.instrument.InstrumentationImpl  

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

On 24/06/2020 7:43 pm, Alan Bateman wrote: On 24/06/2020 10:26, David Holmes wrote: If we call setAccessible(true) then canAccess will return true. Sure but the bug fix will remove the setAccessible(true) so canAccess will do what he wants without needing to catch the exception. This is of

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

On 24/06/2020 10:26, David Holmes wrote: If we call setAccessible(true) then canAccess will return true. Sure but the bug fix will remove the setAccessible(true) so canAccess will do what he wants without needing to catch the exception. This is of course all a side show to the important

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

On 24/06/2020 10:22, David Holmes wrote: The other way around. The setAccessible has been there long before the module system existed, to allow a non-public premain. As a side-effect when the module system came along it also disabled some module access check (I'm not sure exactly what). This

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

On 24/06/2020 5:33 pm, Alan Bateman wrote: On 24/06/2020 07:55, serguei.spit...@oracle.com wrote: Thank you for the example. Yes, I'm working on a helpful message and was thinking to use the Reflection method:  IllegalAccessException newIllegalAccessException(Class currentClass,  

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

On 24/06/2020 4:22 pm, Alan Bateman wrote: On 24/06/2020 06:50, David Holmes wrote: It sounds like the use of setAccessible was hiding the need to disable some module related access checks. This will have a much bigger compatibility problem if agents with a public premain suddenly stop

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

On 24/06/2020 4:24 pm, serguei.spit...@oracle.com wrote: Hi David, On 6/23/20 22:50, David Holmes wrote: Hi Serguei, On 24/06/2020 3:37 pm, serguei.spit...@oracle.com wrote: Hi Larry, Thank you for looking at this! On 6/23/20 21:32, Laurence Cable wrote: should we not consider some form

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

On 24/06/2020 07:55, serguei.spit...@oracle.com wrote: Thank you for the example. Yes, I'm working on a helpful message and was thinking to use the Reflection method:  IllegalAccessException newIllegalAccessException(Class currentClass,                          Class

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

On 6/23/20 23:33, Alan Bateman wrote: On 24/06/2020 07:24, serguei.spit...@oracle.com wrote: : One approach would be to continue using the setAccessible and add extra check for non-public premain method. Something like should probably work:     if

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

On 24/06/2020 07:24, serguei.spit...@oracle.com wrote: : One approach would be to continue using the setAccessible and add extra check for non-public premain method. Something like should probably work:     if (!(Modifier.isPublic(m.getModifiers())) {     throw new

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

Hi David, On 6/23/20 22:50, David Holmes wrote: Hi Serguei, On 24/06/2020 3:37 pm, serguei.spit...@oracle.com wrote: 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

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

On 24/06/2020 06:50, David Holmes wrote: It sounds like the use of setAccessible was hiding the need to disable some module related access checks. This will have a much bigger compatibility problem if agents with a public premain suddenly stop working. I'm trying to understand what you

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

Hi Serguei, On 24/06/2020 3:37 pm, serguei.spit...@oracle.com wrote: 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

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

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

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

Please, hold on. The fix does not work for a number of the jdk_instrument tests. Thanks, Serguei On 6/23/20 19:05, serguei.spit...@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):  

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

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??? while we have a sample of agents that will not be affected there may be some agent that will fail terminally with this change just a thought

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

Thank you a lot, Sundar! Serguei On 6/23/20 19:53, sundararajan.athijegannat...@oracle.com wrote: Looks good -Sundar On 24/06/20 7:35 am, serguei.spit...@oracle.com wrote: Please, review a fix for:   https://bugs.openjdk.java.net/browse/JDK-8165276 CSR draft (one CSR reviewer is needed

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

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.spit...@oracle.com wrote: Please, review a fix for: https://bugs.openjdk.java.net/browse/JDK-8165276 CSR draft (one CSR reviewer

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

Hi Serguei, I'm glad that you have a patch for this. On 6/23/20 7:05 PM, serguei.spit...@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):  

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

Looks good -Sundar On 24/06/20 7:35 am, serguei.spit...@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 Webrev:

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

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 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/instr-setAccessible.1/ The