Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v4]
On Wed, 12 Jun 2024 14:55:31 GMT, Daniel Fuchs wrote: >> Hmm I may have fixed that since changing the policy files, as I'm not seeing >> the problem without that AuthPermission any more. Am just retesting >> everything before updating this... > > (Same with other policy files in which the permission was added of course) Yes these no longer seem needed. I added them in response to failures in an earlier version of the change, thanks for spotting this, I've undone the policy changes. - PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1636753776
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v6]
> JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Undo test policy updates - Changes: - all: https://git.openjdk.org/jdk/pull/19624/files - new: https://git.openjdk.org/jdk/pull/19624/files/9664bef6..6f072338 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19624=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19624=04-05 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19624/head:pull/19624 PR: https://git.openjdk.org/jdk/pull/19624
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v5]
> JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Undo test policy updates - Changes: - all: https://git.openjdk.org/jdk/pull/19624/files - new: https://git.openjdk.org/jdk/pull/19624/files/422011e4..9664bef6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19624=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19624=03-04 Stats: 5 lines in 3 files changed: 0 ins; 5 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19624/head:pull/19624 PR: https://git.openjdk.org/jdk/pull/19624
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v4]
On Wed, 12 Jun 2024 14:31:26 GMT, Alan Bateman wrote: >> test/jdk/javax/management/remote/mandatory/notif/policy.negative line 7: >> >>> 5: permission javax.management.MBeanPermission >>> "[domain:type=NB,name=2]", "addNotificationListener"; >>> 6: permission javax.management.MBeanPermission "*", >>> "removeNotificationListener"; >>> 7: permission javax.security.auth.AuthPermission "doAs"; >> >> I suspect that this means a doPrivileged is missing somewhere. We should not >> require the application to posess new permissions. > > I think Daniel is right, can you remove this permission and paste in the > debug output to see where this is happening? Hmm I may have fixed that since changing the policy files, as I'm not seeing the problem without that AuthPermission any more. Am just retesting everything before updating this... - PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1636613493
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]
On Tue, 11 Jun 2024 20:31:03 GMT, Sean Mullan wrote:
>> src/java.management/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java
>> line 350:
>>
>>> 348: @SuppressWarnings("removal")
>>> 349: private Subject getSubject() {
>>> 350: Subject subject = null;
>>
>> Just call `Subject.current()`. When SM is allowed, it is equivalent to
>> `getSubject(AccessController.getContext())`.
>
> For the same reason you should be able to just call `Subject.current` in the
> tests. I don't think you need the `SimpleStandard.useGetSubjectACC` property
> to toggle the testing of either old or replacement APIs as long as the test
> has runs in both SM allow and disallow (i.e., no SM prop set) modes. Would
> you agree Weijun?
I thought there was a comment elsewhere suggesting using SM=allow to test with
Subject.getSubject() and Subject.current(), and when SM not allowed, to only
use Subject.current() ?
That was the reason for the property in the tests.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1636536113
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v4]
> JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: udpates - Changes: - all: https://git.openjdk.org/jdk/pull/19624/files - new: https://git.openjdk.org/jdk/pull/19624/files/56f9111e..422011e4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19624=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19624=02-03 Stats: 35 lines in 2 files changed: 6 ins; 23 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19624/head:pull/19624 PR: https://git.openjdk.org/jdk/pull/19624
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]
On Tue, 11 Jun 2024 16:55:44 GMT, Weijun Wang wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Sean comments
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
> line 1633:
>
>> 1631: }
>> 1632: } else {
>> 1633: // ACC is present, we have a Subject and SM is
>> permitted:
>
> While extract the `action` variable? The old code on lines 1590-1592 has no
> problem.
OK I can make this more like the original.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1636515151
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]
On Tue, 11 Jun 2024 16:53:09 GMT, Weijun Wang wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Sean comments
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
> line 1438:
>
>> 1436: return
>> AccessController.doPrivileged((PrivilegedExceptionAction) () ->
>> Subject.doAs(subject, op), acc);
>> 1437: }
>> 1438: } catch (PrivilegedActionException pe) {
>
> What is this catch block for? The cause of a `PrivilegedActionException`
> should not be a unchecked exception.
Looks like the catch of PrivilegedActionException is no longer necessary, will
be removed.
Was from from previously using a different doAs or call which needed it.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1636417022
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]
On Tue, 11 Jun 2024 20:58:54 GMT, Sean Mullan wrote:
>> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
>> line 1436:
>>
>>> 1434: } else {
>>> 1435: // ACC is present, we have a Subject and SM is
>>> permitted:
>>> 1436: return
>>> AccessController.doPrivileged((PrivilegedExceptionAction) () ->
>>> Subject.doAs(subject, op), acc);
>>
>> Why is it necessary to call both `doAs` and `doPrivileged`?
>
> Can you just call `AccessController.doPrivileged(op, acc)` as in the original
> code? `Subject.doAs` requires a permission, which is why I assume you are
> wrapping it in a `doPrivileged` but you are asserting all of the permissions
> of the `java.management` module, which means you probably want to use limited
> doPriv and only assert the `getSubject` permission but that is getting
> complicated. I'd go back to the original call as it should work in the SM
> allow case.
Yes, this can be simpler, thanks.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1636417433
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]
On Tue, 11 Jun 2024 16:59:31 GMT, Weijun Wang wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Sean comments
>
> src/java.management/share/classes/javax/management/monitor/Monitor.java line
> 1541:
>
>> 1539: if (ac == null) {
>> 1540: // No SecurityManager:
>> 1541: Subject.doAs(s, action);
>
> What if `s` is null?
Subject.doAs accepts a null Subject, so that's OK?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1636324060
Re: RFR: 8327793: Deprecate jstatd for removal [v4]
> jstatd is an RMI server application which monitors HotSpot VMs, and provides > an interface to the monitoring tool jstat, for use across a remote RMI > connection. > > RMI is not how modern applications communicate. It is an old transport with > long term security concerns, and configuration difficulties with firewalls. > > The jstatd tool should be removed. Deprecating and removing jstatd will not > affect usage of jstat for monitoring local VMs using the Attach API. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Remove annotations - Changes: - all: https://git.openjdk.org/jdk/pull/19658/files - new: https://git.openjdk.org/jdk/pull/19658/files/58892d76..ed386505 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19658=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19658=02-03 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19658/head:pull/19658 PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]
On Tue, 11 Jun 2024 17:03:58 GMT, Weijun Wang wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Sean comments
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
> line 1301:
>
>> 1299: }
>> 1300: };
>> 1301: if (acc == null) {
>
> This is a comment to all the code changes in this PR. I would recommend we
> make use of the `allowSecurityManager()` call everywhere when the behavior is
> different, like this:
>
> if (SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
> if (acc == null)
> return action.run();
> else
> return AccessController.doPrivileged(action, acc);
> } else {
> if (subject == null)
> return action.run();
> else
> return Subject.doAs(subject, action);
> }
>
> This makes me much more confident.
Thanks I'll go through the above comments and update - some of the changes I
see are unnecessary and from when I was trying migrating to callAs, and
doPrivileged, and yes they can be simpler.
On the allowSecurityMananger check:
If we check in e.g. RMIConnectionImpl constructor, and if SM is allowed we
assign an ACC.
Later, we know if we have a non-null ACC then SM is allowed. Are you saying we
should then check allowSecurityManager again?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1635291389
Re: RFR: 8327793: Deprecate jstatd for removal [v3]
> jstatd is an RMI server application which monitors HotSpot VMs, and provides > an interface to the monitoring tool jstat, for use across a remote RMI > connection. > > RMI is not how modern applications communicate. It is an old transport with > long term security concerns, and configuration difficulties with firewalls. > > The jstatd tool should be removed. Deprecating and removing jstatd will not > affect usage of jstat for monitoring local VMs using the Attach API. Kevin Walls has updated the pull request incrementally with two additional commits since the last revision: - Replace (C) - Remove annotations - Changes: - all: https://git.openjdk.org/jdk/pull/19658/files - new: https://git.openjdk.org/jdk/pull/19658/files/72ac8818..58892d76 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19658=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19658=01-02 Stats: 24 lines in 9 files changed: 0 ins; 16 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/19658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19658/head:pull/19658 PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8327793: Deprecate jstatd for removal [v2]
On Tue, 11 Jun 2024 16:54:36 GMT, Kevin Walls wrote: >> jstatd is an RMI server application which monitors HotSpot VMs, and provides >> an interface to the monitoring tool jstat, for use across a remote RMI >> connection. >> >> RMI is not how modern applications communicate. It is an old transport with >> long term security concerns, and configuration difficulties with firewalls. >> >> The jstatd tool should be removed. Deprecating and removing jstatd will not >> affect usage of jstat for monitoring local VMs using the Attach API. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > No annotations for src/jdk.jstatd/share/classes/sun/jvmstat/monitor/remote OK sure - I had thought the src annotations were useful but if not needed I am taking them out. Yes, I was thinking to do a separate man page CR. - PR Comment: https://git.openjdk.org/jdk/pull/19658#issuecomment-2161244463
Re: RFR: 8327793: Deprecate jstatd for removal
On Tue, 11 Jun 2024 13:35:19 GMT, Alan Bateman wrote: > The sun.jvmstat.monitor.remote package is not exported so I don't think > adding `@Deprecated` makes sense. Sure, happy to not add annotations in sun.jvmstat.monitor.remote (RemoteHost.java, RemoteVm.java). - PR Comment: https://git.openjdk.org/jdk/pull/19658#issuecomment-2161212800
Re: RFR: 8327793: Deprecate jstatd for removal [v2]
> jstatd is an RMI server application which monitors HotSpot VMs, and provides > an interface to the monitoring tool jstat, for use across a remote RMI > connection. > > RMI is not how modern applications communicate. It is an old transport with > long term security concerns, and configuration difficulties with firewalls. > > The jstatd tool should be removed. Deprecating and removing jstatd will not > affect usage of jstat for monitoring local VMs using the Attach API. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: No annotations for src/jdk.jstatd/share/classes/sun/jvmstat/monitor/remote - Changes: - all: https://git.openjdk.org/jdk/pull/19658/files - new: https://git.openjdk.org/jdk/pull/19658/files/60bde969..72ac8818 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19658=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19658=00-01 Stats: 5 lines in 2 files changed: 0 ins; 3 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19658/head:pull/19658 PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v2]
On Tue, 11 Jun 2024 14:02:17 GMT, Sean Mullan wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> More consistent style of calls and comments.
>
> test/jdk/javax/management/remote/mandatory/notif/NotificationEmissionTest.java
> line 36:
>
>> 34: * @run main NotificationEmissionTest 1
>> 35:
>> 36: * @run main/othervm NotificationEmissionTest 1
>
> Lines 34 & 36: is it necessary to run NotificationEmissionTest twice with and
> w/o `othervm`?
It's unnecessary, thanks.
> test/jdk/javax/management/remote/mandatory/passwordAuthenticator/SimpleStandard.java
> line 155:
>
>> 153: */
>> 154: private void checkSubject() {
>> 155: Subject subject =
>> Boolean.getBoolean("SimpleStandard.useGetSubjectACC") ?
>
> Nit, remove extra leading space.
got it
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1635157731
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1635157877
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]
> JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Sean comments - Changes: - all: https://git.openjdk.org/jdk/pull/19624/files - new: https://git.openjdk.org/jdk/pull/19624/files/d43f9a34..56f9111e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19624=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19624=01-02 Stats: 5 lines in 2 files changed: 1 ins; 2 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19624/head:pull/19624 PR: https://git.openjdk.org/jdk/pull/19624
RFR: 8327793: Deprecate jstatd for removal
jstatd is an RMI server application which monitors HotSpot VMs, and provides an interface to the monitoring tool jstat, for use across a remote RMI connection. RMI is not how modern applications communicate. It is an old transport with long term security concerns, and configuration difficulties with firewalls. The jstatd tool should be removed. Deprecating and removing jstatd will not affect usage of jstat for monitoring local VMs using the Attach API. - Commit messages: - Remove tmp file - Annotations - Annotations - Merge remote-tracking branch 'upstream/master' into 8327793_jstatd_deprecate - Basic deprecation of module, jstatd prints warning. Changes: https://git.openjdk.org/jdk/pull/19658/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19658=00 Issue: https://bugs.openjdk.org/browse/JDK-8327793 Stats: 35 lines in 12 files changed: 23 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/19658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19658/head:pull/19658 PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8322811: jcmd System.dump_map help info has conflicting statements
On Mon, 10 Jun 2024 17:08:58 GMT, Chris Plummer wrote:
>> @dholmes-ora this is one of yours.
>>
>> This was a tad annoying to fix (fix is simple though), since the jcmd
>> framework has no good way to allow for default parameters that are not used
>> literally. E.g. in this case, the real value for the file name will contain
>> the process pid, which of course cannot be hard-coded.
>>
>> New output:
>>
>>
>> Syntax : System.dump_map [options]
>>
>> Options: (options must be specified using the or = syntax)
>> -H : [optional] Human readable format (BOOLEAN, false)
>> -F : [optional] file path (STRING, vm_memory_map_.txt)
>
> src/hotspot/share/services/diagnosticCommand.cpp line 1211:
>
>> 1209: } else {
>> 1210: name = _filename.value();
>> 1211: }
>
> This code should be considering if a default it specified or not, in case the
> specified value is identical to what is contained in `default_filename`.
> With the current solution, if the user literally specifies
> vm_memory_map_.txt, the part will be expanded to the actual pid.
> See how [JDK-8323546](https://bugs.openjdk.org/browse/JDK-8323546) handled
> this.
Oops yes should definitely be using _filename.is_set() rather than strcmp.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19596#discussion_r1634365367
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v2]
> JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: More consistent style of calls and comments. - Changes: - all: https://git.openjdk.org/jdk/pull/19624/files - new: https://git.openjdk.org/jdk/pull/19624/files/22424a8e..d43f9a34 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19624=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19624=00-01 Stats: 12 lines in 1 file changed: 5 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/19624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19624/head:pull/19624 PR: https://git.openjdk.org/jdk/pull/19624
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed
On Mon, 10 Jun 2024 14:28:25 GMT, Alan Bateman wrote:
>> JMX uses APIs related to the Security Mananger which are deprecated. Use of
>> AccessControlContext will be removed when Security Manager is removed.
>>
>> Until then, updates are needed to not require setting
>> -Djava.security.manager=allow to use JMX authentication.
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
> line 1634:
>
>> 1632: } else {
>> 1633: // We have ACC therefore SM is permitted, and
>> Subject must be non-null:
>> 1634: return Subject.doAs(subject,
>> (PrivilegedExceptionAction) () ->
>> AccessController.doPrivileged((PrivilegedExceptionAction) () ->
>> wrappedClass.cast(mo.get()), acc));
>
> This is not readable. If you create the PAEs explicitly then the casts will
> go away and it will be much easier to read.
Yes will simplify this!
This style was actually from the idea of moving to Subject.callAs, and nesting
calls to keep the doPrivileged, but I don't think it will be necessary.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1633455244
Re: RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed
On Mon, 10 Jun 2024 11:28:28 GMT, Kevin Walls wrote: > JMX uses APIs related to the Security Mananger which are deprecated. Use of > AccessControlContext will be removed when Security Manager is removed. > > Until then, updates are needed to not require setting > -Djava.security.manager=allow to use JMX authentication. When SecurityManager is not permitted, which is the default, use the Subject.current() call. If SM is permitted, due to -Djava.security.manager=allow then the old Subject.getSubject(AccessController.getContext()) call is used. Tests are updated to not require -Djava.security.manager=allow and will test with and without that setting. Also additionally update tests to use Subject.current(), but also have a setting to test the old Subject.getSubject(AccessController.getContext()) call with -Djava.security.manager=allow (see ThreadPoolAccTest and test/jdk/javax/management/remote/mandatory/passwordAuthenticator). - PR Comment: https://git.openjdk.org/jdk/pull/19624#issuecomment-2158515271
RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed
JMX uses APIs related to the Security Mananger which are deprecated. Use of AccessControlContext will be removed when Security Manager is removed. Until then, updates are needed to not require setting -Djava.security.manager=allow to use JMX authentication. - Commit messages: - Additional test combination, old getSubject call with sm=allow - Test updates - whitespace - Merge remote-tracking branch 'upstream/master' into 844_JMX_Subject - 844: JMX attaching of Subject does not work when security manager not allowed Changes: https://git.openjdk.org/jdk/pull/19624/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19624=00 Issue: https://bugs.openjdk.org/browse/JDK-844 Stats: 160 lines in 19 files changed: 109 ins; 10 del; 41 mod Patch: https://git.openjdk.org/jdk/pull/19624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19624/head:pull/19624 PR: https://git.openjdk.org/jdk/pull/19624
Re: RFR: 8322811: jcmd System.dump_map help info has conflicting statements
On Fri, 7 Jun 2024 10:40:07 GMT, Thomas Stuefe wrote: > @dholmes-ora this is one of yours. > > This was a tad annoying to fix (fix is simple though), since the jcmd > framework has no good way to allow for default parameters that are not used > literally. E.g. in this case, the real value for the file name will contain > the process pid, which of course cannot be hard-coded. > > New output: > > > Syntax : System.dump_map [options] > > Options: (options must be specified using the or = syntax) > -H : [optional] Human readable format (BOOLEAN, false) > -F : [optional] file path (STRING, vm_memory_map_.txt) Looks good to me, yes non-literal defaults are a bit awkward here. - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19596#pullrequestreview-2107075791
Re: RFR: 8333680: com/sun/tools/attach/BasicTests.java fails with "SocketException: Permission denied: connect"
On Thu, 6 Jun 2024 02:12:11 GMT, Alex Menkov wrote: > The fix updates com/sun/tools/attach/BasicTests.java to listen and connect > using loopback addresses > > Testing: run the test on all Oracle-supported platforms Marked as reviewed by kevinw (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19571#pullrequestreview-2101206359
Re: RFR: 8332919: SA PointerLocation needs to print a newline after dumping java thread info for JNI Local Ref
On Fri, 24 May 2024 20:03:53 GMT, Chris Plummer wrote: > If PointerLocation discovers that an address is for a JNI local ref, it will > print information about the thread that owns the JNI local ref. For > JavaThreads it calls the printThreadIDOn(tty) method. There's a comment on > the call that says that it 'includes "\n"'. This is actually not true, and a > separate println() is needed. I noticed this when using the clhsdb findpc > command on a JNI local ref and noted that the next "hdsb> " prompt was > printed at the end of the findpc output instead of on a new line. Marked as reviewed by kevinw (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19402#pullrequestreview-2082284417
Integrated: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Wed, 15 May 2024 16:59:59 GMT, Kevin Walls wrote: > Running JConsole from a previous JDK, and attaching to jdk-23 (after > [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java > Management Extension (JMX) Subject Delegation feature), the MBean tab is > blank. > > In javax/management/remote/rmi/RMIConnectionImpl.java: > addNotificationListener rejects a non-null delegationSubjects array, but > older JDKs will send such an array. It could accept the array, and only > reject/throw if it contains a non-null Subject (i.e. if an attempt to use > subject delegation is really happening). > > Manually testing JConsole, the MBean tab is fully populated and usable. This pull request has now been integrated. Changeset: 253508b0 Author:Kevin Walls URL: https://git.openjdk.org/jdk/commit/253508b03a3de4dab00ed7fb57e9f345d8aed1a4 Stats: 19 lines in 2 files changed: 8 ins; 3 del; 8 mod 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation Reviewed-by: dfuchs, cjplummer - PR: https://git.openjdk.org/jdk/pull/19253
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v6]
On Fri, 24 May 2024 18:22:18 GMT, Kevin Walls wrote: >> Running JConsole from a previous JDK, and attaching to jdk-23 (after >> [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java >> Management Extension (JMX) Subject Delegation feature), the MBean tab is >> blank. >> >> In javax/management/remote/rmi/RMIConnectionImpl.java: >> addNotificationListener rejects a non-null delegationSubjects array, but >> older JDKs will send such an array. It could accept the array, and only >> reject/throw if it contains a non-null Subject (i.e. if an attempt to use >> subject delegation is really happening). >> >> Manually testing JConsole, the MBean tab is fully populated and usable. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > entries consistency in param and throws text Thanks Chris, and Daniel, for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/19253#issuecomment-2130210171
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v5]
On Fri, 24 May 2024 18:04:20 GMT, Chris Plummer wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> remove should... from delegationSubjects param
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
> line 978:
>
>> 976: * @throws IOException if a general communication exception
>> occurred.
>> 977: * @throws UnsupportedOperationException if {@code
>> delegationSubjects}
>> 978: * is non-null and contains any non-null values.
>
> Minor consistency issue. For the `delegationSubjects` comment above, you
> refer to "non-null entries". Here you refer to "non-null values". I don't
> have a preference on which you use, but they should be the same in both cases.
OK I'll change the "values" in the throws clause to be "entries", to be
consistent, to keep this moving...
(But I don't think there was any possible confusion here. 8-) )
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1613856276
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v6]
> Running JConsole from a previous JDK, and attaching to jdk-23 (after > [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java > Management Extension (JMX) Subject Delegation feature), the MBean tab is > blank. > > In javax/management/remote/rmi/RMIConnectionImpl.java: > addNotificationListener rejects a non-null delegationSubjects array, but > older JDKs will send such an array. It could accept the array, and only > reject/throw if it contains a non-null Subject (i.e. if an attempt to use > subject delegation is really happening). > > Manually testing JConsole, the MBean tab is fully populated and usable. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: entries consistency in param and throws text - Changes: - all: https://git.openjdk.org/jdk/pull/19253/files - new: https://git.openjdk.org/jdk/pull/19253/files/646a0d96..0b0164e3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19253=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19253=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19253/head:pull/19253 PR: https://git.openjdk.org/jdk/pull/19253
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v5]
> Running JConsole from a previous JDK, and attaching to jdk-23 (after > [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java > Management Extension (JMX) Subject Delegation feature), the MBean tab is > blank. > > In javax/management/remote/rmi/RMIConnectionImpl.java: > addNotificationListener rejects a non-null delegationSubjects array, but > older JDKs will send such an array. It could accept the array, and only > reject/throw if it contains a non-null Subject (i.e. if an attempt to use > subject delegation is really happening). > > Manually testing JConsole, the MBean tab is fully populated and usable. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: remove should... from delegationSubjects param - Changes: - all: https://git.openjdk.org/jdk/pull/19253/files - new: https://git.openjdk.org/jdk/pull/19253/files/4e8f84ec..646a0d96 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19253=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19253=03-04 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19253/head:pull/19253 PR: https://git.openjdk.org/jdk/pull/19253
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v3]
On Fri, 24 May 2024 16:49:25 GMT, Chris Plummer wrote: >> Thanks, appreciate the effort trying to make it perfect. >> Can't quite say "must be null or an array of all null entries" ..because I >> suppose it could be an empty array. >> >> In reality, the only caller is our code that wraps a null Subject value, in >> an array, so it's generally a single null in an array. >> >> I hope we are OK sticking with "which must not contain any non-null entries" >> as that does cover it (and implicitly does tell you an empty array is fine). > > For me the main hold up is using "should". Maybe: > > "Must be null or an array that doesn't contain any non-null entries." OK I think I can get rid of "should" - PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1613782613
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v3]
On Fri, 24 May 2024 15:50:00 GMT, Chris Plummer wrote:
>> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
>> line 961:
>>
>>> 959: * @param delegationSubjects should be {@code null}, but a non-null
>>> 960: * array is also accepted for compatibility reasons, which must not
>>> 961: * contain any non-null entries.
>>
>> The wording is bit unusual for a parameter description. Just wondering if
>> might be clearer to say "null or an array of null elements" and put add an
>> `@apiNote` to explain that it allows an array with null elements for
>> compatibility reasons. What you have is okay too course, I'm just trying to
>> think of another way to present this odd case.
>
> How about "must be null or an array of all null entries". You could still
> have an `@apiNote` explaining why.
Thanks, appreciate the effort trying to make it perfect.
Can't quite say "must be null or an array of all null entries" ..because I
suppose it could be an empty array.
In reality, the only caller is our code that wraps a null Subject value, in an
array, so it's generally a single null in an array.
I hope we are OK sticking with "which must not contain any non-null entries" as
that does cover it (and implicitly does tell you an empty array is fine).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1613758498
Re: RFR: 8332641: Update nsk.share.jpda.Jdb to don't use finalization
On Tue, 21 May 2024 21:49:51 GMT, Leonid Mesnik wrote: > The nsk.share.jdb.Jdb has finalize() nethods that close jdb connection and > output streams. > > The fix renames the method to close() and calls it explicitly after the test > finishes. I verified that close() called for each nsk share jdb test. > > The jdb is also LocalProcess with it's own cleanup(). This part still remains > the same so far. Looks good to me. Nice that you can correct the typo where one of the JdbTest.java log lines was saying finalize instead of killDebuggee. 8-) - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19336#pullrequestreview-2076278959
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Thu, 16 May 2024 08:25:17 GMT, Kevin Walls wrote: >>> > ...Is there any way to run jconsole in a way that would result in it >>> > passing a non-empty delegationSubjects, resulting in this issue still >>> > reproducing? >>> >>> I don't think there is, JConsole has a hard null in this call. Also I don't >>> see in JConsole any calls of getMBeanServerConnection with a Subject being >>> passed, that's the main gateway method to use the removed feature. >>> >>> If there was a way to use Subject Delegation with JConsole (or with >>> anything else), and you try to attach to a jdk-23, then that will fail with >>> the UnsupportedOperationException and that's what we want as the feature is >>> gone. Realistically it's a feature with no known usage as discussed in the >>> deprecation and removal changes. >> >> If jconsole is passing null, why is it triggering this exception? > >> If jconsole is passing null, why is it triggering this exception? > > JConsole passes null, but when running on an older jdk, the older > RMIConnector actually "promotes" it to an array before making the remote > call. If you connect to a jdk-23 with the removal, the exception is thrown. > > (JConsole running on jdk-23 can connect to jdk-23 fine.) > (Also just to note, a jdk-23 JConsole can connect to an older JDK and show > the MBean tab OK.) > @kevinjwalls I assume the RN for the removal of the subject delegation > feature will need to be adjusted once this current PR is integrated. When this fix goes in, there will be a window of builds between jdk 23+18 to jdk-23+build_with_this_fix, where older JMX clients using addNotificationListener() will see an exception. I wasn't going to highlight this in the RN, I thought JDK-8332303 looks quite discoverable already. If the RN is the place to discuss temporary and now-fixed issues with selection of earlier builds before release, I could add something there. - PR Comment: https://git.openjdk.org/jdk/pull/19253#issuecomment-2120560237
Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java
On Thu, 16 May 2024 10:39:43 GMT, Nizar Benalla wrote: > Please review this change. I converted the `package.html` file to > `package-info.java`, because `javac` cannot recognize `package.html`. > I already brought this up [in the mailing > list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html). > The conversion was done in-place, only renaming it in git. > > I also added a couple of `@since` tags, with only 2 changes I don't want to > split these two fixes into separate PRs. > `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in > https://bugs.openjdk.org/browse/JDK-8187556 Looks good - with the asterisk * prefix question from the other comment. I looked to check and yes the since 10 is correct. - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19263#pullrequestreview-2065901430
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v4]
> Running JConsole from a previous JDK, and attaching to jdk-23 (after > [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java > Management Extension (JMX) Subject Delegation feature), the MBean tab is > blank. > > In javax/management/remote/rmi/RMIConnectionImpl.java: > addNotificationListener rejects a non-null delegationSubjects array, but > older JDKs will send such an array. It could accept the array, and only > reject/throw if it contains a non-null Subject (i.e. if an attempt to use > subject delegation is really happening). > > Manually testing JConsole, the MBean tab is fully populated and usable. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: UOE doc correction - Changes: - all: https://git.openjdk.org/jdk/pull/19253/files - new: https://git.openjdk.org/jdk/pull/19253/files/68c791e7..4e8f84ec Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19253=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19253=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19253/head:pull/19253 PR: https://git.openjdk.org/jdk/pull/19253
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v3]
On Fri, 17 May 2024 10:14:43 GMT, Daniel Fuchs wrote:
>> Kevin Walls has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - add an 'also'
>> - typo
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
> line 979:
>
>> 977: * @throws IOException if a general communication exception
>> occurred.
>> 978: * @throws UnsupportedOperationException if {@code
>> delegationSubjects}
>> 979: * contains any non-null values.
>
> Suggestion:
>
> * @throws UnsupportedOperationException if {@code delegationSubjects}
> * is non-null and contains any non-null values.
Yes, thanks well spotted.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1605235219
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v2]
On Thu, 16 May 2024 19:53:48 GMT, Sean Mullan wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> IllegalArgumentException throws doc update
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
> line 959:
>
>> 957: * NotificationFilters. Elements of this array can
>> 958: * be null.
>> 959: * @param delegationSubjects should be {@code null}, but a non-null
>
> Would it be more clear to say: "should be {@code null}. However, an array
> where every entry is null is also accepted for compatibility reasons."
Yes, I like adding the "also".
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
> line 960:
>
>> 958: * be null.
>> 959: * @param delegationSubjects should be {@code null}, but a non-null
>> 960: * array is accepted for compatibilty reasons, which must not
>> contain
>
> Typo: s/compatibilty/compatibility/
oops yes thanks, done.
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
> line 969:
>
>> 967: * @throws IllegalArgumentException if names or
>> 968: * filters is null, or if names contains
>> 969: * a null element, or if these two arrays do not have the same size.
>
> Was this actually an oversight in the previous change to remove subject
> delegation? When `delegationSubjects` is null, then the 3 arrays are never
> going to be the same size.
Yes, this is an oversight from the previous change.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1603976969
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1603974595
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1603975797
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v3]
> Running JConsole from a previous JDK, and attaching to jdk-23 (after > [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java > Management Extension (JMX) Subject Delegation feature), the MBean tab is > blank. > > In javax/management/remote/rmi/RMIConnectionImpl.java: > addNotificationListener rejects a non-null delegationSubjects array, but > older JDKs will send such an array. It could accept the array, and only > reject/throw if it contains a non-null Subject (i.e. if an attempt to use > subject delegation is really happening). > > Manually testing JConsole, the MBean tab is fully populated and usable. Kevin Walls has updated the pull request incrementally with two additional commits since the last revision: - add an 'also' - typo - Changes: - all: https://git.openjdk.org/jdk/pull/19253/files - new: https://git.openjdk.org/jdk/pull/19253/files/6c97b5ed..68c791e7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19253=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19253=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19253/head:pull/19253 PR: https://git.openjdk.org/jdk/pull/19253
Re: RFR: 8328866: Add raw monitor rank support to the debug agent. [v7]
On Thu, 16 May 2024 15:31:15 GMT, Chris Plummer wrote: >> This PR adds ranked monitor support to the debug agent. The debug agent has >> a large number of monitors, and it's really hard to know which order to grab >> them in, and for that matter which monitors might already be held at any >> given moment. By imposing a rank on each monitor, we can check to make sure >> they are always grabbed in the order of their rank. Having this in place >> when I was working on >> [JDK-8324868](https://bugs.openjdk.org/browse/JDK-8324868) would have made >> it much easier to detect a deadlock that was occuring, and the reason for >> it. That's what motivated me to do this work >> >> There were 2 or 3 minor rank issues discovered as a result of these changes. >> I also learned a lot about some of the more ugly details of the locking >> support in the process. >> >> Tested with the following on all supported platforms and with virtual >> threads: >> >> com/sun/jdi >> vmTestbase/nsk/jdi >> vmTestbase/nsk/jdb >> vmTestbase/nsk/jdwp >> >> Still need to run tier2 and tier5. >> >> Details of the changes follow in the first comment. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Simplify by getting rid of special logic around leaf monitors. src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 1781: > 1779: * popFrameEventLock. Meanwhile when the target thread gets the > SINGLE_STEP event, > 1780: * it always enters popFrameProceedLock first (which is always > available), then > 1781: * popFrameProceedLock second. It will always succeed because the > Reader thread Is that "then popFrameEventLock second" Drawing these out in two columns I can't see a deadlock either 8-) - PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1603777908
Re: RFR: 8328866: Add raw monitor rank support to the debug agent. [v7]
On Thu, 16 May 2024 15:31:15 GMT, Chris Plummer wrote: >> This PR adds ranked monitor support to the debug agent. The debug agent has >> a large number of monitors, and it's really hard to know which order to grab >> them in, and for that matter which monitors might already be held at any >> given moment. By imposing a rank on each monitor, we can check to make sure >> they are always grabbed in the order of their rank. Having this in place >> when I was working on >> [JDK-8324868](https://bugs.openjdk.org/browse/JDK-8324868) would have made >> it much easier to detect a deadlock that was occuring, and the reason for >> it. That's what motivated me to do this work >> >> There were 2 or 3 minor rank issues discovered as a result of these changes. >> I also learned a lot about some of the more ugly details of the locking >> support in the process. >> >> Tested with the following on all supported platforms and with virtual >> threads: >> >> com/sun/jdi >> vmTestbase/nsk/jdi >> vmTestbase/nsk/jdb >> vmTestbase/nsk/jdwp >> >> Still need to run tier2 and tier5. >> >> Details of the changes follow in the first comment. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Simplify by getting rid of special logic around leaf monitors. src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 1749: > 1747: * the JDWP Command Reader thread and the PopFrame() target thread will > grab > 1748: * both of these locks. However, one curious trait of these two locks > is that > 1749: * the these two threads do not both grab them in the same order (and > they need nit: "that the these two" - PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1603728625
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v2]
On Thu, 16 May 2024 11:38:28 GMT, Daniel Fuchs wrote: >> Yes, completely understand. I just don't think it has any benefit. > > Hmmm... the spec still says: > > * @throws IllegalArgumentException if names or > * filters is null, or if names contains > * a null element, or if the three arrays do not all have the same > * size. > ``` Thanks for additionally spotting the throws IAE needs updating, adding that now... (Will update the CSR also.) - PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1603191504
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation [v2]
> Running JConsole from a previous JDK, and attaching to jdk-23 (after > [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java > Management Extension (JMX) Subject Delegation feature), the MBean tab is > blank. > > In javax/management/remote/rmi/RMIConnectionImpl.java: > addNotificationListener rejects a non-null delegationSubjects array, but > older JDKs will send such an array. It could accept the array, and only > reject/throw if it contains a non-null Subject (i.e. if an attempt to use > subject delegation is really happening). > > Manually testing JConsole, the MBean tab is fully populated and usable. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: IllegalArgumentException throws doc update - Changes: - all: https://git.openjdk.org/jdk/pull/19253/files - new: https://git.openjdk.org/jdk/pull/19253/files/ef84485e..6c97b5ed Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19253=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19253=00-01 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19253/head:pull/19253 PR: https://git.openjdk.org/jdk/pull/19253
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Thu, 16 May 2024 10:26:25 GMT, Daniel Fuchs wrote: >> That the JConsole tab was blank shows that the older RMIConnector's >> addListenerWithSubject creates a new single-entry array from the given >> delegationSubject (which is null) and passes it onwards. The app is not >> creating the array itself., that's us. >> >> (also, maybe that JConsole relies on listeners in order to show a screen >> that doesn't really need to depend on them, but this change is obviously >> about being compatible with that) >> >> We all know that that is the only use case out there, the current wisdom is >> that this feature is not used, nobody is creating the Subject array and >> calling addListenersWithSubjects (plural) with it... >> >> IF we find such an app app, we are going to ignore the array unless it >> contains a non-null entry. This seems safe and efficient. We are >> documenting that it should be null, and it is weird to document a length >> requirement for something that should be null... 8-) > > This shows that when SubjectDelegation was not used, a null-filled array of > the same length as the two other arrays was expected before (in previous > versions of the JDK where SubjectDelegation was supported, but in the case > where it wasn't used). > I am not suggesting to document the length requirement. The length > requirement was enforced before and undocumented. I'm just suggesting that we > allow null and null-filled but don't allow something (null filled array of > wrong length) that would have been rejeceted in previous JDK versions. I > would also suggest to check the length before the content - in case an array > is supplied. Yes, completely understand. I just don't think it has any benefit. - PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1603106058
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Thu, 16 May 2024 09:40:58 GMT, Daniel Fuchs wrote: >> That seems pretty extreme -- it's an array we explicitly don't want and are >> going to ignore? If somebody passes a non-null member, we will throw as >> unsupported. I was thinking that was enough, hence removing the sbjs array >> to be quite certain we can't pass on any supplied Subjects. > > Well my thinking was this: the fact that the jconsole tab was blank shows > that the array may being passed. The previous code verified that all three > arrays had the same length - so it would have failed if the array had a > length different than the other two. So I would prefer if we kept on throwing > in that case. In other words, we now allow and prefer `null` - but if > non-null - we will allow a null-filled array passed by an older client but > we should not accept something that would have been invalid then. That the JConsole tab was blank shows that the older RMIConnector's addListenerWithSubject creates a new single-entry array from the given delegationSubject (which is null) and passes it onwards. The app is not creating the array itself., that's us. (also, maybe that JConsole relies on listeners in order to show a screen that doesn't really need to depend on them, but this change is obviously about being compatible with that) We all know that that is the only use case out there, the current wisdom is that this feature is not used, nobody is creating the Subject array and calling addListenersWithSubjects (plural) with it... IF we find such an app app, we are going to ignore the array unless it contains a non-null entry. This seems safe and efficient. We are documenting that it should be null, and it is weird to document a length requirement for something that should be null... 8-) - PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1603064299
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Wed, 15 May 2024 22:44:03 GMT, Serguei Spitsyn wrote:
>> Running JConsole from a previous JDK, and attaching to jdk-23 (after
>> [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java
>> Management Extension (JMX) Subject Delegation feature), the MBean tab is
>> blank.
>>
>> In javax/management/remote/rmi/RMIConnectionImpl.java:
>> addNotificationListener rejects a non-null delegationSubjects array, but
>> older JDKs will send such an array. It could accept the array, and only
>> reject/throw if it contains a non-null Subject (i.e. if an attempt to use
>> subject delegation is really happening).
>>
>> Manually testing JConsole, the MBean tab is fully populated and usable.
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
> line 979:
>
>> 977: for (Subject s: delegationSubjects) {
>> 978: if (s != null) {
>> 979: throw new UnsupportedOperationException("Subject
>> Delegation has been removed.");
>
> Q1: Would it make sense to provide any details about the non-null
> `delegationSubject` element?
> Q2: Does this fix needs a CSR? My guess is that this issue is very minor, so
> CSR is not needed.
Q1 We have a couple of places where we use that text, when a non-null Subject
is given. It's quite generic and that is a good fit. Your app would have to
go to a significant effort to use the Subject Delegation feature, and we aren't
aware of any apps that use it. If JConsole were using the feature, you would
know as you would have had to specifically configure it somehow (as well as on
the remote end with SM and policies), so I really think the generic message is
explicit enough.
Q2 Yes, Daniel very quickly added the CSR flag. I was presuming that
"paperwork" would need doing, although yes as spec changes go, it is very very
minor.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1602917924
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Wed, 15 May 2024 23:40:00 GMT, Chris Plummer wrote: > If jconsole is passing null, why is it triggering this exception? JConsole passes null, but when running on an older jdk, the older RMIConnector actually "promotes" it to an array before making the remote call. If you connect to a jdk-23 with the removal, the exception is thrown. (JConsole running on jdk-23 can connect to jdk-23 fine.) - PR Comment: https://git.openjdk.org/jdk/pull/19253#issuecomment-2114453521
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Wed, 15 May 2024 21:20:25 GMT, Chris Plummer wrote: > ...Is there any way to run jconsole in a way that would result in it passing > a non-empty delegationSubjects, resulting in this issue still reproducing? I don't think there is, JConsole has a hard null in this call. Also I don't see in JConsole any calls of getMBeanServerConnection with a Subject being passed, that's the main gateway method to use the removed feature. If there was a way to use Subject Delegation with JConsole (or with anything else), and you try to attach to a jdk-23, then that will fail with the UnsupportedOperationException and that's what we want as the feature is gone. Realistically it's a feature with no known usage as discussed in the deprecation and removal changes. - PR Comment: https://git.openjdk.org/jdk/pull/19253#issuecomment-2113498480
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Wed, 15 May 2024 19:09:54 GMT, Chris Plummer wrote: > I'm just trying to understand current and previous behavior of jconsole a bit > better. Right, to be clear it's not JConsole's fault. The early part of JConsole's stack is: ...connection at java.management.rmi/javax.management.remote.rmi.RMIConnectionImpl_Stub.addNotificationListeners(RMIConnectionImpl_Stub.java:136) at java.management.rmi/javax.management.remote.rmi.RMIConnector.addListenersWithSubjects(RMIConnector.java:595) at java.management.rmi/javax.management.remote.rmi.RMIConnector.addListenerWithSubject(RMIConnector.java:571) at java.management.rmi/javax.management.remote.rmi.RMIConnector$RemoteMBeanServerConnection.addNotificationListener(RMIConnector.java:1238) at jdk.jconsole/sun.tools.jconsole.MBeansTab$1.doInBackground(MBeansTab.java:93) ...older frames... ...and when it is running with an older JDK, RMIConnector.addListenerWithSubject() would create a new array of Subjects, containing what it was given (which is null). It passes on the array, and then is going to get the UnsupportedOperationException. i.e. Older client code passing a null Subject, actually "promotes" it to an array and hits the Exception, although it really is not trying to use the removed feature. This problem is specifically with addListenerWithSubject. - PR Comment: https://git.openjdk.org/jdk/pull/19253#issuecomment-2113397029
Re: RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
On Wed, 15 May 2024 17:49:07 GMT, Daniel Fuchs wrote:
>> Running JConsole from a previous JDK, and attaching to jdk-23 (after
>> [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java
>> Management Extension (JMX) Subject Delegation feature), the MBean tab is
>> blank.
>>
>> In javax/management/remote/rmi/RMIConnectionImpl.java:
>> addNotificationListener rejects a non-null delegationSubjects array, but
>> older JDKs will send such an array. It could accept the array, and only
>> reject/throw if it contains a non-null Subject (i.e. if an attempt to use
>> subject delegation is really happening).
>>
>> Manually testing JConsole, the MBean tab is fully populated and usable.
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java
> line 984:
>
>> 982: }
>> 983: if (names.length != filters.length) {
>> 984: final String msg = "The lengths of names and filters
>> parameters are not same.";
>
> I wonder if we should check that if present, `delegationSubjects.length ==
> names.length`?
That seems pretty extreme -- it's an array we explicitly don't want and are
going to ignore? If somebody passes a non-null member, we will throw as
unsupported. I was thinking that was enough, hence removing the sbjs array to
be quite certain we can't pass on any supplied Subjects.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19253#discussion_r1602181014
RFR: 8332303: Better JMX interoperability with older JDKs, after removing Subject Delegation
Running JConsole from a previous JDK, and attaching to jdk-23 (after [JDK-832](https://bugs.openjdk.org/browse/JDK-832): Remove the Java Management Extension (JMX) Subject Delegation feature), the MBean tab is blank. In javax/management/remote/rmi/RMIConnectionImpl.java: addNotificationListener rejects a non-null delegationSubjects array, but older JDKs will send such an array. It could accept the array, and only reject/throw if it contains a non-null Subject (i.e. if an attempt to use subject delegation is really happening). Manually testing JConsole, the MBean tab is fully populated and usable. - Commit messages: - Remove unnecessary Subject array in RMIConnectionImpl, update docs in RMIConnection - JConsole's MBean tab is blank after JMX Subject Delegation removal. Changes: https://git.openjdk.org/jdk/pull/19253/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19253=00 Issue: https://bugs.openjdk.org/browse/JDK-8332303 Stats: 18 lines in 2 files changed: 9 ins; 2 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/19253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19253/head:pull/19253 PR: https://git.openjdk.org/jdk/pull/19253
Integrated: 8331950: Remove MemoryPoolMBean/isCollectionUsageThresholdExceeded from ZGC ProblemLists
On Wed, 8 May 2024 17:05:30 GMT, Kevin Walls wrote: > Remove from zgc problemlists. > Trivial fix. > > This was omitted when https://bugs.openjdk.org/browse/JDK-8303136 was > integrated. > > I see the tests passing, including with ZGC. Just ran my own batch of tests > in addition, and it includes passes with e.g. -XX:+UseZGC -XX:+ZGenerational This pull request has now been integrated. Changeset: aa4cddd4 Author:Kevin Walls URL: https://git.openjdk.org/jdk/commit/aa4cddd4b8a6a12ba5d0360a721aebaabf362fff Stats: 2 lines in 2 files changed: 0 ins; 2 del; 0 mod 8331950: Remove MemoryPoolMBean/isCollectionUsageThresholdExceeded from ZGC ProblemLists Reviewed-by: sspitsyn - PR: https://git.openjdk.org/jdk/pull/19143
Re: RFR: 8331950: Remove MemoryPoolMBean/isCollectionUsageThresholdExceeded from ZGC ProblemLists
On Wed, 8 May 2024 17:05:30 GMT, Kevin Walls wrote: > Remove from zgc problemlists. > Trivial fix. > > This was omitted when https://bugs.openjdk.org/browse/JDK-8303136 was > integrated. > > I see the tests passing, including with ZGC. Just ran my own batch of tests > in addition, and it includes passes with e.g. -XX:+UseZGC -XX:+ZGenerational Thanks Serguei! - PR Comment: https://git.openjdk.org/jdk/pull/19143#issuecomment-2102918974
Re: RFR: 8330146: assert(!_thread->is_in_any_VTMS_transition()) failed
On Thu, 2 May 2024 10:07:35 GMT, Serguei Spitsyn wrote: > Any event posting code except CFLH, ClassPrepare and ClassLoad events has a > conditional return in case if the event is posted during a VTMS transition. > The CFLH, ClassPrepare and ClassLoad event posting code has just an assert > instead. The ClassPrepare and ClassLoad events also have a conditional return > in a case of temporary VTMS transition. > This update is to align the CFLH, ClassPrepare and ClassLoad events with all > other events in this area. > > Testing: > - TBD: submit mach5 tiers 1-6 Looks good to me. post_class_file_load_hook during VirtualThread$VThreadContinuation.onPinned, and because it's the first onPinned call it causes classloading, so just avoid that circular problem (these may not be the class loading events that most people are looking for most of the time). I wonder if this change is needed only until "jdk.tracePinnedThreads" is removed? Maybe this becomes unnecessary if TRACE_PINNING_MODE is gone, as maybe there won't be any possible classloading. Just a thought, no need for any more in this change. - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19054#pullrequestreview-2047999334
RFR: 8331950: Remove MemoryPoolMBean/isCollectionUsageThresholdExceeded from ZGC ProblemLists
Remove from zgc problemlists. This was omitted when https://bugs.openjdk.org/browse/JDK-8303136 was integrated. I see the tests passing, including with ZGC. Just ran my own batch of tests in addition, and it includes passes with e.g. -XX:+UseZGC -XX:+ZGenerational - Commit messages: - Remove MemoryPoolMBean/isCollectionUsageThresholdExceeded from ZGC ProblemLists Changes: https://git.openjdk.org/jdk/pull/19143/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19143=00 Issue: https://bugs.openjdk.org/browse/JDK-8331950 Stats: 2 lines in 2 files changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19143.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19143/head:pull/19143 PR: https://git.openjdk.org/jdk/pull/19143
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v16]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 33 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - Update XX option - Update decode_raw usage to decode_without_asserts, after 8328698 - nits - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - Update XX flag requirement - VMInspectTest update - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - One dbg_is_good_oop method - Test more pointer types: compiled method and metadata. - ... and 23 more: https://git.openjdk.org/jdk/compare/e5180fba...bb92a849 - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/7e6c7b97..bb92a849 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=15 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=14-15 Stats: 16719 lines in 562 files changed: 8816 ins; 4016 del; 3887 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Integrated: 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock
On Tue, 9 Apr 2024 11:08:31 GMT, Kevin Walls wrote: > This test incorrectly fails, although rarely, thinking its "thread 2" has > deadlocked. > A change of sleep will likely fix this, but there are other issues, so > cleaning up the test a little. > > Remove the probe for the ManagementFactory class, to check we are on jdk5 or > later. 8-) > > When sleeping, sleep 100, not 1ms, we don't need to spin fast and actually > race with the other thread. > > We have a 1000 iteration loop, but don't seem to use it. We only check once > then either return (pass), fail, or break (which is also fail). Use the loop > to check for the status change, which is likely what was intended. > > Show the stackframes on all failures. This pull request has now been integrated. Changeset: 58911ccc Author:Kevin Walls URL: https://git.openjdk.org/jdk/commit/58911ccc2c48b4b5dd2ebc9d2a44dcf3737eca50 Stats: 42 lines in 1 file changed: 9 ins; 21 del; 12 mod 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock Reviewed-by: cjplummer, lmesnik - PR: https://git.openjdk.org/jdk/pull/18687
Re: RFR: 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock [v2]
On Tue, 16 Apr 2024 09:26:58 GMT, Kevin Walls wrote: >> This test incorrectly fails, although rarely, thinking its "thread 2" has >> deadlocked. >> A change of sleep will likely fix this, but there are other issues, so >> cleaning up the test a little. >> >> Remove the probe for the ManagementFactory class, to check we are on jdk5 or >> later. 8-) >> >> When sleeping, sleep 100, not 1ms, we don't need to spin fast and actually >> race with the other thread. >> >> We have a 1000 iteration loop, but don't seem to use it. We only check once >> then either return (pass), fail, or break (which is also fail). Use the >> loop to check for the status change, which is likely what was intended. >> >> Show the stackframes on all failures. > > Kevin Walls has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Remove System.exit calls > - Merge remote-tracking branch 'upstream/master' into > 8188784_BroadcasterSupportDeadlockTest > - 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java > - TEST FAILED: deadlock Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/18687#issuecomment-2058745011
Re: RFR: 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock [v2]
On Tue, 16 Apr 2024 00:31:10 GMT, Leonid Mesnik wrote:
>> Kevin Walls has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains three additional
>> commits since the last revision:
>>
>> - Remove System.exit calls
>> - Merge remote-tracking branch 'upstream/master' into
>> 8188784_BroadcasterSupportDeadlockTest
>> - 8188784:
>> javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST
>> FAILED: deadlock
>
> test/jdk/javax/management/notification/BroadcasterSupportDeadlockTest.java
> line 122:
>
>> 120: java.util.Map traces =
>> Thread.getAllStackTraces();
>> 121: showStackTrace("Thread 1", traces.get(t1));
>> 122: showStackTrace("Thread 2", traces.get(t2));
>
> Could you please replace System.exit() with throwing Exception. Other looks
> good.
OK sure.
FYI I count 137 System.exit calls in test/jdk/javax/management
That's after I take out the existing 3 that are in this test. 8-)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18687#discussion_r1567098465
Re: RFR: 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock [v2]
> This test incorrectly fails, although rarely, thinking its "thread 2" has > deadlocked. > A change of sleep will likely fix this, but there are other issues, so > cleaning up the test a little. > > Remove the probe for the ManagementFactory class, to check we are on jdk5 or > later. 8-) > > When sleeping, sleep 100, not 1ms, we don't need to spin fast and actually > race with the other thread. > > We have a 1000 iteration loop, but don't seem to use it. We only check once > then either return (pass), fail, or break (which is also fail). Use the loop > to check for the status change, which is likely what was intended. > > Show the stackframes on all failures. Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Remove System.exit calls - Merge remote-tracking branch 'upstream/master' into 8188784_BroadcasterSupportDeadlockTest - 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock - Changes: - all: https://git.openjdk.org/jdk/pull/18687/files - new: https://git.openjdk.org/jdk/pull/18687/files/5237f1d8..a77b3c85 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18687=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18687=00-01 Stats: 16713 lines in 561 files changed: 8813 ins; 4016 del; 3884 mod Patch: https://git.openjdk.org/jdk/pull/18687.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18687/head:pull/18687 PR: https://git.openjdk.org/jdk/pull/18687
Re: RFR: 8329774: Break long lines in jdk/src/jdk.hotspot.agent/doc /transported_core.html [v2]
On Thu, 11 Apr 2024 07:12:20 GMT, Ludvig Janiuk wrote: >> I used "fold -sw 120" and removed trailing whitespaces. > > Ludvig Janiuk has updated the pull request incrementally with three > additional commits since the last revision: > > - prepends > - fold 100 > - Revert "8329774 Break long lines in jdk/src/jdk.hotspot.agent/doc > /transported_core.html" > >This reverts commit 12346b1b178e7ac3b1fbaddec76890f05edb9525. Looks good to me! - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18654#pullrequestreview-1993615107
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v15]
On Tue, 9 Apr 2024 06:00:21 GMT, David Holmes wrote: > sounds far too general. I would have expected something that was obviously > connected to jcmd and/or the specific jcmd under discussion. I don't expect so many more of these, but EnableVMInspectDCmd looked too specific, I didn't want to pursue a 1:1 mapping of a diagnostic command to feature, so looked for something more general. To HotSpot it's a Diagnostic Command, to users it's a jcmd, so maybe don't want to specifically say either in the flag. But EnableVMInspect is not right (there are many ways we inspect the VM!), and EnableVMInspectFeatures or UnlockVMInspectionFeatures again might be too generic. Next suggestion: EnableVMInspectCommand - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2045006108
RFR: 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock
This test incorrectly fails, although rarely, thinking its "thread 2" has deadlocked. A change of sleep will likely fix this, but there are other issues, so cleaning up the test a little. Remove the probe for the ManagementFactory class, to check we are on jdk5 or later. 8-) When sleeping, sleep 100, not 1ms, we don't need to spin fast and actually race with the other thread. We have a 1000 iteration loop, but don't seem to use it. We only check once then either return (pass), fail, or break (which is also fail). Use the loop to check for the status change, which is likely what was intended. Show the stackframes on all failures. - Commit messages: - 8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock Changes: https://git.openjdk.org/jdk/pull/18687/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18687=00 Issue: https://bugs.openjdk.org/browse/JDK-8188784 Stats: 36 lines in 1 file changed: 9 ins; 18 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/18687.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18687/head:pull/18687 PR: https://git.openjdk.org/jdk/pull/18687
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v15]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request incrementally with two additional commits since the last revision: - Update decode_raw usage to decode_without_asserts, after 8328698 - nits - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/510feaa1..7e6c7b97 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=14 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=13-14 Stats: 7 lines in 2 files changed: 0 ins; 4 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v14]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 29 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - Update XX flag requirement - VMInspectTest update - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - One dbg_is_good_oop method - Test more pointer types: compiled method and metadata. - Argument to be named address. - test nit - Undo include - Change to jcmd VM.inspect - ... and 19 more: https://git.openjdk.org/jdk/compare/dfb10c33...510feaa1 - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/17f22d65..510feaa1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=13 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=12-13 Stats: 13454 lines in 374 files changed: 5613 ins; 6011 del; 1830 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v13]
On Thu, 4 Apr 2024 12:46:26 GMT, Kevin Walls wrote: >> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object >> information. >> >> Not recommended for live production use. Requires UnlockDiagnosticVMOptions >> and not included in jcmd help output, to remind us this is not a >> general-purpose customer-facing tool. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > VMInspectTest update Updating to change the "guard" flag. I was proposing using a -XX "guard" flag at startup time, to enable jcmd VM.inspect, because it allows inspection of arbitrary pointers given by the the user. With the improved dbg_is_good_oop I have not managed to cause any crashes, but using the guard flag is a cautious approach. Will not use UnlockDiagnosticVMOptions as that is intended for command-line option processing. Therefore a new -XX product flag: -XX:+UnlockDiagnosticVMFeatures which defaults to false and is true in debug builds. While named similarly to UnlockDiagnosticVMOptions, that option is quite long established and well-documented with many search results, so we should not cause confusion. The logic is that that option relates to enabling further command-line options, and the new option relates to diagnostic features in the VM at runtime. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2043037830
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Mon, 8 Apr 2024 13:20:03 GMT, Thomas Stuefe wrote: > Thank you for answering our questions. No problem, thanks Thomas and Andrew. 8-) - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2043005639
Integrated: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature
On Tue, 27 Feb 2024 10:44:20 GMT, Kevin Walls wrote: > The deprecated Subject Delegation feature in JMX will be removed. > > This was marked in JDK 21 as deprecated for removal (JDK-8298966). This pull request has now been integrated. Changeset: 6382a129 Author: Kevin Walls URL: https://git.openjdk.org/jdk/commit/6382a1290fbd7cc8fd097a2972bfcfc06fa4de79 Stats: 2026 lines in 35 files changed: 214 ins; 1632 del; 180 mod 832: Remove the Java Management Extension (JMX) Subject Delegation feature Reviewed-by: mchung, dfuchs - PR: https://git.openjdk.org/jdk/pull/18025
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v16]
> The deprecated Subject Delegation feature in JMX will be removed. > > This was marked in JDK 21 as deprecated for removal (JDK-8298966). Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 25 commits: - Merge remote-tracking branch 'upstream/master' into 832_SubjectDelegation_remove - Merge - Missing code doc nit. - Missing code doc nit. - RMIConnectionImpl_Stub also should explicity inherit the unchecked UOE. - Clarify JMXConnector equivalence comment. - RMIConnectionImpl needs to explicity inherit the unchecked UOE. - typo - Javadoc update - Remove unnecessary constructor in private class - ... and 15 more: https://git.openjdk.org/jdk/compare/b9da1401...7fec01c7 - Changes: https://git.openjdk.org/jdk/pull/18025/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18025=15 Stats: 2026 lines in 35 files changed: 214 ins; 1632 del; 180 mod Patch: https://git.openjdk.org/jdk/pull/18025.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18025/head:pull/18025 PR: https://git.openjdk.org/jdk/pull/18025
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 05:26:09 GMT, Chris Plummer wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Undo include
>
> src/hotspot/share/utilities/debug.cpp line 680:
>
>> 678:
>> 679: // Additional "good oop" checks, separate method to not disturb
>> existing asserts.
>> 680: extern "C" bool dbg_is_good_oop_detailed(oopDesc* o) {
>
> What was the motivation for adding this? Is this copied from other HS code?
> How do we know it's doing a good enough job of verification? What happens if
> verification is not thorough enough?
On use of dbg_is_good_oop() and do we need our own dbg_is_good_oop_detailed():
The worst case is a crash if the data looks enough like an oop to pass basic
checks, but if the klass pointer or a field turns out to be a wild pointer. So
I wanted a stricter and more careful "is good oop" check, e.g. to make it
verify the klass pointer is safe, before calling is_klass(). I cannot now see
a crash when taking a valid object and examining thousands of what must be
wrong pointers in memory after it.
I kept these separate, as had seen an assert when making the existing
dbg_is_good_oop more strict. But that was an error, I had the pointer
arithmetic wrong.
This check is called by asserts in two existing places, plus the new
diagnosticCommand in this PR. With this update I'm running all of tier1 OK,
which includes ContFramePopTest.java where I could previously trigger an assert.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1551632113
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v11]
On Fri, 29 Mar 2024 04:05:53 GMT, Serguei Spitsyn wrote:
>> Kevin Walls has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Test more pointer types: compiled method and metadata.
>
> test/hotspot/jtreg/serviceability/dcmd/vm/VMInspectTest.java line 117:
>
>> 115: output = executor.execute("VM.inspect -1");
>> 116: output.shouldContain("address not safe");
>> 117:
>
> Nit: Just a suggestion to make the test more readable. Now when more test
> cases have been added you may want to refactor it to call a separate method
> for each sub-test.
> E.g.: `testBaddAddresses()`, `testMisalignedAddress()`,
> `testCompiledMethodAddress()`, `testMetadataAddress()`, `testClassAddress()`,
> `testThreadAddress()`, etc.
Thanks, yes have updated the test and split up the tests.
Also added a retry on the Java object inspection part, as I saw a rare failure
where the pointer found in Thread.print is no longer correct when we do the
inspect.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1551614153
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v13]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: VMInspectTest update - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/e7fc0325..17f22d65 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=12 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=11-12 Stats: 82 lines in 1 file changed: 47 ins; 9 del; 26 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v12]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 26 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - One dbg_is_good_oop method - Test more pointer types: compiled method and metadata. - Argument to be named address. - test nit - Undo include - Change to jcmd VM.inspect - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - Test update - Show description if unknown subcommand. - ... and 16 more: https://git.openjdk.org/jdk/compare/ca24041f...e7fc0325 - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/9ed958f6..e7fc0325 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=11 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=10-11 Stats: 13189 lines in 329 files changed: 4768 ins; 6005 del; 2416 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: JDK-8322042: HeapDumper should perform merge on the current thread instead of VMThread [v2]
On Tue, 2 Apr 2024 21:13:33 GMT, Alex Menkov wrote: >> The fix updated HeapDumper to always perform merge on the current thread. >> >> Testing: tier1-5, all HeapDump-related tests >> Covered heap dumping scenarios: >> - `jcmd GC.heap_dump` command; >> - `HotSpotDiagnosticMXBean.dumpHeap()`; >> - `HeapDumpBeforeFullGC`, `HeapDumpAfterFullGC` VM options; >> - `HeapDumpOnOutOfMemoryError` VM option. > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > comment Marked as reviewed by kevinw (Reviewer). Thanks Alex, happy you've looked into that. 8-) - PR Review: https://git.openjdk.org/jdk/pull/18571#pullrequestreview-1978117624 PR Comment: https://git.openjdk.org/jdk/pull/18571#issuecomment-2035648620
Re: RFR: JDK-8322042: HeapDumper should perform merge on the current thread instead of VMThread [v2]
On Tue, 2 Apr 2024 21:13:33 GMT, Alex Menkov wrote: >> The fix updated HeapDumper to always perform merge on the current thread. >> >> Testing: tier1-5, all HeapDump-related tests >> Covered heap dumping scenarios: >> - `jcmd GC.heap_dump` command; >> - `HotSpotDiagnosticMXBean.dumpHeap()`; >> - `HeapDumpBeforeFullGC`, `HeapDumpAfterFullGC` VM options; >> - `HeapDumpOnOutOfMemoryError` VM option. > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > comment Are we saying there is never any need to perform the merge in a VM Operation? Originally (JDK-8306441) it's either done in the attach thread, or a VM operation if we are in another thread. But maybe that was just being cautious. Keeping only the invoking thread busy, rather than a VM Operation, sounds like a good goal. I'm interested in if we can hit a problem by causing multiple heap dumps at once, e.g. jcmd, the app calling the MXBean, and maybe even the OnOOM trigger as well, could all happen at the same time. 8-) Using the MXBean, we may be able to make multiple heap dumps concurrently (either multiple MXBean invocations, or one MXBean invocation while a jcmd runs...). jcmd keeps the atttach listener thread busy, so we can't have concurrent dumps from that method. I think that explains the original code, where all other methods use the VM Operation. - PR Review: https://git.openjdk.org/jdk/pull/18571#pullrequestreview-1976273140
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 18:39:38 GMT, Chris Plummer wrote: > I think we also need to consider the flip side of this argument. Is this > something that some customers might want to always enable, but don't want to > always have UnlockDiagnosticVMOptions enabled. A new command line flag would > be needed in that case. > > Also, isn't UnlockDiagnosticVMOptions meant for enabling the use of > diagnostic command line flags? Do we have examples of it enabling a hotspot > feature that does not also require setting a diagnostic command line flag? Thanks Chris - that is correct now I check the wording, UnlockDiagnosticVMOptions: "Enable normal processing of flags relating to field diagnostics" Yes it makes flags which are DIAGNOSTIC available at the command-line. We have UnlockExperimentalVMOptions also. My original reason for the guard, is that the risk of fooling the "good oop" check is that we could possibly crash (so that's answering your other question). Inspecting an arbitrary pointer that looks enough like a Java heap object to fool the test, means it might try and resolve bad addresses for Klass pointer or field data. I also have a stress test which examines every address in a Java heap to test for crashes. That's obviously long-running as it does a jcmd for every iteration. But I can't currently get a crash, trying G1, ZGC, ZGCGenerational. Having a global guard to prevent usage of VM.inspect may be needed less than I thought, but will wait on the other thread before saying that is really the case. We could have a new -XX flag to guard it (whether specific to this command or a more general UnlockDiagnosticFeatures. I would definitely stick to this being in all builds, as we frequently "debug" non-debug builds. Also, for your comment above about the new version of dbg_is_good_oop: this was added because although there are not many callers of it, I did not want to upset them. During my earlier testing the detailed version of this method did upset something, although I cannot reproduce that today. I will rerun some tests on these items... - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2025726278
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Thu, 28 Mar 2024 14:12:08 GMT, Andrew Dinn wrote: >>> It looks like the test only inspects a thread and a java object. Perhaps >>> you could add tests for additional VM objects. Maybe grab a frame PC from a >>> thread stack trace. >> >> Yes - added a couple of metadata tests, and a compiled method test, which >> gets an address from Events info. We know that should resolve to a compiled >> method (if we have such an event, so this copes if there are none). >> >> >> Also the VM.info and Thread.print runs with the CommandExecutor are now >> silent. They are long, and max out the test log file and make it truncate. >> That output could mainly be useful if the regexes can't match items, but >> the output format should be reproducible in a new run. Also if we fail to >> find something we need, like a thread id, it will print the full output it >> searched. > > @kevinjwalls Hi Kevin. Sorry to offer a drive-by comment but I have been > watching this thread and I can understand why @tstuefe is expressing his > concern about the potential for security issues when it comes to using jcmd > to expose JVM internals. > > Exposure of details like a thread/stack/metadata value address or a datum > embedded at such a location does look to me like something bad agents might > be able to profit from. The danger is not just being able to retrieve > specific details of the layout or content of JVM structures themselves, but > the opportunity to use that knowledge to upgrade a weak security hole like, > say, a memory exposure, into a stronger targeted attack that knows where or > to what it might want to apply the crowbar. > > So, first off, please understand that I am not suggesting there *is* a > problem here. I am happy to accept your conclusion that your proposed jcmd > changes do not expose new data to users who ought not to be able to view > either via local or via remote accesses. However, not being an expert when it > comes to the jcmd/DCmd implementation, I'm not sure I really understand why > that is so from your current explanation. In particular I don't understand > what you said about visibility of different commands for local and remote > access. > > That confession of my own ignorance is not really of any immediate importance > as I have no intention of trying to review this change. However, I still feel > it might be useful if you could summarize on this JIRA precisely what > security safeguards are currently in place when it comes to running specific > jcmds/DCmds and why that means exposure of JVM internal details via jcmd, > whether locally or remotely via JMX, will only be to a safe audience of > users. That would help any maintainer/developer who refers to this JIRA to > follow how those safeguards apply to the proposed commands (in particular it > might be of great help to those performing and assessing the correctness of > backports). However, it would probably also as a prophylactic to ensure that > any future development work does not inadvertently lead to unexpected > exposure, which I think is the thing Thomas is more worried about. Indeed, as > Thomas suggested, a clear statement of what policies and mechanisms are (or > should be) in plac e to ensure jcmd content is securely viewable might be a good starting point for you and/or Thomas to raise follow-up JIRAs if needed to: > 1. add comments to the code base to ensure devs to not inadvertently add or > modify new DCmds ... Hi @adinn Drive-bys are welcome. 8-) Providing new crowbars to attackers is not something we want to do. jcmd is protected by the attach api and is not open to other users. We are trusting in that. If you can satisfy the attach API connection, you have all the jcmds/DiagnosticCommands available. No limits. Likewise you can use other tools to examine abritrary memory in the target process, take a gcore, kill it, etc... Why will VM.inspect not be available remotely? Because this marks the jcmd as "hidden": src/hotspot/share/services/diagnosticCommand.cpp: DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl(full_export, true, true /* hidden */ )); (The new test for VM.inspect checks it does not appear in the jcmd help output, to ensure it stays hidden.) In JMX, DiagnosticCommandMBean(Impl) does not expose hidden commands: The route for that is DiagnosticCommandImpl.java calling its native getDiagnosticCommands(), that gets into management.cpp's jmm_GetDiagnosticCommands() calling DCmdFactory::DCmd_list(). In diagnosticFramework.cpp, this iterates DCmdFactories, choosing those which are not hidden. Querying over JMX I cannot see or access vmInspect, like I can see vmInfo, gcClassHistogram etc... through the exposed DiagnosticCommandMBean. That "hidden" flag could be more obvious or better documented perhaps. Also I don't think we actually say in the DiagnosticCommandMBean API docs that it provides access to the DiagnosticCommand
Integrated: 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java failed with BindException: Address already in use
On Mon, 25 Mar 2024 13:15:48 GMT, Kevin Walls wrote:
> Test uses jdk.test.lib.Utils.getFreePort() when launching a new Java command.
> Looks like it already recognises "java.rmi.server.ExportException: Port
> already in use: " and retries, but there is a long-standing typo in the check.
>
> e.g.
>
> test output:
> Error: Exception thrown by the agent: java.rmi.server.ExportException: Port
> already in use: 37049; nested exception is:
> java.net.BindException: Address already in use
>
> Test checks for:
> !output.getOutput().contains("Exception thrown by the agent :
> java.rmi.server.ExportException: Port already in use")
>
> Oops, we have an extra space in there. A day-one typo from JDK-7195249.
>
> While here, try to clarify the while loop which recognises this port failure.
> Also add something to clarify which test(s) failed, and correct a comment in
> test2 of AbstractFilePermissionTest.java
This pull request has now been integrated.
Changeset: 2af0312c
Author:Kevin Walls
URL:
https://git.openjdk.org/jdk/commit/2af0312c958e693b1377f4c014ae8f84cabf6b83
Stats: 19 lines in 1 file changed: 7 ins; 2 del; 10 mod
8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java
failed with BindException: Address already in use
Reviewed-by: lmesnik, dfuchs
-
PR: https://git.openjdk.org/jdk/pull/18470
Re: RFR: 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java failed with BindException: Address already in use [v2]
On Mon, 25 Mar 2024 22:46:47 GMT, Kevin Walls wrote:
>> Test uses jdk.test.lib.Utils.getFreePort() when launching a new Java
>> command.
>> Looks like it already recognises "java.rmi.server.ExportException: Port
>> already in use: " and retries, but there is a long-standing typo in the
>> check.
>>
>> e.g.
>>
>> test output:
>> Error: Exception thrown by the agent: java.rmi.server.ExportException: Port
>> already in use: 37049; nested exception is:
>> java.net.BindException: Address already in use
>>
>> Test checks for:
>> !output.getOutput().contains("Exception thrown by the agent :
>> java.rmi.server.ExportException: Port already in use")
>>
>> Oops, we have an extra space in there. A day-one typo from JDK-7195249.
>>
>> While here, try to clarify the while loop which recognises this port
>> failure. Also add something to clarify which test(s) failed, and correct a
>> comment in test2 of AbstractFilePermissionTest.java
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> show exit code
Thanks Daniel.
Yes, the variations of the starting-with-a-free-port problem will be nice to
standardize at some point... 8-)
-
PR Comment: https://git.openjdk.org/jdk/pull/18470#issuecomment-2024969513
Integrated: 8327505: Test com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.java fails
On Tue, 19 Mar 2024 16:23:27 GMT, Kevin Walls wrote: > Client.java has a fixed 30-second timeout on the CountDownLatch to wait for > 10 notifications. > > If it fails, you can't tell if CountDownLatch.await threw, or returned false > and the app threw InterruptedException, due to the way Client.java handles > these. > > Seems most likely the 30 second wait expired, as we are dealing with -Xcomp > failures in a debug build. Passing runs can take a few seconds, but can be > 25 seconds. > > Increasing the timeout and tidying up the handling so we can see the specific > reason in future. This pull request has now been integrated. Changeset: 2b79c22c Author:Kevin Walls URL: https://git.openjdk.org/jdk/commit/2b79c22c43a2de0815e77c9aa71f010906be8670 Stats: 7 lines in 1 file changed: 3 ins; 0 del; 4 mod 8327505: Test com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.java fails Reviewed-by: lmesnik, stuefe - PR: https://git.openjdk.org/jdk/pull/18381
Re: RFR: 8327505: Test com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.java fails
On Tue, 19 Mar 2024 16:23:27 GMT, Kevin Walls wrote: > Client.java has a fixed 30-second timeout on the CountDownLatch to wait for > 10 notifications. > > If it fails, you can't tell if CountDownLatch.await threw, or returned false > and the app threw InterruptedException, due to the way Client.java handles > these. > > Seems most likely the 30 second wait expired, as we are dealing with -Xcomp > failures in a debug build. Passing runs can take a few seconds, but can be > 25 seconds. > > Increasing the timeout and tidying up the handling so we can see the specific > reason in future. Thanks Thomas! - PR Comment: https://git.openjdk.org/jdk/pull/18381#issuecomment-2024715786
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
On Wed, 27 Mar 2024 05:44:25 GMT, Chris Plummer wrote: > It looks like the test only inspects a thread and a java object. Perhaps you > could add tests for additional VM objects. Maybe grab a frame PC from a > thread stack trace. Yes - added a couple of metadata tests, and a compiled method test, which gets an address from Events info. We know that should resolve to a compiled method (if we have such an event, so this copes if there are none). Also the VM.info and Thread.print runs with the CommandExecutor are now silent. They are long, and max out the test log file and make it truncate. That output could mainly be useful if the regexes can't match items, but the output format should be reproducible in a new run. Also if we fail to find something we need, like a thread id, it will print the full output it searched. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2023578758
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v11]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Test more pointer types: compiled method and metadata. - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/837154c1..9ed958f6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=10 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=09-10 Stats: 58 lines in 1 file changed: 46 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v10]
On Wed, 27 Mar 2024 12:23:50 GMT, Kevin Walls wrote: >> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object >> information. >> >> Not recommended for live production use. Requires UnlockDiagnosticVMOptions >> and not included in jcmd help output, to remind us this is not a >> general-purpose customer-facing tool. > > Kevin Walls has updated the pull request incrementally with two additional > commits since the last revision: > > - Argument to be named address. > - test nit Thanks for the comments, addressed nits and will update further on the remaining points. Thomas thanks for your comments. Yes, you do see stale CompilerCommand directives etc... hanging around at times, we can only encourage good housekeeping! Remote access over JMX is not required for this command (hidden). - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2023297192
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v10]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request incrementally with two additional commits since the last revision: - Argument to be named address. - test nit - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/bf5e4b26..837154c1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=09 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=08-09 Stats: 28 lines in 3 files changed: 6 ins; 3 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v8]
On Tue, 26 Mar 2024 20:52:30 GMT, Serguei Spitsyn wrote: >> Kevin Walls has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 20 additional >> commits since the last revision: >> >> - Change to jcmd VM.inspect >> - Merge remote-tracking branch 'upstream/master' into >> 8318026_jcmd_VMdebug_command >> - Test update >> - Show description if unknown subcommand. >> - Remove unnecessary 'events' subcommand. >> - Usage correction >> - Help to clarify this is VM inspection. Comment to relate source to >> debug.cpp. >> - jcheck trailing whitespace >> - Test update omitted from previous commit. >> - Merge remote-tracking branch 'upstream/master' into >> 8318026_jcmd_VMdebug_command >> - ... and 10 more: https://git.openjdk.org/jdk/compare/91a3bbb4...739bcbfa > > src/hotspot/share/services/diagnosticCommand.cpp line 59: > >> 57: #include "runtime/jniHandles.hpp" >> 58: #include "runtime/os.hpp" >> 59: #include "runtime/threads.hpp" > > Is the `#include` lines a leftover from the initial version? Do we still need > them? Yes, undoing that, thanks! - PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540153524
Re: RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Undo include - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/739bcbfa..bf5e4b26 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=08 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=07-08 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v15]
> The deprecated Subject Delegation feature in JMX will be removed. > > This was marked in JDK 21 as deprecated for removal (JDK-8298966). Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 24 commits: - Merge - Missing code doc nit. - Missing code doc nit. - RMIConnectionImpl_Stub also should explicity inherit the unchecked UOE. - Clarify JMXConnector equivalence comment. - RMIConnectionImpl needs to explicity inherit the unchecked UOE. - typo - Javadoc update - Remove unnecessary constructor in private class - Merge remote-tracking branch 'upstream/master' into 832_SubjectDelegation_remove - ... and 14 more: https://git.openjdk.org/jdk/compare/cc1800fa...903ce55b - Changes: https://git.openjdk.org/jdk/pull/18025/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18025=14 Stats: 2026 lines in 35 files changed: 214 ins; 1632 del; 180 mod Patch: https://git.openjdk.org/jdk/pull/18025.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18025/head:pull/18025 PR: https://git.openjdk.org/jdk/pull/18025
Re: RFR: 8318026: jcmd should provide access to low-level JVM debug information [v8]
> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object > information. > > Not recommended for live production use. Requires UnlockDiagnosticVMOptions > and not included in jcmd help output, to remind us this is not a > general-purpose customer-facing tool. Kevin Walls has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 20 additional commits since the last revision: - Change to jcmd VM.inspect - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - Test update - Show description if unknown subcommand. - Remove unnecessary 'events' subcommand. - Usage correction - Help to clarify this is VM inspection. Comment to relate source to debug.cpp. - jcheck trailing whitespace - Test update omitted from previous commit. - Merge remote-tracking branch 'upstream/master' into 8318026_jcmd_VMdebug_command - ... and 10 more: https://git.openjdk.org/jdk/compare/872dc5a5...739bcbfa - Changes: - all: https://git.openjdk.org/jdk/pull/17655/files - new: https://git.openjdk.org/jdk/pull/17655/files/3f566649..739bcbfa Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17655=07 - incr: https://webrevs.openjdk.org/?repo=jdk=17655=06-07 Stats: 458556 lines in 4892 files changed: 36637 ins; 91690 del; 330229 mod Patch: https://git.openjdk.org/jdk/pull/17655.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17655/head:pull/17655 PR: https://git.openjdk.org/jdk/pull/17655
Re: RFR: 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java failed with BindException: Address already in use [v2]
> Test uses jdk.test.lib.Utils.getFreePort() when launching a new Java command.
> Looks like it already recognises "java.rmi.server.ExportException: Port
> already in use: " and retries, but there is a long-standing typo in the check.
>
> e.g.
>
> test output:
> Error: Exception thrown by the agent: java.rmi.server.ExportException: Port
> already in use: 37049; nested exception is:
> java.net.BindException: Address already in use
>
> Test checks for:
> !output.getOutput().contains("Exception thrown by the agent :
> java.rmi.server.ExportException: Port already in use")
>
> Oops, we have an extra space in there. A day-one typo from JDK-7195249.
>
> While here, try to clarify the while loop which recognises this port failure.
> Also add something to clarify which test(s) failed, and correct a comment in
> test2 of AbstractFilePermissionTest.java
Kevin Walls has updated the pull request incrementally with one additional
commit since the last revision:
show exit code
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18470/files
- new: https://git.openjdk.org/jdk/pull/18470/files/78456dac..5047f7d4
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=18470=01
- incr: https://webrevs.openjdk.org/?repo=jdk=18470=00-01
Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod
Patch: https://git.openjdk.org/jdk/pull/18470.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18470/head:pull/18470
PR: https://git.openjdk.org/jdk/pull/18470
Re: RFR: 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java failed with BindException: Address already in use
On Mon, 25 Mar 2024 19:00:37 GMT, Chris Plummer wrote:
>> Test uses jdk.test.lib.Utils.getFreePort() when launching a new Java
>> command.
>> Looks like it already recognises "java.rmi.server.ExportException: Port
>> already in use: " and retries, but there is a long-standing typo in the
>> check.
>>
>> e.g.
>>
>> test output:
>> Error: Exception thrown by the agent: java.rmi.server.ExportException: Port
>> already in use: 37049; nested exception is:
>> java.net.BindException: Address already in use
>>
>> Test checks for:
>> !output.getOutput().contains("Exception thrown by the agent :
>> java.rmi.server.ExportException: Port already in use")
>>
>> Oops, we have an extra space in there. A day-one typo from JDK-7195249.
>>
>> While here, try to clarify the while loop which recognises this port
>> failure. Also add something to clarify which test(s) failed, and correct a
>> comment in test2 of AbstractFilePermissionTest.java
>
> test/jdk/sun/management/jmxremote/bootstrap/AbstractFilePermissionTest.java
> line 144:
>
>> 142:
>> 143: if (doTest() != 0) {
>> 144: System.out.println("FAILURE");
>
> Would be better to print out which test failed and include the error #.
Maybe very marginally useful. I'll add it. 8-)
It's the exit code of the JVM process, being zero or not zero, and the output
already printed before the "FAILURE" note that will be the guide as to what
went wrong.
If it's the port failure, you'll see the retries then the eventual FAILURE, so
that will be clear.
If the JVM failed to startup, it's more the output than the exit code that we
want.
I really wanted the FAILURE printed because it tests two things, and when you
see this test for the first time, it may not be obvious which of the tests is
the actual failure - particularly as test2 is expected to cause an error and a
non-zero exit code. So it says "1 failure", and you can stare at two things
that look like failures. 8-)
Maybe very marginally useful. I'll add it. 8-)
It's the exit code of the JVM process, being zero or not zero, and the output
already printed before the "FAILURE" note will be the guide as to what went
wrong. (If it's the port failure, you'll see the retries then the eventual
FAILURE, so that will be clear.)
If the JVM failed to startup, it's more the text output than the exit code that
we want.
I really wanted the FAILURE printed because it tests two things, and when you
see this test for the first time, it may not be obvious which of the tests is
the actual failure - particularly as test2 is expected to cause an error and a
non-zero exit code. So it says "1 failure", and you can stare at two things
that look like failures. 8-)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18470#discussion_r1538323593
RFR: 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java failed with BindException: Address already in use
Test uses jdk.test.lib.Utils.getFreePort() when launching a new Java command.
Looks like it already recognises "java.rmi.server.ExportException: Port already
in use: " and retries, but there is a long-standing typo in the check.
e.g.
test output:
Error: Exception thrown by the agent: java.rmi.server.ExportException: Port
already in use: 37049; nested exception is:
java.net.BindException: Address already in use
Test checks for:
!output.getOutput().contains("Exception thrown by the agent :
java.rmi.server.ExportException: Port already in use")
Oops, we have an extra space in there. A day-one typo from JDK-7195249.
While here, try to clarify the while loop which recognises this port failure.
Also add something to clarify which test(s) failed, and correct a comment in
test2 of AbstractFilePermissionTest.java
-
Commit messages:
- 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java
failed with BindException: Address already in use
Changes: https://git.openjdk.org/jdk/pull/18470/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=18470=00
Issue: https://bugs.openjdk.org/browse/JDK-8328619
Stats: 15 lines in 1 file changed: 5 ins; 2 del; 8 mod
Patch: https://git.openjdk.org/jdk/pull/18470.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18470/head:pull/18470
PR: https://git.openjdk.org/jdk/pull/18470
RFR: 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java failed with BindException: Address already in use
Test uses jdk.test.lib.Utils.getFreePort() when launching a new Java command.
Looks like it already recognises "java.rmi.server.ExportException: Port already
in use: " and retries, but there is a long-standing typo in the check.
e.g.
test output:
Error: Exception thrown by the agent: java.rmi.server.ExportException: Port
already in use: 37049; nested exception is:
java.net.BindException: Address already in use
Test checks for:
!output.getOutput().contains("Exception thrown by the agent :
java.rmi.server.ExportException: Port already in use")
Oops, we have an extra space in there. A day-one typo from JDK-7195249.
While here, try to clarify the while loop which recognises this port failure.
Also add something to clarify which test(s) failed, and correct a comment in
test2 of AbstractFilePermissionTest.java
-
Commit messages:
- 8328619: sun/management/jmxremote/bootstrap/SSLConfigFilePermissionTest.java
failed with BindException: Address already in use
Changes: https://git.openjdk.org/jdk/pull/18470/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=18470=00
Issue: https://bugs.openjdk.org/browse/JDK-8328619
Stats: 15 lines in 1 file changed: 5 ins; 2 del; 8 mod
Patch: https://git.openjdk.org/jdk/pull/18470.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18470/head:pull/18470
PR: https://git.openjdk.org/jdk/pull/18470
Re: RFR: 8327864: Support segmented heap dump for HotSpotDiagnosticMXBean
On Tue, 12 Mar 2024 07:59:12 GMT, Yi Yang wrote: > We've received feedback from users of cloud APM platform wanting the new > version of the JDK to allow the HotSpotDiagnosticMXBean.dumpHeap > underpinnings to reduce STW time using sgemented heapdump. Supporting > segmented heapdump for mxbean is a reasonable requirement and adds little > additional complexity, both in terms of maintainability and understanding. We should close this now the issue is marked as a duplicate, thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18221#issuecomment-2017694182
Re: RFR: 8318026: jcmd should provide access to low-level JVM debug information [v7]
On Tue, 5 Mar 2024 11:31:13 GMT, Kevin Walls wrote: >> Introduce the jcmd "VM.debug" to implement access to a useful set of the >> established debug.cpp utilities, with "jcmd PID VM.debug subcommand ...". >> >> Not recommended for live production use. Calling these "debug" utilities, >> and not including them in the jcmd help output, is to remind us they are not >> general customer-facing tools. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Test update Having worked with this for a while, I'm quite liking simply adding jcmds, and not adding a group of commands as I initially proposed. The VM.debug group had a story to it, as I mentioned, but maybe that's just history. A user just wants to use jcmd, and if VM.inspect is the command to inspect a JVM object, then OK. The history in debug.cpp may not be that interesting. The command group does make it easy to restrict the commands with UnlockDiagnosticVMOptions, for example, so that was a benefit, but minor as it's just a flag check. Another difference with the command group was you have to read the first argument and direct to the right utility, so you're doing some minor parsing rather than relying on the DiagnosticCommand parser, which is a minor downside. Ease of adding commands is generally similar either way. VM.inspect is the primary utility I want to add here. I plan to rework this do to that. Others can be added in separate jcmds as needs arise. - PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2015039530
Re: RFR: 8327505: Test com/sun/jmx/remote/NotificationMarshalVersions/TestSerializationMismatch.java fails
On Tue, 19 Mar 2024 16:23:27 GMT, Kevin Walls wrote: > Client.java has a fixed 30-second timeout on the CountDownLatch to wait for > 10 notifications. > > If it fails, you can't tell if CountDownLatch.await threw, or returned false > and the app threw InterruptedException, due to the way Client.java handles > these. > > Seems most likely the 30 second wait expired, as we are dealing with -Xcomp > failures in a debug build. Passing runs can take a few seconds, but can be > 25 seconds. > > Increasing the timeout and tidying up the handling so we can see the specific > reason in future. Thanks Leonid! - PR Comment: https://git.openjdk.org/jdk/pull/18381#issuecomment-2011885747
RE: RFD: Remove Hotspot-internal MBeans from sun.management
Hi, 8328618: HotspotInternalMBean should be removed https://bugs.openjdk.org/browse/JDK-8328618 I looked a bit further and logged this, with notes on what the HotSpotXX MBeans actually do. It is almost all about providing a Java MBean interface to perf counters. HotspotThread additionally provides thread count and thread CPU usage by “internal” threads, although the definition of that is maybe unclear. Thanks Kevin From: Kevin Walls Sent: 20 March 2024 10:53 To: Mandy Chung ; Eirik Bjørsnøs ; serviceability-dev@openjdk.org Cc: Volker Simonis ; Alan Bateman Subject: RE: RFD: Remove Hotspot-internal MBeans from sun.management Yes, I looked into it a little since you raised the previous issue, that looks like the right direction. HotspotInternalMBean should be removed (an undocumented, unsupported and experimental interface) HotspotInternalMBean creates: sun.management…. HotspotClassLoading HotspotMemory HotspotRuntime HotspotThread HotspotCompilation These have a peculiar way of accessing that we do not document as far as I can see. It would be good to look at whether anything in there is not adequately available by other MBeans, or jstat, or any other mechanism. Thanks Kevin From: Mandy Chung mailto:mandy.ch...@oracle.com>> Sent: 19 March 2024 16:48 To: Eirik Bjørsnøs mailto:eir...@gmail.com>>; serviceability-dev@openjdk.org<mailto:serviceability-dev@openjdk.org> Cc: Volker Simonis mailto:volker.simo...@gmail.com>>; Kevin Walls mailto:kevin.wa...@oracle.com>>; Alan Bateman mailto:alan.bate...@oracle.com>> Subject: Re: RFD: Remove Hotspot-internal MBeans from sun.management Would a PR to remove these APIs be welcome? Good with me. Mandy On 3/19/24 9:41 AM, Eirik Bjørsnøs wrote: Hi, Last September, Volker shared the observation that we have Hotspot-internal MBeans in sun.management which are strongly encapsulated and not used internally by OpenJDK besides their unit tests: https://www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html<https://urldefense.com/v3/__https:/www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html__;!!ACWV5N9M2RV99hQ!KfW2PiXn5Gb38tJmxUNxLW_fhwy1H7NijeBmgeWH0ZWNw6QriJmDY_ZD5miuxMQq9q0Rl_k6ZOdb-b0$> A summary of the email thread: Mandy pointed out: We added these HotSpot internal MBeans in JDK 5 to expose the internal metrics. Most of these internal metrics are exposed via jstat tool too. We didn't receive much feedback regarding these HotSpot internal MBeans.Removing them is fine and good cleanup effort. Alan made a similar point: It's left over from experiments on exposing some internal metrics, I think during JDK 5. It's code that should probably have been removed a long time ago. Kirk P raised a concern: It would be a shame to lose these metrics because many of them have been very useful over time and some would be even more useful with some modifications. To which Mandy responded: What we're referring to is to remove sun.management.Hotspot*, the internal MBeans which are never exposed and registered in the platform MBeanServer. The internal metrics in HotSpot VM should be retained as they are exposed through other ways like jstat, GC logs, etc. The email thread seems to have ended here without further action taken. My interpretation of the above is that we have a consensus that these Hotspot-internal MBeans can be removed. Since I was not part of the initial discussion and some time has passed, I'd like some confirmation that my interpretation is correct. Would a PR to remove these APIs be welcome? (This would remove HotspotInternalMBean, HotspotMemoryMBean, HotspotRuntimeMBean, HotspotThreadMBean, with associated implementation, factory methods, tests and probably also some native code in libmanagement. Details can be discussed in a PR) Cheers, Eirik.
RE: RFD: Remove Hotspot-internal MBeans from sun.management
Yes, I looked into it a little since you raised the previous issue, that looks like the right direction. HotspotInternalMBean should be removed (an undocumented, unsupported and experimental interface) HotspotInternalMBean creates: sun.management…. HotspotClassLoading HotspotMemory HotspotRuntime HotspotThread HotspotCompilation These have a peculiar way of accessing that we do not document as far as I can see. It would be good to look at whether anything in there is not adequately available by other MBeans, or jstat, or any other mechanism. Thanks Kevin From: Mandy Chung Sent: 19 March 2024 16:48 To: Eirik Bjørsnøs ; serviceability-dev@openjdk.org Cc: Volker Simonis ; Kevin Walls ; Alan Bateman Subject: Re: RFD: Remove Hotspot-internal MBeans from sun.management Would a PR to remove these APIs be welcome? Good with me. Mandy On 3/19/24 9:41 AM, Eirik Bjørsnøs wrote: Hi, Last September, Volker shared the observation that we have Hotspot-internal MBeans in sun.management which are strongly encapsulated and not used internally by OpenJDK besides their unit tests: https://www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html<https://urldefense.com/v3/__https:/www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html__;!!ACWV5N9M2RV99hQ!KfW2PiXn5Gb38tJmxUNxLW_fhwy1H7NijeBmgeWH0ZWNw6QriJmDY_ZD5miuxMQq9q0Rl_k6ZOdb-b0$> A summary of the email thread: Mandy pointed out: We added these HotSpot internal MBeans in JDK 5 to expose the internal metrics. Most of these internal metrics are exposed via jstat tool too. We didn't receive much feedback regarding these HotSpot internal MBeans.Removing them is fine and good cleanup effort. Alan made a similar point: It's left over from experiments on exposing some internal metrics, I think during JDK 5. It's code that should probably have been removed a long time ago. Kirk P raised a concern: It would be a shame to lose these metrics because many of them have been very useful over time and some would be even more useful with some modifications. To which Mandy responded: What we're referring to is to remove sun.management.Hotspot*, the internal MBeans which are never exposed and registered in the platform MBeanServer. The internal metrics in HotSpot VM should be retained as they are exposed through other ways like jstat, GC logs, etc. The email thread seems to have ended here without further action taken. My interpretation of the above is that we have a consensus that these Hotspot-internal MBeans can be removed. Since I was not part of the initial discussion and some time has passed, I'd like some confirmation that my interpretation is correct. Would a PR to remove these APIs be welcome? (This would remove HotspotInternalMBean, HotspotMemoryMBean, HotspotRuntimeMBean, HotspotThreadMBean, with associated implementation, factory methods, tests and probably also some native code in libmanagement. Details can be discussed in a PR) Cheers, Eirik.
