RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed [v3]

[Kevin Walls]

On Tue, 11 Jun 2024 17:03:58 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Kevin Walls has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Sean comments
>
> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java line 1301:
> 
>> 1299:                     }
>> 1300:             };
>> 1301:             if (acc == null) {
> 
> This is a comment to all the code changes in this PR. I would recommend we make use of the `allowSecurityManager()` call everywhere when the behavior is different, like this:
> 
> if (SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
>     if (acc == null)
>         return action.run();
>     else
>         return AccessController.doPrivileged(action, acc);
> } else {
>     if (subject == null)
>         return action.run();
>     else
>         return Subject.doAs(subject, action);
> }
> 
> This makes me much more confident.

Thanks I'll go through the above comments and update - some of the changes I see are unnecessary and from when I was trying migrating to callAs, and doPrivileged, and yes they can be simpler.

On the allowSecurityMananger check: 
If we check in e.g. RMIConnectionImpl constructor, and if SM is allowed we assign an ACC.
Later, we know if we have a non-null ACC then SM is allowed.  Are you saying we should then check allowSecurityManager  again?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19624#discussion_r1635291389