On Tue, 2 Mar 2021 11:39:10 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>>> @plummercj Ok, I try to add debug server support to `attach` clhsdb >>> command. Then should we still need CSR? >> >> Pushed new commit to be implemented as `attach`. It works fine with >> serviceability/sa jtreg tests on my Linux x64. >> I will add CSR if you are ok. > > Hi, > I ran the first version and agree it works, the connect command worked OK > when I ran a local, manual test. But I do like adding to the attach Command > rather than connect, avoiding introducing a new Command name. > > Assuming an int is a PID like in Tool.java is reasonable. > You can have a leading numeral since a certain RFC, but although I can't find > clear language to say it's definitely illegal, actualy using a fully numeric > hostname seems unwise. > > CLHSDB.java line 185 > Now that "private void attachDebugger(int pid)" takes an int, we don't want > to catch NumberFormatException, that try/catch block is not wanted. > > In the testcase, I think it will fail if a command throws an Exception, but > that is limiting what problems it can detect: > could we make the test check at least some of the key output to show that we > are attached and getting information, not errors or something unexpected? If > at least some key commands we issue are checked that they contain some output > indicating success that would be great. Thanks @kevinjwalls ! > CLHSDB.java line 185 > Now that "private void attachDebugger(int pid)" takes an int, we don't want > to catch NumberFormatException, that try/catch block is not wanted. I fixed it in new commit, and also I fixed HSDB.java. > In the testcase, I think it will fail if a command throws an Exception, but > that is limiting what problems it can detect: > could we make the test check at least some of the key output to show that we > are attached and getting information, not errors or something unexpected? If > at least some key commands we issue are checked that they contain some output > indicating success that would be great. I added calling `class java.lang.Object` and `class java.lang.String` in testcase. We can check whether the connection works fine through them. ------------- PR: https://git.openjdk.java.net/jdk/pull/2773
