Thanks a lot for review, Mandy!
I also, asked Leonid to look if the test changes can be simplified
Thanks,
Serguei
On 6/29/20 09:46, Mandy Chung wrote:
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 look
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
grammatically
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:
//
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:
//
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:
http://cr.openjdk.java.net/~sspitsyn/webrev
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 (St
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
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 th
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
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 agent
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 reje
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 i
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 "ma
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: c
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
(
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
c
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 issue
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
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,
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 wo
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 o
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 m
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 (!(Modifier.isPublic(m.getModifiers()))
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 IllegalAccess
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 p
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 me
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
w
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
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):
htt
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
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 bef
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 i
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):
https://bugs.openjdk.java.net/browse/JDK-82481
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:
http://cr.openjdk.ja
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 java.lang.instrume
35 matches
Mail list logo