Re: RFR: 8305237: CompilerDirectives DCmds permissions correction
On Fri, 31 Mar 2023 08:24:19 GMT, Kevin Walls wrote: > The Permissions in DCmds relate to remote usage over JMX. > "monitor" is generally for reading information, and "control" is generally > for making changes. > The DCmds for changing compiler directives should have "control" as the > required permission. > > Tests in test/hotspot/jtreg/serviceability/dcmd/compiler and > test/hotspot/jtreg/compiler/compilercontrol still pass with this change. Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/13262#issuecomment-1496667870
Re: RFR: 8305237: CompilerDirectives DCmds permissions correction
On Fri, 31 Mar 2023 08:24:19 GMT, Kevin Walls wrote: > The Permissions in DCmds relate to remote usage over JMX. > "monitor" is generally for reading information, and "control" is generally > for making changes. > The DCmds for changing compiler directives should have "control" as the > required permission. > > Tests in test/hotspot/jtreg/serviceability/dcmd/compiler and > test/hotspot/jtreg/compiler/compilercontrol still pass with this change. Marked as reviewed by sspitsyn (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13262#pullrequestreview-1370387666
Re: RFR: 8305237: CompilerDirectives DCmds permissions correction
On Fri, 31 Mar 2023 08:24:19 GMT, Kevin Walls wrote: > The Permissions in DCmds relate to remote usage over JMX. > "monitor" is generally for reading information, and "control" is generally > for making changes. > The DCmds for changing compiler directives should have "control" as the > required permission. > > Tests in test/hotspot/jtreg/serviceability/dcmd/compiler and > test/hotspot/jtreg/compiler/compilercontrol still pass with this change. Marked as reviewed by cjplummer (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13262#pullrequestreview-1369628119
Re: RFR: 8305237: CompilerDirectives DCmds permissions correction
On Fri, 31 Mar 2023 08:24:19 GMT, Kevin Walls wrote:
> The Permissions in DCmds relate to remote usage over JMX.
> "monitor" is generally for reading information, and "control" is generally
> for making changes.
> The DCmds for changing compiler directives should have "control" as the
> required permission.
>
> Tests in test/hotspot/jtreg/serviceability/dcmd/compiler and
> test/hotspot/jtreg/compiler/compilercontrol still pass with this change.
Right, I don't think we test this remotely, and I don't think we test it with
permissions.
We have
test/hotspot/jtreg/serviceability/dcmd/compiler/CompilerDirectivesDCMDTest.java
which uses test/lib/jdk/test/lib/dcmd/JMXExecutor.java
...where JMXExecutor implements a CommandExecutor passed to the test's run
method
and the test does things like: executor.execute("Compiler.directives_clear");
CompilerDirectivesDCMDTest currently tests local usage in the running VM, which
is fine and this change does not affect it.
I don't really think this feature (remote, permissions) is likely to be
actively used, as it is not well documented, so this change is just to try and
correct the record of the intent of the feature. I think additional testing
should come as a next step as we make changes to the implementation required by
the upcoming removal of Security Manager and related APIs.
-
PR Comment: https://git.openjdk.org/jdk/pull/13262#issuecomment-1493278465
Re: RFR: 8305237: CompilerDirectives DCmds permissions correction
On Fri, 31 Mar 2023 08:24:19 GMT, Kevin Walls wrote: > The Permissions in DCmds relate to remote usage over JMX. > "monitor" is generally for reading information, and "control" is generally > for making changes. > The DCmds for changing compiler directives should have "control" as the > required permission. > > Tests in test/hotspot/jtreg/serviceability/dcmd/compiler and > test/hotspot/jtreg/compiler/compilercontrol still pass with this change. I assume this means we have no tests that try to change these compiler directives. Should we? - PR Comment: https://git.openjdk.org/jdk/pull/13262#issuecomment-1492504793
Re: RFR: 8305237: CompilerDirectives DCmds permissions correction
On Fri, 31 Mar 2023 08:24:19 GMT, Kevin Walls wrote: > The Permissions in DCmds relate to remote usage over JMX. > "monitor" is generally for reading information, and "control" is generally > for making changes. > The DCmds for changing compiler directives should have "control" as the > required permission. > > Tests in test/hotspot/jtreg/serviceability/dcmd/compiler and > test/hotspot/jtreg/compiler/compilercontrol still pass with this change. This has a lot of labels for a trivial change in a very niche feature, but they all seem relevant. - PR Comment: https://git.openjdk.org/jdk/pull/13262#issuecomment-1491551796
RFR: 8305237: CompilerDirectives DCmds permissions correction
The Permissions in DCmds relate to remote usage over JMX. "monitor" is generally for reading information, and "control" is generally for making changes. The DCmds for changing compiler directives should have "control" as the required permission. Tests in test/hotspot/jtreg/serviceability/dcmd/compiler and test/hotspot/jtreg/compiler/compilercontrol still pass with this change. - Commit messages: - 8305237: CompilerDirectives DCmds permissions correction Changes: https://git.openjdk.org/jdk/pull/13262/files Webrev: https://webrevs.openjdk.org/?repo=jdk=13262=00 Issue: https://bugs.openjdk.org/browse/JDK-8305237 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/13262.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13262/head:pull/13262 PR: https://git.openjdk.org/jdk/pull/13262
