Use direct mapping call in Kernel32Library#9923
Conversation
|
This looks much simpler to me! |
There was a problem hiding this comment.
Can we remove this boolean? There is only one constructor for this class, so if it succeeds, then we loaded. otherwise we have no object to call addConsoleCtrlHandler() on, so we can remove the check for it there, too.
There was a problem hiding this comment.
and i guess where we instantiate singleton, we just let Kernel32Library be null if it cant load? I dont know if its cleaner actually, it means callers woudl have to check for that. i just think its wierd to have all methods have to check isLoaded and do nothing. Currently there are only a few, but if the class increases...
There was a problem hiding this comment.
Ok, I understand your point. I think we can remove the boolean and the method isLoaded(). If the native lib cannot be loaded, the singleton is still instantiated and not null, but any call to a native method (or call to Kernel32Library.addConsoleCtrlHandler which calls the native SetConsoleCtrlHandler) will throw an UnsatisfiedLinkError which is completely acceptable to me since, well, we are doing native calls :)
|
@tlrx i added a comment, but this looks good to me. |
|
+1, looks great. |
This commit modifies the Kernel32Library to use direct mapping instead of a proxy class when doing native calls on Windows platforms. It also adds the "createSecurityManager" permission to the tests.policy file, and adds unit tests that should have failed when the Java security manager is enabled. Closes elastic#9802
This commit modifies the Kernel32Library to use direct mapping instead of a proxy class when doing native calls on Windows platforms. It also adds the "createSecurityManager" permission to the tests.policy file, and adds unit tests that should have failed when the Java security manager is enabled.
Closes #9802