On Wed, 6 May 2026 21:04:48 GMT, Kevin Walls <[email protected]> wrote:

>> This implements "jcmd on core files" for Linux, and for MiniDumps on Windows 
>> (MacOS is "future work").
>> jcmd "revives" the VM memory and .so/.dll from the core/minidump, and runs 
>> the existing native diagnostic command parser and command implementations.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Kevin Walls has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   TableStatistics not static, remove Windows special case

src/jdk.attach/share/classes/com/sun/tools/attach/VirtualMachine.java line 214:

> 212:      *
> 213:      * Details as per the {@link attach(String)} method.
> 214:      * This method additionally accepts a Map of named parameters and 
> values.

I assume "Details as per the .." is just temporary. It might be better to move 
the text from the 1-arg attach method to the 2-arg method, then have the 1-arg 
method be specified in terms of the 2-arg method. You'll see examples of this 
all over the JDK when the overloads with fewer parameters are specified in 
terms of the overload with all the parameters.

src/jdk.attach/share/classes/com/sun/tools/attach/VirtualMachine.java line 220:

> 218:      *
> 219:      * @param   env
> 220:      *          A Map of provider-specific settings to configure the 
> attach, may be null or empty.

What is the reason for allowing null here?

src/jdk.attach/share/classes/com/sun/tools/attach/VirtualMachine.java line 224:

> 222:      * @return  A VirtualMachine representing the target VM.
> 223:      *
> 224:      * @implNote The Oracle JDK ships with an attach provider which 
> recognises the {@code id} as a

I think you should speak about the HotSpot provider, not "Oracle JDK".

src/jdk.attach/share/classes/com/sun/tools/attach/spi/AttachProvider.java line 
144:

> 142:      * Attach to a Java virtual machine.
> 143:      *
> 144:      * Details as per the {@link attachVirtualMachine(String)} method.

"Details as per ..." is too casual and I assume temporary. 

As per the API, I don't think env should allowed to be null. Additionally, if 
env is empty then this method can invoke the 1-arg attachVirtualMachine method. 
This will require an implSpec to document this for implementers.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/31011#discussion_r3200677486
PR Review Comment: https://git.openjdk.org/jdk/pull/31011#discussion_r3200707453
PR Review Comment: https://git.openjdk.org/jdk/pull/31011#discussion_r3200711934
PR Review Comment: https://git.openjdk.org/jdk/pull/31011#discussion_r3200729199

Reply via email to