RFR: 8262520: Add CLHSDB command to connect to debug server [v3]
Chris Plummer
cjplummer at openjdk.java.net
Wed Mar 3 22:22:57 UTC 2021
On Wed, 3 Mar 2021 02:32:04 GMT, Yasumasa Suenaga <ysuenaga at openjdk.org> wrote:
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/HSDB.java line 100:
>>
>>> 98: execPath = null;
>>> 99: coreFilename = null;
>>> 100: remoteMachineName = null;
>>
>> In general it's not clear to me why HSDB.java needed to be modified in a way similar to CLHSDB.java. If the goal is really just to add debug server support to the `clshdb attach` command, why is HSDB involved in that? Or is this adding debug server support elsewhere also (in which case it's not clear to me where this exposed to the user)?
>
> As I said in above, CLHSDB.java and HSDB.java are similar, and also I added `attach(String debugServerName)` to `CommandProcessor$DebuggerInterface` which is used in HSDB.java, so I changed HSDB.java even though it does not affect to HSDB.
>
> We can implement `attach(String debugServerName)` as empty method, and it might be reasonable. What do you think?
I see now. Both HSDB and CLHSDB implement CommandProcessor$DebuggerInterface, and they pass an instance of this class to the CommandProcessor constructor, and that allows CommandProcessor to do callbacks for the attach (or connect). Given this, it looks like you do want to add support for all 3 attach modes to the HSDB CommandProcessor$DebuggerInterface implementation. Otherwise you won't be able to attach to a debug server when you bring up the command line support using the HSDB gui.
What's been confusing, and I'm finally starting to understand, is that CommandProcessor is the SA command line support, and clhsdb (CLHSDB) and hsdb (HSDB) both use CommandProcessor to provide the command line support to the user. Maybe you should re-purpose this CR/PR as "Add SA CommandProcessor support to connect to debug server", and make sure the CR talks about how this impacts both clhsdb and hsdb.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2773
More information about the serviceability-dev
mailing list