OpenJ9 Attach API Diagnostic extension #2543

[d0k1]

To implement command line diagnostic utilities like jstack / jmap / etc I decided to use Attact API just as jstack does. Attach API extension contains 8 new commands. Among them, there are commands to get javadump/systemdump/heapdump/snapdump and to get Information about memory, about loaded/unloaded classes about JIT compilation time. There is one command to get a thread dump through com.ibm.lang.management.internal.ExtendedThreadMXBeanImpl. The communication between JVM and Attach API client is going through IPC mechanism. Some commands have to send back some data to a client so they use java serialization to serialize a response and send it. Besides there are some new methods in com.ibm.tools.attach.attacher.OpenJ9VirtualMachine that uses the extension. #2543

Fixes #2543

Signed-off-by: Denis Kirpichenkov denis.kirpichenkov@gmail.com

[DanHeidinga]

Thanks for opening this PR, @d0k1!

Have you signed the Eclipse Contributor Agreement? As an Eclipse project, all contributors need to sign the ECA and have their commits pass the ip-validation check by using signed commits (ie: git commit -s)

Can you confirm you've signed the ECA and update the commit message to include the correct Signed-off by: tag?

Also, is there an existing issue that describes the approach taken to adding these extensions? If there is, can you link to it from this item? Any additional description of the design will help when reviewing the PR.

Thanks again!

[DanHeidinga]

@pdbain-ibm Can you take a look at the highlevel approach taken here and comment on how well it integrates into the existing attach api code?

[pdbain-ibm]

Dan

Will do.

@d0k1 As you can appreciate, security is an issue with the JVM and particularly attachAPI. Would you kindly indicate what steps you have taken to avoid compromising security. Thank you.

[DanHeidinga]

Please create a new class which extends OpenJ9VirtualMachine and move the new methods to same.

@pdbain-ibm What benefit does this bring?

[pdbain-ibm]

a) it keeps the OpenJ9VirtualMachine API congruent with the com.sun implementation, which has been a design goal from the start of the attach API project.
b) it separates the core attach API methods from the extensions thereto.

[d0k1]

@pdbain-ibm You asked about steps I took to avoid compromising security. Honestly, I did nothing. This PR is just a prototype of extended Attach API.
I would glad to hear advice about security check I need to implement.
May be I should do the same security check as Dump class (jcl/src/openj9.jvm/share/classes/com/ibm/jvm/Dump.java) does in it's methods? May be I should check if current user that tries to make a dump is the same who started a JVM?

[AdamBrousseau]

Jenkins line endings check

[pshipton]

@d0k1 why close this pull request? Have you decided not to pursue this?

[d0k1]

@pshipton Unfortunately adaptation of OpenJ9 has been stopped in project I'm involved in. I had no time to continue working on this issue, so I decided to close this pull request to keep repository clean of this unfinished work

[pshipton]

@d0k1 if you are able to share any details of why you decided to stop using OpenJ9, can you please let me know, either here or by direct email. I'm wondering if there are any specific areas we should be focusing on to improve OpenJ9.

[thjaeckle]

Very sad to not have that included in OpenJ9 - we would need to use the same (e.g. using jmap in order to get a heapdump from a running JVM).

[DanHeidinga]

@d0k1 Thanks for creating this pull request.

If you have no objections, we'd like to reopen this PR with goal of getting it merged. If you're interested in helping with that, great! If not, we'll take on the work required to get this through reviews and into OpenJ9.

[pdbain-ibm]

This will also need test cases.

[SueChaplain]

@pdbain-ibm - does this require any user documentation please? If so, please add the doc:externals label and open an issue at the docs repo: https://github.com/eclipse/openj9-docs/issues/new?template=new-documentation-change.md

[pshipton]

Not sure we'll be merging this particular PR as-is, but I've added a depends:doc tag for tracking anyway.

[d0k1]

@DanHeidinga Thanks for interest in this PR. Of course you can use it freely, I have no objections at all.
I'm just curios what about some concerns @pdbain-ibm had written about?
I'd like to add that, this API extension is just an extension, and OpenJ9 at the moment is still missing utilities that would use it. Is there any plans to implements such utilities?

I'd say that it's great to continue working on this PR, but I don't know where to start at the moment.

[DanHeidinga]

Thanks @d0k1. Re: @pdbain-ibm's concerns, they can be handled in review - validating that appropriate security concerns have been identified and mitigated.

[pshipton]

As this is being implemented by other PRs, closing this one.