Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-26 Thread Daniil Titov
Hi Yasumasa and Serguei, Thank you for reviewing this change. Best regards, --Daniil On 3/25/20, 1:01 PM, "serguei.spit...@oracle.com" wrote: Hi Daniil, On 3/24/20 10:00, Daniil Titov wrote: > Hi Serguei, > >> It looks like you removed the last call site of Debug

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-25 Thread serguei.spit...@oracle.com
Hi Daniil, On 3/24/20 10:00, Daniil Titov wrote: Hi Serguei, It looks like you removed the last call site of DebugServer.main. Yes. It is correct. Do we need to remove the DebugServer.java as well? I was considering this but since it is a public class I think it needs to be deprec

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-24 Thread Daniil Titov
Hi Serguei, >It looks like you removed the last call site of DebugServer.main. Yes. It is correct. >Do we need to remove the DebugServer.java as well? I was considering this but since it is a public class I think it needs to be deprecated first. I also think that it would be better to d

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-23 Thread serguei.spit...@oracle.com
Hi Daniil, It looks pretty good in general. It looks like you removed the last call site of DebugServer.main. Do we need to remove the DebugServer.java as well? Thanks, Serguei On 3/22/20 15:29, Daniil Titov wrote: Hi Yasumasa, Serguei and Alex, Please review a new version of the webrev tha

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-22 Thread Yasumasa Suenaga
Hi Daniil, Looks good! Yasumasa On 2020/03/23 7:29, Daniil Titov wrote: Hi Yasumasa, Serguei and Alex, Please review a new version of the webrev that merges SADebugDTest.java with changes done in [2]. Also the CRS [3] and the help message for debug server in SALauncher.java were updat

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-22 Thread Daniil Titov
Hi Yasumasa, Serguei and Alex, Please review a new version of the webrev that merges SADebugDTest.java with changes done in [2]. Also the CRS [3] and the help message for debug server in SALauncher.java were updated to specify that '--hostname' option could be a hostname or an IPv4/IPv6 ad

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-16 Thread serguei.spit...@oracle.com
Hi Daniil, The update looks pretty good to me so far. I'll make another pass tomorrow. Thanks, Serguei On 3/13/20 15:05, Daniil Titov wrote: Hi Yasumasa, Serguei and Alex, Please review a new version of the webrev that includes the changes Yasumasa suggested. Shutdown hook is already regi

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-13 Thread Yasumasa Suenaga
Hi Daniil, On 2020/03/14 7:05, Daniil Titov wrote: Hi Yasumasa, Serguei and Alex, Please review a new version of the webrev that includes the changes Yasumasa suggested. Shutdown hook is already registered in c'tor of HotSpotAgent. It works same as shutdownServer(), so I think shutdown h

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-13 Thread Daniil Titov
Hi Yasumasa, Serguei and Alex, Please review a new version of the webrev that includes the changes Yasumasa suggested. > Shutdown hook is already registered in c'tor of HotSpotAgent. >It works same as shutdownServer(), so I think shutdown hook at SALauncher > is not needed. The shutdown ho

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-06 Thread Yasumasa Suenaga
Hi Daniil, On 2020/03/07 3:38, Daniil Titov wrote: Hi Yasumasa, -> checkBasicOptions() is needed? I think you can remove this method and embed it in caller. I think that having a piece of code that invokes a method named "buildAttachArgs" with a copy of the argument map just for its side

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-03-06 Thread Daniil Titov
Hi Yasumasa, -> checkBasicOptions() is needed? I think you can remove this method and embed it in caller. I think that having a piece of code that invokes a method named "buildAttachArgs" with a copy of the argument map just for its side-effect ( it throws an exception if parameters are inc

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-02-25 Thread serguei . spitsyn
Hi Daniil, Okay, thanks! Serguei On 2/25/20 11:38 AM, Daniil Titov wrote: Hi Serguei, I will update the CSR and the fix to include this change. Thank you, Daniil On 2/25/20, 11:07 AM, "serguei.spit...@oracle.com" wrote: Hi Daniil, Thank you for reply. I agree with

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-02-25 Thread Daniil Titov
Hi Serguei, I will update the CSR and the fix to include this change. Thank you, Daniil On 2/25/20, 11:07 AM, "serguei.spit...@oracle.com" wrote: Hi Daniil, Thank you for reply. I agree with the approach to avoid using system properties. Then it is better to be consisten

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-02-25 Thread serguei . spitsyn
Hi Daniil, Thank you for reply. I agree with the approach to avoid using system properties. Then it is better to be consistent. I'd consider adding an RMI registry port option as well. Will look at your comments in the CSR and reply there. Thanks, Serguei On 2/25/20 10:05 AM, Daniil Titov wrot

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-02-25 Thread Daniil Titov
Hi Serguei, I added my comments there. In brief, I believe that in long term in the serviceability tools we should avoid using the system properties and prefer the command line options instead. Thanks, Daniil On 2/24/20, 11:04 AM, "serguei.spit...@oracle.com" wrote: Hi Daniil,

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-02-24 Thread serguei.spit...@oracle.com
Hi Daniil, I've looked at CSR and posted a couple of questions there. It'd be nice if you help to resolve my confusion. :) Thanks, Serguei On 2/23/20 20: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 connect

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-02-24 Thread Yasumasa Suenaga
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`

RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

2020-02-23 Thread Daniil Titov
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