RFR: 8344181: Remove SecurityManager and related calls from jdk.management and jdk.management.agent [v2]

[Kevin Walls]

On Wed, 20 Nov 2024 05:00:13 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Kevin Walls has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - space
>>  - Missed getBoolean
>
> src/jdk.management/share/classes/com/sun/management/internal/HotSpotDiagnostic.java line 189:
> 
>> 187:                 if (cause instanceof RuntimeException e)
>> 188:                     throw e;
>> 189:                 throw new RuntimeException(cause);
> 
> I think you've made an assumption that these are all related to lack of prividleges. I don't think that is the case, and they are all exceptions that can result from calling dumpThreads(). I think you need a try block around the dumpThreads() call that handles all these exceptions in the same manner.

OK yes it looks that way but I think we're OK:

Previously, IOException and RuntimeException were rethrown, and that is I think everything we expected to see wrapped in an PrivilegedActionException.
Anything else was wrapped in a RuntimeException, but outside of IOException which was declared to be thrown, surely it's going to be an unchecked RuntimeException...


Here in dumpThreads we declare IOException and Runtime is unchecked.
So those two get thrown as before, not sure if we need to catch anything else.


Experimenting: invoking dumpHeap through the HotSpotDiagnostic MBean, I can see:

javax.management.RuntimeMBeanException: java.lang.IllegalArgumentException: heapdump file must have .hprof extension
or
javax.management.MBeanException: java.io.IOException: Permission denied


It looks like IO or IAException (which is a RuntimeException) get handled OK, and the MBean invocation, which called dumpHeap, wraps them in other exceptions.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22155#discussion_r1850214900