Re: JVMTI retransformation and addition of private methods

[David Holmes]

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to 
reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those who 
were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix it in 11.
However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of allowing 
this. I _think_ it is safe but ...



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too strict 
in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 morphed 
into "Permit the adding or deleting of private final/static methods 
with redefine" - nor why those changes failed to make any change to 
the spec itself. It is also unclear whether the add/delete is 
restricted to final/static methods or any private method? I can see 
that the intent was to only allow something that would not perturb the 
vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the one 
in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to missing 
functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an alternate to 
the "native rebinding" technique that had been used previously. See the 
final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep with 
late attach"


which was closed as a duplicate.

David
-



Thanks,
Serguei



--
David



thanks,
Karen

Re: JVMTI retransformation and addition of private methods

[serguei.spit...@oracle.com]

  
  
Hi Karen and David,
  
  
  On 2/20/18 19:52, David Holmes wrote:

Hi
  Karen,
  
  
  On 21/02/2018 1:54 AM, Karen Kinnear wrote:
  
  Folks,


As part of the Valhalla EG discussions for JVMTI changes for
nestmates (thank you Serguei and David),

IBM brought up a request that we update the JVMTI documentation
to reflect that we allow addition

of private methods.


Is there a reason we do not document this? I’m inviting those
who were involved at the time - please include

others if needed.
  


I support documenting this in the JVMTI spec and had a plan to fix
it in 11.
However, it is not clear to me yet if we have a consensus on it.

This
  issue is tracked by:
  
  
  https://bugs.openjdk.java.net/browse/JDK-8192936
  
  
  "RI does not follow the JVMTI RedefineClasses spec that is too
  strict in the definition"
  


Yes, this is the one.
Thank you, David, for posting the link.


As I
  wrote there ... It is not at all clear how JDK-6404550 morphed
  into "Permit the adding or deleting of private final/static
  methods with redefine" - nor why those changes failed to make any
  change to the spec itself. It is also unclear whether the
  add/delete is restricted to final/static methods or any private
  method? I can see that the intent was to only allow something that
  would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the
one in the Bugtraq system?

The JDK-6404550 in JIRA has a different synopsis:
   https://bugs.openjdk.java.net/browse/JDK-6404550
 Cannot implement late attach in NetBeans Profiler due to
missing functionality in JVMTI


Thanks,
Serguei


--
  
  David
  
  
  
  thanks,

Karen


  


  

Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris

[serguei.spit...@oracle.com]

Hi David,


On 2/20/18 20:02, David Holmes wrote:

Hi Daniil,

Good find on this!

What does the actual spec say about the length of things and how they 
may be split across multiple packets? Are we guaranteed that at most 
two packets will be involved? What about for other things eg:


 68 protected byte[] readJdwpString(DataInputStream ds) throws 
