Fixes #12318 SecurityUtils should not elminate calls to existing methods #12319
Conversation
| @@ -93,8 +83,7 @@ public static Object getSecurityManager() | |||
| { | |||
| try | |||
| { | |||
| if (!USE_SECURITY_MANAGER) | |||
| return null; | |||
| // TODO cache this method on class loading like the other two methods ? | |||
| @@ -113,8 +102,7 @@ public static Object getSecurityManager() | |||
| */ | |||
| public static void checkPermission(Permission permission) throws SecurityException | |||
| { | |||
| if (!USE_SECURITY_MANAGER) | |||
| return; | |||
| // TODO cache this methodon class loading like the other two methods ? | |||
| @@ -142,9 +130,11 @@ public static void checkPermission(Permission permission) throws SecurityExcepti | |||
| */ | |||
| public static <T> T doPrivileged(PrivilegedAction<T> action) | |||
| { | |||
| if (!USE_SECURITY_MANAGER || doPrivileged == null) | |||
| // Keep this method short and inlineable. | |||
| MethodHandle methodHandle = doPrivileged; | |||
There was a problem hiding this comment.
not sure why the local methodHandle variable is needed? doPrivileged is final.
There was a problem hiding this comment.
I'm not sure either, it's in 12.0 where I copied this from.
Perhaps its required for Java to automatically inline the function ? I don't know much about the automatic inlining behaviour.
I have removed it.
stoty
force-pushed
the
revertUseSecurityManager
branch
from
3b4e520 to
0a25638
Compare
|
Thank you for the review, I have tried to address your comments. Shouldn't we use CAPS for the methodHandle constants ? |
We have a bit of a mixed usage of CAPS for |
|
@olamy can you look at CI failure? Doesn't look like a flake, but then doesn't look related to this PR either. |
|
@gregw build is fine @stoty please have a read here https://www.eclipse.org/legal/eca/ once you have signed let us know and we will revalidate the PR |
|
Thank you. |
@stoty perfect. PR validated Thanks! |
No description provided.