Try not to lose stacktraces (#24426)

This adds `-XX:-OmitStackTraceInFastThrow` to the JVM arguments
which *should* prevent the JVM from omitting stack traces on
common exception sites. Even though these sites are common, we'd
still like the exceptions to debug them.

This also adds the flag when running tests and adapts some tests
that had workarounds for the absense of the flag.

Closes #24376
This commit is contained in:
Nik Everett 2017-05-02 11:34:12 -04:00 committed by GitHub
parent d0a10cf140
commit 3b47355e56
5 changed files with 19 additions and 13 deletions

View File

@ -469,6 +469,7 @@ class BuildPlugin implements Plugin<Project> {
heapdumpDir.mkdirs() heapdumpDir.mkdirs()
jvmArg '-XX:HeapDumpPath=' + heapdumpDir jvmArg '-XX:HeapDumpPath=' + heapdumpDir
argLine System.getProperty('tests.jvm.argline') argLine System.getProperty('tests.jvm.argline')
argLine '-XX:-OmitStackTraceInFastThrow'
// we use './temp' since this is per JVM and tests are forbidden from writing to CWD // we use './temp' since this is per JVM and tests are forbidden from writing to CWD
systemProperty 'java.io.tmpdir', './temp' systemProperty 'java.io.tmpdir', './temp'

View File

@ -62,6 +62,10 @@
# use our provided JNA always versus the system one # use our provided JNA always versus the system one
-Djna.nosys=true -Djna.nosys=true
# turn off a JDK optimization that throws away stack traces for common
# exceptions because stack traces are important for debugging
-XX:-OmitStackTraceInFastThrow
# use old-style file permissions on JDK9 # use old-style file permissions on JDK9
-Djdk.io.permissionsUseCanonicalPath=true -Djdk.io.permissionsUseCanonicalPath=true

View File

@ -80,9 +80,14 @@ public abstract class ArrayLikeObjectTestCase extends ScriptTestCase {
IndexOutOfBoundsException e = expectScriptThrows(IndexOutOfBoundsException.class, () -> IndexOutOfBoundsException e = expectScriptThrows(IndexOutOfBoundsException.class, () ->
exec(script, singletonMap("val", val), true)); exec(script, singletonMap("val", val), true));
try { try {
/* If this fails you *might* be missing -XX:-OmitStackTraceInFastThrow in the test jvm
* In Eclipse you can add this by default by going to Preference->Java->Installed JREs,
* clicking on the default JRE, clicking edit, and adding the flag to the
* "Default VM Arguments".
*/
assertThat(e.getMessage(), outOfBoundsExceptionMessageMatcher(index, 5)); assertThat(e.getMessage(), outOfBoundsExceptionMessageMatcher(index, 5));
} catch (AssertionError ae) { } catch (AssertionError ae) {
ae.addSuppressed(e); // Mark the exception we are testing as suppressed so we get its stack trace. If it has one :( ae.addSuppressed(e); // Mark the exception we are testing as suppressed so we get its stack trace.
throw ae; throw ae;
} }
} }

View File

@ -25,9 +25,7 @@ import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
/** Tests for working with lists. */ /** Tests for working with lists. */
public class ListTests extends ArrayLikeObjectTestCase { public class ListTests extends ArrayLikeObjectTestCase {
@ -61,10 +59,7 @@ public class ListTests extends ArrayLikeObjectTestCase {
if (index > size) { if (index > size) {
return equalTo("Index: " + index + ", Size: " + size); return equalTo("Index: " + index + ", Size: " + size);
} }
Matcher<String> matcher = equalTo(Integer.toString(index)); return equalTo(Integer.toString(index));
// If we set -XX:-OmitStackTraceInFastThrow we wouldn't need this
matcher = anyOf(matcher, nullValue());
return matcher;
} else { } else {
// This exception is locale dependent so we attempt to reproduce it // This exception is locale dependent so we attempt to reproduce it
List<Object> list = new ArrayList<>(); List<Object> list = new ArrayList<>();

View File

@ -128,12 +128,13 @@ public abstract class ScriptTestCase extends ESTestCase {
if (e instanceof ScriptException) { if (e instanceof ScriptException) {
boolean hasEmptyScriptStack = ((ScriptException) e).getScriptStack().isEmpty(); boolean hasEmptyScriptStack = ((ScriptException) e).getScriptStack().isEmpty();
if (shouldHaveScriptStack && hasEmptyScriptStack) { if (shouldHaveScriptStack && hasEmptyScriptStack) {
if (0 != e.getCause().getStackTrace().length) { /* If this fails you *might* be missing -XX:-OmitStackTraceInFastThrow in the test jvm
// Without -XX:-OmitStackTraceInFastThrow the jvm can eat the stack trace which causes us to ignore script_stack * In Eclipse you can add this by default by going to Preference->Java->Installed JREs,
AssertionFailedError assertion = new AssertionFailedError("ScriptException should have a scriptStack"); * clicking on the default JRE, clicking edit, and adding the flag to the
assertion.initCause(e); * "Default VM Arguments". */
throw assertion; AssertionFailedError assertion = new AssertionFailedError("ScriptException should have a scriptStack");
} assertion.initCause(e);
throw assertion;
} else if (false == shouldHaveScriptStack && false == hasEmptyScriptStack) { } else if (false == shouldHaveScriptStack && false == hasEmptyScriptStack) {
AssertionFailedError assertion = new AssertionFailedError("ScriptException shouldn't have a scriptStack"); AssertionFailedError assertion = new AssertionFailedError("ScriptException shouldn't have a scriptStack");
assertion.initCause(e); assertion.initCause(e);