On Mon, 17 Jun 2024 12:54:44 GMT, Kevin Walls <kev...@openjdk.org> 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. > > Kevin Walls has updated the pull request incrementally with two additional > commits since the last revision: > > - style update > - whitespace The new version looks much better to me. I wonder if we can abstract away some of the repeated logic though. src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1314: > 1312: return AccessController.doPrivileged(action, acc); > 1313: } > 1314: } This piece of code is repeated at several places in similar but different forms - sometime we check for `SharedSecrets.getJavaLangAccess().allowSecurityManager()`, sometime for `! SharedSecrets.getJavaLangAccess().allowSecurityManager()`, sometime we check for `acc == null`, sometime for `acc != null` etc.. I wonder if we could abstract that to some utility method somewhere. Would that make sense? Maybe that's not possible due to using sometime `PrivelegedAction` and sometimes `PrivilegedExceptionAction` - but maybe we could use `PrivilegedExceptionAction` everywhere at the cost of handling `PrivilegedActionException`? If that doesn't prove useful (if handling `PrivilegedExceptionAction` adds again an equivalent amount of clutter) then maybe we could at least make sure we do the same checks in the same way (typically `acc == null` vs `acc != null`)? ------------- PR Review: https://git.openjdk.org/jdk/pull/19624#pullrequestreview-2118725434 PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1642864523
