Mark halting the virtual machine as privileged#19923
Merged
jasontedor merged 2 commits intoelastic:masterfrom Aug 11, 2016
jasontedor:privileged-exit
Merged
Mark halting the virtual machine as privileged#19923jasontedor merged 2 commits intoelastic:masterfrom jasontedor:privileged-exit
jasontedor merged 2 commits intoelastic:masterfrom
jasontedor:privileged-exit
Conversation
Today in the uncaught exception handler, we attempt to halt the virtual machine on fatal errors. Yet, halting the virtual machine requires privileges which might not be granted to the caller when the exception is thrown for example from a scripting engine. This means that if an OutOfMemoryError or another fatal error is hit inside a script, the virtual machine will not exit because the halt call will be denied for securiry privileges. In this commit, we mark this halt call as trusted so that the virtual machine can be halted if a fatal error is encountered in a script.
jasontedor
added
review
:Core/Infra/Core
Member
|
LGTM |
This commit removes a suppress forbidden annotation to the right place. This suppress forbidden annotation appeared in the wrong place after the forbidden method that it was suppressing was moved to a method on an anonymous class.
Contributor
|
+1 I think individual scripting impls should handle the cases they know are safe but this should be the "default". For example, I think StackOverflowError/OOM is actually fine to catch within a painless script, because we don't have any concurrency primitives or even really any state at all, besides our inline cache. We can open an issue to catch those and handle them more gracefully, since we control how things work there and we know that no data structure corruption can happen. |
dakrone
added a commit
to dakrone/elasticsearch
that referenced
this pull request
Aug 12, 2016
Previously, we only caught subclasses of Exception, however, there are some cases when Errors are thrown instead of Exceptions. These two cases are `assert` and when a class cannot be found. Without this change, the exception would bubble up to the `uncaughtExceptionHandler`, which would in turn, exit the JVM (related: elastic#19923). A note of difference between regular Java asserts and Groovy asserts, from http://docs.groovy-lang.org/docs/latest/html/documentation/core-testing-guide.html "Another important difference from Java is that in Groovy assertions are enabled by default. It has been a language design decision to remove the possibility to deactivate assertions." In the event that a user uses an assert such as: ```groovy def bar=false; assert bar, "message"; ``` The GroovyScriptEngineService throws a NoClassDefFoundError being unable to find the `java.lang.StringBuffer` class. It is *highly* recommended that any Groovy scripting user switch to using regular exceptions rather than unconfiguration Groovy assertions. Resolves elastic#19806
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today in the uncaught exception handler, we attempt to halt the virtual
machine on fatal errors. Yet, halting the virtual machine requires
privileges which might not be granted to the caller when the exception
is thrown for example from a scripting engine. This means that if an
OutOfMemoryError or another fatal error is hit inside a script, the
virtual machine will not exit because the halt call will be denied for
securiry privileges. In this commit, we mark this halt call as trusted
so that the virtual machine can be halted if a fatal error is
encountered in a script.
Relates #19272, relates #19806