RFR: 8344394: Remove SecurityManager and related calls from java.management.rmi [v2]

[Alex Menkov]

On Fri, 22 Nov 2024 19:19:37 GMT, Kevin Walls <kevinw at openjdk.org> wrote:

>> Remove redundant SecurityManager, AccessController references
>> (following on from JDK-8338411: Implement JEP 486: Permanently Disable the Security Manager).
>
> Kevin Walls has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove last import sun.reflect.misc.ReflectUtil

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 108:

> 106:         ClassLoaderRepository repository = mbeanServer.getClassLoaderRepository();
> 107:         this.classLoaderWithRepository = new ClassLoaderWithRepository(repository, dcl);
> 108:         this.defaultContextClassLoader = new CombinedClassLoader(Thread.currentThread().getContextClassLoader(), dcl);

`this.` are not needed

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1231:

> 1229:                 return getServerNotifFwd().fetchNotifs(clientSequenceNumber, timeout, maxNotifications);
> 1230:             } else {
> 1231:                 return Subject.callAs(subject, () -> getServerNotifFwd().fetchNotifs(clientSequenceNumber, timeout, maxNotifications));

This call may throw CompletionException instead of PrivilegedActionException
That's strange that this case does not go through standard `doPrivilegedOperation` way.
I think this class needs to be improved (most likely as a separate PR).
For most of operations it created "operation id" and creates array of arguments, but then `doOperation` parses the id and arguments and performs required action. It would be much simpler if doPrivilegedOperation/doOperation accept Callable.

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1313:

> 1311:                     Throwable e = ce.getCause();
> 1312:                     if (e instanceof SecurityException) {
> 1313:                         throw (SecurityException) e;

Suggestion:

                    if (e instanceof SecurityException se) {
                        throw se;

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1315:

> 1313:                         throw (SecurityException) e;
> 1314:                     } else if (e instanceof Exception) {
> 1315:                         throw new PrivilegedActionException((Exception) e);

Suggestion:

                    } else if (e instanceof Exception e1) {
                        throw new PrivilegedActionException(e1);

src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1484:

> 1482:                 setCcl(old);
> 1483:             }
> 1484:         } catch (Exception e) {

Need to handle CompletionException (or maybe at Subject.callAs call)?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22270#discussion_r1854766377
PR Review Comment: https://git.openjdk.org/jdk/pull/22270#discussion_r1854784308
PR Review Comment: https://git.openjdk.org/jdk/pull/22270#discussion_r1854786790
PR Review Comment: https://git.openjdk.org/jdk/pull/22270#discussion_r1854787507
PR Review Comment: https://git.openjdk.org/jdk/pull/22270#discussion_r1854789725