On Tue, 19 Nov 2024 14:24:15 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>> Remove redundant SecurityManager, AccessController references
>> (following on from JDK-8338411: Implement JEP 486: Permanently Disable the
>> Security Manager).
>>
>> src/jdk.management/share/classes/com/sun/management/internal/GarbageCollectionNotifInfoCompositeData.java
>> There is an existing theoretical path where GcInfoBuilder stays null, should
>> never happen, "com.sun.management.GcInfo" exists...
>>
>> src/jdk.management/share/classes/com/sun/management/internal/GcInfoCompositeData.java
>> Similarly there is an existing assumption that
>> Class.forName("com.sun.management.GcInfo") succeeds.
>
> Kevin Walls has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - space
> - Missed getBoolean
Changes requested by cjplummer (Reviewer).
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.
-------------
PR Review: https://git.openjdk.org/jdk/pull/22155#pullrequestreview-2447385562
PR Review Comment: https://git.openjdk.org/jdk/pull/22155#discussion_r1849535744
