Hi Yasumasa, Serguei and Alex,

Please review a new version of the fix [1] that addresses your comments. The 
new version in addition to RMI connector
port option introduces two more options to specify RMI registry port and RMI 
connector host name. Currently, these
last two settings could be specified using the system properties but the system 
properties have the following disadvantages
comparing to the command line options:
   -  It’s hard to know about them: they are not listed in tool’s help.
   -  They have long names that hard to remember
   -   It is easy to mistype them  in the command line and you will not get any 
warning about it.

The CSR [2] was also updated and needs to be reviewed.

Testing: Manual testing with attaching the debug server to the running Java 
process or to the core file inside a docker
container  and connecting  to it with the GUI debugger.  Mach5 tier1-tier3 
tests (that include serviceability/sa/sadebugd tests) succeeded.

[1] Webrev: http://cr.openjdk.java.net/~dtitov/8196751/webrev.02/
[2] CSR: https://bugs.openjdk.java.net/browse/JDK-8239831
[3] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8196751

Thank you,
Daniil

On 2/24/20, 5:45 AM, "Yasumasa Suenaga" <suen...@oss.nttdata.com> wrote:

    Hi Daniil,
    
       - SALauncher::buildAttachArgs is not only to build arguments but also to 
check consistency of arguments.
         Thus you should use buildAttachArgs() in runDEBUGD(). If you do so, 
runDEBUGD() would be more simply.
    
       - SADebugDTest::testWithPidAndRmiPort would retry until `--rmiport` can 
be used.
         But you can use same port number as RMI registry (1099).
         It is same as relation between jmxremote.port and jmxremote.rmi.port.
    
    
    Thanks,
    
    Yasumasa
    
    
    On 2020/02/24 13:21, Daniil Titov wrote:
    > Please review change that adds a new command line option to jhsdb tool 
for the debugd mode to specify a RMI connector port.
    > Currently a random port is used that prevents the debug server from being 
used behind a firewall or in a container.
    > 
    > New CSR [3] was created for this change and it needs to be reviewed as 
well.
    > 
    > Man pages for jhsdb will be updated in a separate issue.
    > 
    > The current implementation (sun.jvm.hotspot.SALauncher)  parses the 
command line options passed to jhsdb tool,
    > converts them to the ones for the debug server and then delegates the 
call  to sun.jvm.hotspot.DebugServer.main().
    > 
    >                // delegate to the actual SA debug server.
    >   367         DebugServer.main(newArgArray.toArray(new String[0]));
    > 
    > However,  sun.jvm.hotspot.DebugServer  doesn't support named options and 
that prevents from efficiently adding new options to the tool.
    > I found it more suitable to start Hotspot agent directly in  SALauncher 
rather than  adding a new option in  both sun.jvm.hotspot.SALauncher
    >   and sun.jvm.hotspot.DebugServer and  delegating the call.  With this 
change I think sun.jvm.hotspot.DebugServer could be marked as a deprecated
    > but I would prefer to address it in a separate issue.
    > 
    > Testing: Manual testing with attaching the debug server to the running 
Java process or to the core file inside a docker
    >                  container  and connecting  to it with the GUI debugger.
    >                 Mach5 tier1-tier3 tests (that include 
serviceability/sa/sadebugd tests) succeeded.
    > 
    > [1] Webrev: http://cr.openjdk.java.net/~dtitov/8196751/webrev.01
    > [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8196751
    > [3] CSR: https://bugs.openjdk.java.net/browse/JDK-8239831
    > 
    > Thank you,
    > Daniil
    > 
    > 
    


Reply via email to