IOException {

  69 byte[] str = null;
  70 int len = ds.readInt();
  71 if (len > 0) {
  72 str = new byte[len];
  73 ds.read(str, 0, len);
  74 }

might we get a short-read of the string if it is split across multiple 
packets?


Nice catch!
Even though this fix is enough to resolve this problem now, there is a 
chance,

it can fail in the future when more modules are added to the platform.


I'm wondering if all these reads should be loops, ensuring we read the 
expected amount of data.


One further comment - not sure why we need the print out for when we 
do read multiple packets?

That would seem to be a debugging aid.


Yes, it helps to understand what happens.
Many tests have a lack of tracing which makes it harder to debug and 
understand failures.



Thanks,
Serguei



Thanks,
David

On 21/02/2018 10:14 AM, Daniil Titov wrote:

Hi Serguei,

A new version of the webrev that has these strings reformatted is at 
http://cr.openjdk.java.net/~dtitov/8170541/webrev.02/


Thank you!

Best regards,

Daniil

*From: *"serguei.spit...@oracle.com" 
*Date: *Tuesday, February 20, 2018 at 3:00 PM
*To: *Daniil Titov , 
"serviceability-dev@openjdk.java.net" 

*Subject: *Re: RFR 8170541: 
serviceability/jdwp/AllModulesCommandTest.java fails intermittently 
on Windows and Solaris


Hi Daniil,

Interesting issue...
Thank you for finding to the root cause so quickly!

The fix looks good.
Could I ask you to reformat these lines to make the L54 shorter ?:

   54 System.out.println("[" + getClass().getName() + 
"] Only " + bytesRead + " bytes of " + dataLength +


   55 " were read in the first packet. 
Reading the rest...");


Thanks,
Serguei


On 2/20/18 09:24, Daniil Titov wrote:

    Please review the changes that fix intermittent failure of
    serviceability/jdwp/AllModulesCommandTest.java test.

    The problem here is that for a large data the JDWP agent
    (socketTransport_writePacket() method in
src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c )
    sends 2 packets and in some cases only the first packet is received
    at the time when the test reads the reply from the JDWP agent. Since
    the test does not check that all data is received in the first
    packet the correlation between commands and replies became broken
    (the unread second packet is read by the next command and the reply
    for the next command is read by the next after next command and 
so on).


    Bug: https://bugs.openjdk.java.net/browse/JDK-8170541

    Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01

    The tests ran successfully with Mach5.

    Best regards,

    Daniil

Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties

[Harsha Wardhana B]

On Wednesday 21 February 2018 01:51 AM, mandy chung wrote:

The code review and CSR review can be in parallel.  For this case,
I agree with Kumar to have CSR written that would help the code
review. Please specify the behavior and its relationship with
jcmd and other relevant diagnosability tools.

ok.


On 2/20/18 6:41 AM, Kumar Srinivasan wrote:


What is the behavior when -Dcom.sun.management.jmxremote.port=1234 
--start-management-agent port=2345 
-Dcom.sun.management.jmxremote.port=3456?


What is the value of the system property 
com.sun.management.jmxremote.port at runtime?  What port number 
does the management server start with?
As said earlier, values set via new flags override values set by -D 
flags. So 2345 will be the value of 
com.sun.management.jmxremote.port. Added a test case to validate that.


VM options are the last one wins if same option specified multiple 
times.  In this case, it could cause confusion (the last -D option 
sets the value to 3456 but it's set to a different value).


Why not taking the simplest approach - when --start-management-agent 
is set, it does not accept mixing the old way (i.e. does not accept 
the management properties to be set via -D)?   This RFE is to make the 
command-line simpler and ease-of-use.  I don't see any downside to 
migrate entirely to the new form.
We cannot get rid of specifying options via -D. We have plenty of -D 
flags but very few have short-hand alternative via 
--start-management-agent. If management properties are specified by 
--start-management-agent, the options specified by -D are anyway 
overwritten if specified.


Mandy

Harsha

Re: RFR : JDK-8196744 : JMX: Not enough JDP packets received before timeout

[Chris Plummer]

Hi Harsha,

Not a review, but just a request that you add the explanation of the 
problem to the CR so we have a record of it. Also, the copyright needs 
to be updated.


thanks,

Chris

On 2/20/18 3:30 AM, Harsha Wardhana B wrote:

Hi All,

Please find the fix below for the Jdp test-case.

issue: https://bugs.openjdk.java.net/browse/JDK-8196028
webrev : http://cr.openjdk.java.net/~hb/8196028/webrev.00/

Fix details : The test was receiving JDP packets from other VM and 
hence the multi-cast socket was not timing-out. The default timeout 
handler was causing test to fail. Added a shutdown method that passes 
the test in case of timeout.


Thanks
Harsha

Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris

[Daniil Titov]

Hi Serguei,

 

It is correct.  I was able to reproduce it locally and on Mach5. Before the fix 
the failure rate on Mach5 was about 15%.  After the fix I run it on Mach5 400 
times and no any failure was reported.

 

Best regards,

Daniil

 

 

From: "serguei.spit...@oracle.com" 
Date: Tuesday, February 20, 2018 at 7:31 PM
To: Daniil Titov , 
"serviceability-dev@openjdk.java.net" 
Subject: Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails 
intermittently on Windows and Solaris

 

Hi Daniil,

It looks good to me.

It seems, you have been able to reproduce this failure, and with this fix
the test does not fail anymore. Is that right?
Just wanted to be sure.

Thanks,
Serguei


On 2/20/18 16:14, Daniil Titov wrote:

Hi Serguei,

 

A new version of the webrev that has these strings reformatted is at 
http://cr.openjdk.java.net/~dtitov/8170541/webrev.02/

 

Thank you!

 

Best regards,

Daniil

 

 

 

From: "serguei.spit...@oracle.com" 
Date: Tuesday, February 20, 2018 at 3:00 PM
To: Daniil Titov , 
"serviceability-dev@openjdk.java.net" 
Subject: Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails 
intermittently on Windows and Solaris

 

Hi Daniil,

Interesting issue...
Thank you for finding to the root cause so quickly!

The fix looks good.
Could I ask you to reformat these lines to make the L54 shorter ?:
  54 System.out.println("[" + getClass().getName() + "] Only " 
+ bytesRead + " bytes of " + dataLength +
  55 " were read in the first packet. Reading the 
rest...");
Thanks,
Serguei


On 2/20/18 09:24, Daniil Titov wrote:
Please review the changes that fix intermittent failure of 
serviceability/jdwp/AllModulesCommandTest.java test.
 
The problem here is that for a large data the JDWP agent 
(socketTransport_writePacket() method in 
src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c ) sends 2 
packets and in some cases only the first packet is received at the time when 
the test reads the reply from the JDWP agent. Since the test does not check 
that all data is received in the first packet the correlation between commands 
and replies became broken (the unread second packet is read by the next command 
and the reply for the next command is read by the next after next command and 
so on).
 
Bug: https://bugs.openjdk.java.net/browse/JDK-8170541 
Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01 
 
The tests ran successfully with Mach5.
 
 
Best regards,
Daniil
 
 
 

Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris

[David Holmes]

Hi Daniil,

Good find on this!

What does the actual spec say about the length of things and how they 
may be split across multiple packets? Are we guaranteed that at most two 
packets will be involved? What about for other things eg:


 68 protected byte[] readJdwpString(DataInputStream ds) throws 
IOException {

  69 byte[] str = null;
  70 int len = ds.readInt();
  71 if (len > 0) {
  72 str = new byte[len];
  73 ds.read(str, 0, len);
  74 }

might we get a short-read of the string if it is split across multiple 
packets?


I'm wondering if all these reads should be loops, ensuring we read the 
expected amount of data.


One further comment - not sure why we need the print out for when we do 
read multiple packets? That would seem to be a debugging aid.


Thanks,
David

On 21/02/2018 10:14 AM, Daniil Titov wrote:

Hi Serguei,

A new version of the webrev that has these strings reformatted is at 
http://cr.openjdk.java.net/~dtitov/8170541/webrev.02/


Thank you!

Best regards,

Daniil

*From: *"serguei.spit...@oracle.com" 
*Date: *Tuesday, February 20, 2018 at 3:00 PM
*To: *Daniil Titov , 
"serviceability-dev@openjdk.java.net" 
*Subject: *Re: RFR 8170541: 
serviceability/jdwp/AllModulesCommandTest.java fails intermittently on 
Windows and Solaris


Hi Daniil,

Interesting issue...
Thank you for finding to the root cause so quickly!

The fix looks good.
Could I ask you to reformat these lines to make the L54 shorter ?:

   54 System.out.println("[" + getClass().getName() + "] 
Only " + bytesRead + " bytes of " + dataLength +


   55 " were read in the first packet. Reading 
the rest...");


Thanks,
Serguei


On 2/20/18 09:24, Daniil Titov wrote:

Please review the changes that fix intermittent failure of
serviceability/jdwp/AllModulesCommandTest.java test.

The problem here is that for a large data the JDWP agent
(socketTransport_writePacket() method in
src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c )
sends 2 packets and in some cases only the first packet is received
at the time when the test reads the reply from the JDWP agent. Since
the test does not check that all data is received in the first
packet the correlation between commands and replies became broken
(the unread second packet is read by the next command and the reply
for the next command is read by the next after next command and so on).

Bug: https://bugs.openjdk.java.net/browse/JDK-8170541

Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01

The tests ran successfully with Mach5.

Best regards,

Daniil

Re: JVMTI retransformation and addition of private methods

[David Holmes]

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for nestmates (thank 
you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to reflect that 
we allow addition
of private methods.

Is there a reason we do not document this? I’m inviting those who were involved 
at the time - please include
others if needed.


This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too strict in 
the definition"


As I wrote there ... It is not at all clear how JDK-6404550 morphed into 
"Permit the adding or deleting of private final/static methods with 
redefine" - nor why those changes failed to make any change to the spec 
itself. It is also unclear whether the add/delete is restricted to 
final/static methods or any private method? I can see that the intent 
was to only allow something that would not perturb the vtable for 
existing instances.


--
David



thanks,
Karen

Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris

[serguei.spit...@oracle.com]

  
  
Hi Daniil,
  
  It looks good to me.
  
  It seems, you have been able to reproduce this failure, and with
  this fix
  the test does not fail anymore. Is that right?
  Just wanted to be sure.
  
  Thanks,
  Serguei
  
  
  On 2/20/18 16:14, Daniil Titov wrote:


  
  
  
Hi Serguei,
 
A new version of the webrev that has these
  strings reformatted is at http://cr.openjdk.java.net/~dtitov/8170541/webrev.02/
 
Thank you!
 
Best regards,
Daniil
 
 
 

  From: "serguei.spit...@oracle.com"
  
  Date: Tuesday, February 20, 2018 at 3:00 PM
  To: Daniil Titov
  ,
  "serviceability-dev@openjdk.java.net"
  
  Subject: Re: RFR 8170541:
  serviceability/jdwp/AllModulesCommandTest.java fails
  intermittently on Windows and Solaris


   


  Hi Daniil,
  
  Interesting issue...
  Thank you for finding to the root cause so quickly!
  
  The fix looks good.
  Could I ask you to reformat these lines to make the L54
  shorter ?:
    54 System.out.println("[" + getClass().getName() + "] Only " + bytesRead + " bytes of " + dataLength +
    55 " were read in the first packet. Reading the rest...");
  Thanks,
  Serguei
  
  
  On 2/20/18 09:24, Daniil Titov wrote:


  Please review the changes that fix intermittent failure of serviceability/jdwp/AllModulesCommandTest.java test.
   
  The problem here is that for a large data the JDWP agent (socketTransport_writePacket() method in src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c ) sends 2 packets and in some cases only the first packet is received at the time when the test reads the reply from the JDWP agent. Since the test does not check that all data is received in the first packet the correlation between commands and replies became broken (the unread second packet is read by the next command and the reply for the next command is read by the next after next command and so on).
   
  Bug: https://bugs.openjdk.java.net/browse/JDK-8170541 
  Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01 
   
  The tests ran successfully with Mach5.
   
   
  Best regards,
  Daniil
   
   



  
  


  

Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

[mandy chung]

Hi Jeremy,

Another approach is to change ThreadInfoCompositeData::validateCompositeData
to validate the given CompositeData.   I also revised 
ThreadInfoCompositeData to

return the default value of the attributes, if not present.

This is the patch:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/webrev.00/

What do you think?
Mandy

On 2/20/18 4:46 PM, Jeremy Manson wrote:

(I dropped serviceability-dev from this thread by mistake! Oops!)

Okay. Here's the revised patch.  LMK if that's what you had in mind.

http://cr.openjdk.java.net/~jmanson/8198253/webrev.01/ 



On Fri, Feb 16, 2018 at 6:20 PM, mandy chung > wrote:


Hi Jeremy,

lockedMonitors and lockedSynchronizers attribute are not optional if that's
the issue you try to resolve. I think the specification should be clarified.

ThreadInfo::from supports the three different versions for interoperability:
1. CompositeData for JDK 1.5 ThreadInfo with no lockedMonitors and
lockedSynchronizers attribute
2. CompositeData for JDK 6 ThreadInfo with lockedMonitors and 
lockedSynchronizers
attributes
3. CompositeData for JDK 9 ThreadInfo with lockedMonitors and 
lockedSynchronizers
    attributes and with daemon and priority attribute.

JMX client can connect to a running VM in any version and get back a
proper ThreadInfo.

If ThreadInfo::from is called with a CompositeData containing daemon and
priority attribute but lockedMonitors and lockedSynchronizers attributes are
absent then the given CompositeData is invalid and IllegalArgumentException
should be thrown.

Does this help?

Mandy


On 2/15/18 4:46 PM, Jeremy Manson wrote:

Hi folks,

Been a long time!  I'd like someone to do a code review.  This is
in code I wrote a few years ago, and got wrong.  At the time,
David Holmes, Staffan Larsen, and Mandy Chung reviewed it.  It
does mean that people using ThreadInfo.from(CompositeData) may be
getting the wrong values out for ThreadInfo, so it is definitely
worth fixing.

The patch below fixes the bug, but is a pretty questionable
approach.  Better approaches happily considered.

Patch:
http://cr.openjdk.java.net/~jmanson/8198253/webrev.00/


Bug:
https://bugs.openjdk.java.net/browse/JDK-8198253


Thanks!

Jeremy

Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris

[Daniil Titov]

Hi Serguei,

 

A new version of the webrev that has these strings reformatted is at 
http://cr.openjdk.java.net/~dtitov/8170541/webrev.02/

 

Thank you!

 

Best regards,

Daniil

 

 

 

From: "serguei.spit...@oracle.com" 
Date: Tuesday, February 20, 2018 at 3:00 PM
To: Daniil Titov , 
"serviceability-dev@openjdk.java.net" 
Subject: Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails 
intermittently on Windows and Solaris

 

Hi Daniil,

Interesting issue...
Thank you for finding to the root cause so quickly!

The fix looks good.
Could I ask you to reformat these lines to make the L54 shorter ?:
  54 System.out.println("[" + getClass().getName() + "] Only " 
+ bytesRead + " bytes of " + dataLength +
  55 " were read in the first packet. Reading the 
rest...");
Thanks,
Serguei


On 2/20/18 09:24, Daniil Titov wrote:
Please review the changes that fix intermittent failure of 
serviceability/jdwp/AllModulesCommandTest.java test.
 
The problem here is that for a large data the JDWP agent 
(socketTransport_writePacket() method in 
src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c ) sends 2 
packets and in some cases only the first packet is received at the time when 
the test reads the reply from the JDWP agent. Since the test does not check 
that all data is received in the first packet the correlation between commands 
and replies became broken (the unread second packet is read by the next command 
and the reply for the next command is read by the next after next command and 
so on).
 
Bug: https://bugs.openjdk.java.net/browse/JDK-8170541 
Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01 
 
The tests ran successfully with Mach5.
 
 
Best regards,
Daniil
 
 

Re: RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris

[serguei.spit...@oracle.com]

  
  
Hi Daniil,
  
  Interesting issue...
  Thank you for finding to the root cause so quickly!
  
  The fix looks good.
  Could I ask you to reformat these lines to make the L54 shorter ?:
54 System.out.println("[" + getClass().getName() + "] Only " + bytesRead + " bytes of " + dataLength +
  55 " were read in the first packet. Reading the rest...");
  Thanks,
  Serguei
  
  
  On 2/20/18 09:24, Daniil Titov wrote:


  Please review the changes that fix intermittent failure of serviceability/jdwp/AllModulesCommandTest.java test.

The problem here is that for a large data the JDWP agent (socketTransport_writePacket() method in src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c ) sends 2 packets and in some cases only the first packet is received at the time when the test reads the reply from the JDWP agent. Since the test does not check that all data is received in the first packet the correlation between commands and replies became broken (the unread second packet is read by the next command and the reply for the next command is read by the next after next command and so on).

Bug: https://bugs.openjdk.java.net/browse/JDK-8170541 
Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01 

The tests ran successfully with Mach5.


Best regards,
Daniil





  

Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties

[mandy chung]

The code review and CSR review can be in parallel.  For this case,
I agree with Kumar to have CSR written that would help the code
review. Please specify the behavior and its relationship with
jcmd and other relevant diagnosability tools.


On 2/20/18 6:41 AM, Kumar Srinivasan wrote:


What is the behavior when -Dcom.sun.management.jmxremote.port=1234 
--start-management-agent port=2345 
-Dcom.sun.management.jmxremote.port=3456?


What is the value of the system property 
com.sun.management.jmxremote.port at runtime?  What port number 
does the management server start with?
As said earlier, values set via new flags override values set by -D 
flags. So 2345 will be the value of 
com.sun.management.jmxremote.port. Added a test case to validate that.


VM options are the last one wins if same option specified multiple 
times.  In this case, it could cause confusion (the last -D option sets 
the value to 3456 but it's set to a different value).


Why not taking the simplest approach - when --start-management-agent is 
set, it does not accept mixing the old way (i.e. does not accept the 
management properties to be set via -D)?   This RFE is to make the 
command-line simpler and ease-of-use.  I don't see any downside to 
migrate entirely to the new form.


Mandy

RFR 8170541: serviceability/jdwp/AllModulesCommandTest.java fails intermittently on Windows and Solaris

[Daniil Titov]

Please review the changes that fix intermittent failure of 
serviceability/jdwp/AllModulesCommandTest.java test.

The problem here is that for a large data the JDWP agent 
(socketTransport_writePacket() method in 
src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c ) sends 2 
packets and in some cases only the first packet is received at the time when 
the test reads the reply from the JDWP agent. Since the test does not check 
that all data is received in the first packet the correlation between commands 
and replies became broken (the unread second packet is read by the next command 
and the reply for the next command is read by the next after next command and 
so on).

Bug: https://bugs.openjdk.java.net/browse/JDK-8170541 
Webrev: http://cr.openjdk.java.net/~dtitov/8170541/webrev.01 

The tests ran successfully with Mach5.


Best regards,
Daniil

JVMTI retransformation and addition of private methods

[Karen Kinnear]

Folks,

As part of the Valhalla EG discussions for JVMTI changes for nestmates (thank 
you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to reflect that 
we allow addition
of private methods.

Is there a reason we do not document this? I’m inviting those who were involved 
at the time - please include
others if needed.

thanks,
Karen

Re: RFR : JDK-8196028 : JMX: Not enough JDP packets received before timeout

[Harsha Wardhana B]

[Correcting bug Id in the subject]

Thanks Erik.

Harsha

On Tuesday 20 February 2018 08:44 PM, Erik Gahlin wrote:

Looks OK.

Erik


Hi All,

Please find the fix below for the Jdp test-case.

issue: https://bugs.openjdk.java.net/browse/JDK-8196028
webrev : http://cr.openjdk.java.net/~hb/8196028/webrev.00/

Fix details : The test was receiving JDP packets from other VM and 
hence the multi-cast socket was not timing-out. The default timeout 
handler was causing test to fail. Added a shutdown method that passes 
the test in case of timeout.


Thanks
Harsha

Re: RFR : JDK-8196744 : JMX: Not enough JDP packets received before timeout

[Erik Gahlin]

Looks OK.

Erik


Hi All,

Please find the fix below for the Jdp test-case.

issue: https://bugs.openjdk.java.net/browse/JDK-8196028
webrev : http://cr.openjdk.java.net/~hb/8196028/webrev.00/

Fix details : The test was receiving JDP packets from other VM and 
hence the multi-cast socket was not timing-out. The default timeout 
handler was causing test to fail. Added a shutdown method that passes 
the test in case of timeout.


Thanks
Harsha

Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties

[Kumar Srinivasan]

You are going backwards. You need to have a CSR approved first.

Kumar


Ping. Could I please have reviews for below webrev.

Thanks
Harsha

On Wednesday 14 February 2018 09:59 PM, Harsha Wardhana B wrote:

Hi,

Below is the updated webrev.

http://cr.openjdk.java.net/~hb/8187498/webrev.03/


On Wednesday 14 February 2018 01:15 AM, mandy chung wrote:



On 2/13/18 1:30 AM, Harsha Wardhana B wrote:

Hi,

Please find below the revised webrev.

http://cr.openjdk.java.net/~hb/8187498/webrev.02/


On Friday 09 February 2018 05:07 AM, mandy chung wrote:

On 2/7/18 1:19 AM, Harsha Wardhana B wrote:
> > --start-management-agent will not be recognized in the current 
format and

> hence will not default to --start-management-agent=local=true.

`--start-local-management-server` is one option as Alan suggests.
Another option is to make `--start-management-agent` to accept
an optional argument.  VM can accept `--start-management-agent`
or `--start-management-agent=port=1234,ssl=on`.  It may require
more launcher change to support it.
Yes. It requires lot more changes to launcher. Hence optional 
argument to --start-management-agent was not added in order to keep 
the launcher impact minimum.


This is just one option for you to consider.   So the current 
proposal of the new option to start a remote management server, right?
Not necessarily. --start-management-agent=local=true starts local 
server and --start-management-agent=port=1234 starts remote 
management server.

Agent.java
  If --start-management-agent is set, -Dcom.sun.management.* takes
  precedence.  Do you really want to do that?  The new VM option
  intends to simplify the command-line to type in.  I think it's
  cleaner and reasonable if --start-management-agent is specified,
  -Dcom.sun.management.* are ignored (maybe worth emit a warning 
if set)
We can probably document that -D options will be overridden instead 
of emitting a warning.


What is the behavior when -Dcom.sun.management.jmxremote.port=1234 
--start-management-agent port=2345 
-Dcom.sun.management.jmxremote.port=3456?


What is the value of the system property 
com.sun.management.jmxremote.port at runtime?  What port number does 
the management server start with?
As said earlier, values set via new flags override values set by -D 
flags. So 2345 will be the value of 
com.sun.management.jmxremote.port. Added a test case to validate that.



  The implementation uses VMManagement::getVmArguments and scan
  the VM options for -start-management-agent.  The VM is the one
  invoking jdk.internal.agent.Agent::startAgent.  A simpler option
  is to change Agent::startAgent to take an argument parameter
  that will be passed with the argument of --start-management-agent
  or null if not set.

  Similarly for startRemoteManagementAgent and startLocalAgent_name.
Implementing this change requires lot of changes to argument 
parsing in native code and hence it is simpler to handle this at 
java layer.


Can you describe how --start-management-agent option will be used 
for jcmd and any other tool that attaches to a running VM to start 
the agent?  Example command-line will be useful.
--start-management-agent cannot be used by jcmd (or dynamic attach) 
as it accepts flags only in -D format or -D flags with 
com.sun.management removed. That code to parse string passed via jcmd 
was a mistake and hence I have removed it.



Mandy

-Harsha

Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties

[Harsha Wardhana B]

Ping. Could I please have reviews for below webrev.

Thanks
Harsha

On Wednesday 14 February 2018 09:59 PM, Harsha Wardhana B wrote:

Hi,

Below is the updated webrev.

http://cr.openjdk.java.net/~hb/8187498/webrev.03/


On Wednesday 14 February 2018 01:15 AM, mandy chung wrote:



On 2/13/18 1:30 AM, Harsha Wardhana B wrote:

Hi,

Please find below the revised webrev.

http://cr.openjdk.java.net/~hb/8187498/webrev.02/


On Friday 09 February 2018 05:07 AM, mandy chung wrote:

On 2/7/18 1:19 AM, Harsha Wardhana B wrote:
> 
> --start-management-agent will not be recognized in the current format and

> hence will not default to --start-management-agent=local=true.

`--start-local-management-server` is one option as Alan suggests.
Another option is to make `--start-management-agent` to accept
an optional argument.  VM can accept `--start-management-agent`
or `--start-management-agent=port=1234,ssl=on`.  It may require
more launcher change to support it.
Yes. It requires lot more changes to launcher. Hence optional 
argument to --start-management-agent was not added in order to keep 
the launcher impact minimum.


This is just one option for you to consider.   So the current 
proposal of the new option to start a remote management server, right?
Not necessarily. --start-management-agent=local=true starts local 
server and --start-management-agent=port=1234 starts remote management 
server.

Agent.java
  If --start-management-agent is set, -Dcom.sun.management.* takes
  precedence.  Do you really want to do that?  The new VM option
  intends to simplify the command-line to type in.  I think it's
  cleaner and reasonable if --start-management-agent is specified,
  -Dcom.sun.management.* are ignored (maybe worth emit a warning if set)
We can probably document that -D options will be overridden instead 
of emitting a warning.


What is the behavior when -Dcom.sun.management.jmxremote.port=1234 
--start-management-agent port=2345 
-Dcom.sun.management.jmxremote.port=3456?


What is the value of the system property 
com.sun.management.jmxremote.port at runtime?  What port number does 
the management server start with?
As said earlier, values set via new flags override values set by -D 
flags. So 2345 will be the value of com.sun.management.jmxremote.port. 
Added a test case to validate that.



  The implementation uses VMManagement::getVmArguments and scan
  the VM options for -start-management-agent.  The VM is the one
  invoking jdk.internal.agent.Agent::startAgent.  A simpler option
  is to change Agent::startAgent to take an argument parameter
  that will be passed with the argument of --start-management-agent
  or null if not set.

  Similarly for startRemoteManagementAgent and startLocalAgent_name.
Implementing this change requires lot of changes to argument parsing 
in native code and hence it is simpler to handle this at java layer.


Can you describe how --start-management-agent option will be used for 
jcmd and any other tool that attaches to a running VM to start the 
agent?  Example command-line will be useful.
--start-management-agent cannot be used by jcmd (or dynamic attach) as 
it accepts flags only in -D format or -D flags with com.sun.management 
removed. That code to parse string passed via jcmd was a mistake and 
hence I have removed it.



Mandy

-Harsha

RFR : JDK-8196744 : JMX: Not enough JDP packets received before timeout

[Harsha Wardhana B]

Hi All,

Please find the fix below for the Jdp test-case.

issue: https://bugs.openjdk.java.net/browse/JDK-8196028
webrev : http://cr.openjdk.java.net/~hb/8196028/webrev.00/

Fix details : The test was receiving JDP packets from other VM and hence 
the multi-cast socket was not timing-out. The default timeout handler 
was causing test to fail. Added a shutdown method that passes the test 
in case of timeout.


Thanks
Harsha