On Tue, 2 Mar 2021 12:55:04 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:
>> `attach` command on CLHSDB supports to attach live process (PID) and
>> coredump, but it cannot connect to debug server. CLHSDB does not have a
>> command to connect to debug server.
>>
>> Other jhsdb commands (jstack, jmap, etc...) can connect debug server via
>> `--connect` option, so CLHSDB should connect to it.
>>
>> After this change, you can connect to debug server with 'connect <hostname>'.
>
> Yasumasa Suenaga has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix comments
This pre-existing code in CLHSDB.java is a little concerning:
127 private void doUsage() {
128 System.out.println("Usage: java CLHSDB [[pid] |
[path-to-java-executable [path-to-corefile]] | help ]");
129 System.out.println(" pid: attach to
the process whose id is 'pid'");
130 System.out.println(" path-to-java-executable: Debug a
core file produced by this program");
131 System.out.println(" path-to-corefile: Debug this
corefile. The default is 'core'");
132 System.out.println(" If no arguments are specified, you can
select what to do from the GUI.\n");
133 HotSpotAgent.showUsage();
134 }
First concern is that you have not added "debug server" support to it. 2nd
concern is the "If no arguments are specified, you can select what to do from
the GUI" part. It looks like using the GUI you can launch the CLHSDB console,
although I can't find in the source where/how that is actually done. In any
case, i don't see any way to pass command arguments when you do that.
I'm just wondering how much of this CLHSDB support we want to keep beyond what
the clhsdb command needs. Do we still want to support invoking CLHSDB.main()
from the java command line?
src/jdk.hotspot.agent/doc/clhsdb.html line 39:
> 37: Available commands:
> 38: assert true | false <font color="red">turn on/off asserts in SA
> code</font>
> 39: attach pid | exec core | remote_server <font color="red">attach SA to
> a process, core, or remote debug server</font>
The `jhsdb` help says `jhsdb jstack --connect debugserver`, so maybe
`debug_server` would be better than `remote_server`. I think "remote debug
server" in the help text is fine.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CLHSDB.java line 53:
> 51: // and coreFilename is the pathname of a core file we are
> 52: // supposed to attach to.
> 53: // Finally, if remoteMachineName != null, we are supposed to
> connect to remote debug server.
Split this line so its length is more consistent with the previous lines.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CLHSDB.java line 125:
> 123: private String execPath;
> 124: private String coreFilename;
> 125: private String remoteMachineName;
I think `debugServerName` or `remoteDebugServerName` would be better.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CLHSDB.java line 230:
> 228: System.out.println("Connecting to debug server, please
> wait...");
> 229: agent.attach(remoteMachineName);
> 230: this.remoteMachineName = remoteMachineName;
I'm a little surprised to see that there is already some support for connecting
to a debug server in CLHSDB.java. Was it previously used from anywhere?
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/HSDB.java line 436:
> 434: // and coreFilename is the pathname of a core file we are
> 435: // supposed to attach to.
> 436: // Finally, if remoteMachineName != null, we are supposed to connect
> to remote debug server.
Split into 2 lines.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/HSDB.java line 84:
> 82: private String execPath;
> 83: private String coreFilename;
> 84: private String remoteMachineName;
`debugServerName` or `remoteDebugServerName`
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)?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2773
