From d0808c7148484e89944d740850e0d2c580b313eb Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 27 Oct 2015 23:37:39 -0400 Subject: [PATCH] Remove uncaught exception handler in tests. This is not needed: full mvn verify passes. Furthermore, there are all kinds of checks for this case (rejected while shutting down) in the actual code, so there is no need to have it here. If its supposed to be non-fatal, then we add the missing places to the actual code, not globally to all threads. --- .../elasticsearch/bootstrap/security.policy | 4 -- .../bootstrap/test-framework.policy | 3 +- .../index/analysis/PatternAnalyzerTests.java | 42 +------------------ .../elasticsearch/test/ESIntegTestCase.java | 1 - .../org/elasticsearch/test/ESTestCase.java | 38 ----------------- 5 files changed, 3 insertions(+), 85 deletions(-) diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy index 244d5be6511..8d9581409c0 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -73,10 +73,6 @@ grant { // otherwise can be provided only to test libraries permission java.lang.RuntimePermission "getStackTrace"; - // needed by ESTestCase for leniency of thread exceptions (?!) - // otherwise can be provided only to test libraries - permission java.lang.RuntimePermission "setDefaultUncaughtExceptionHandler"; - // needed by JMX instead of getFileSystemAttributes, seems like a bug... permission java.lang.RuntimePermission "getFileStoreAttributes"; diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy index f038c51c596..d6de7cd88eb 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy @@ -38,7 +38,8 @@ grant codeBase "${codebase.lucene-test-framework-5.4.0-snapshot-1708254.jar}" { grant codeBase "${codebase.randomizedtesting-runner-2.1.17.jar}" { // optionally needed for access to private test methods (e.g. beforeClass) permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; - + // needed to fail tests on uncaught exceptions from other threads + permission java.lang.RuntimePermission "setDefaultUncaughtExceptionHandler"; // needed for top threads handling permission java.lang.RuntimePermission "modifyThreadGroup"; }; diff --git a/core/src/test/java/org/elasticsearch/index/analysis/PatternAnalyzerTests.java b/core/src/test/java/org/elasticsearch/index/analysis/PatternAnalyzerTests.java index 9c578ef6385..6fa2e21fbd1 100644 --- a/core/src/test/java/org/elasticsearch/index/analysis/PatternAnalyzerTests.java +++ b/core/src/test/java/org/elasticsearch/index/analysis/PatternAnalyzerTests.java @@ -20,7 +20,6 @@ package org.elasticsearch.index.analysis; */ import java.io.IOException; -import java.lang.Thread.UncaughtExceptionHandler; import java.util.Arrays; import java.util.regex.Pattern; @@ -110,45 +109,6 @@ public class PatternAnalyzerTests extends ESTokenStreamTestCase { /** blast some random strings through the analyzer */ public void testRandomStrings() throws Exception { Analyzer a = new PatternAnalyzer(Pattern.compile(","), true, StopAnalyzer.ENGLISH_STOP_WORDS_SET); - - // dodge jre bug http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7104012 - final UncaughtExceptionHandler savedHandler = Thread.getDefaultUncaughtExceptionHandler(); - Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() { - @Override - public void uncaughtException(Thread thread, Throwable throwable) { - assumeTrue("not failing due to jre bug ", !isJREBug7104012(throwable)); - // otherwise its some other bug, pass to default handler - savedHandler.uncaughtException(thread, throwable); - } - }); - - try { - Thread.getDefaultUncaughtExceptionHandler(); - checkRandomData(random(), a, 10000*RANDOM_MULTIPLIER); - } catch (ArrayIndexOutOfBoundsException ex) { - assumeTrue("not failing due to jre bug ", !isJREBug7104012(ex)); - throw ex; // otherwise rethrow - } finally { - Thread.setDefaultUncaughtExceptionHandler(savedHandler); - } - } - - static boolean isJREBug7104012(Throwable t) { - if (!(t instanceof ArrayIndexOutOfBoundsException)) { - // BaseTokenStreamTestCase now wraps exc in a new RuntimeException: - t = t.getCause(); - if (!(t instanceof ArrayIndexOutOfBoundsException)) { - return false; - } - } - StackTraceElement trace[] = t.getStackTrace(); - for (StackTraceElement st : trace) { - if ("java.text.RuleBasedBreakIterator".equals(st.getClassName()) || - "sun.util.locale.provider.RuleBasedBreakIterator".equals(st.getClassName()) - && "lookupBackwardState".equals(st.getMethodName())) { - return true; - } - } - return false; + checkRandomData(random(), a, 10000*RANDOM_MULTIPLIER); } } diff --git a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java index ae81c8d679c..b9d935c4628 100644 --- a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java @@ -325,7 +325,6 @@ public abstract class ESIntegTestCase extends ESTestCase { } protected final void beforeInternal() throws Exception { - assert Thread.getDefaultUncaughtExceptionHandler() instanceof ElasticsearchUncaughtExceptionHandler; final Scope currentClusterScope = getCurrentClusterScope(); switch (currentClusterScope) { case SUITE: diff --git a/core/src/test/java/org/elasticsearch/test/ESTestCase.java b/core/src/test/java/org/elasticsearch/test/ESTestCase.java index d77abdb3977..c5e87f4a689 100644 --- a/core/src/test/java/org/elasticsearch/test/ESTestCase.java +++ b/core/src/test/java/org/elasticsearch/test/ESTestCase.java @@ -145,20 +145,6 @@ public abstract class ESTestCase extends LuceneTestCase { PathUtilsForTesting.teardown(); } - // setup a default exception handler which knows when and how to print a stacktrace - private static Thread.UncaughtExceptionHandler defaultHandler; - - @BeforeClass - public static void setDefaultExceptionHandler() throws Exception { - defaultHandler = Thread.getDefaultUncaughtExceptionHandler(); - Thread.setDefaultUncaughtExceptionHandler(new ElasticsearchUncaughtExceptionHandler(defaultHandler)); - } - - @AfterClass - public static void restoreDefaultExceptionHandler() throws Exception { - Thread.setDefaultUncaughtExceptionHandler(defaultHandler); - } - // randomize content type for request builders @BeforeClass @@ -551,30 +537,6 @@ public abstract class ESTestCase extends LuceneTestCase { return builder; } - // ----------------------------------------------------------------- - // Failure utilities - // ----------------------------------------------------------------- - - static final class ElasticsearchUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler { - - private final Thread.UncaughtExceptionHandler parent; - private final ESLogger logger = Loggers.getLogger(getClass()); - - private ElasticsearchUncaughtExceptionHandler(Thread.UncaughtExceptionHandler parent) { - this.parent = parent; - } - - @Override - public void uncaughtException(Thread t, Throwable e) { - if (e instanceof EsRejectedExecutionException) { - if (e.getMessage() != null && ((EsRejectedExecutionException) e).isExecutorShutdown()) { - return; // ignore the EsRejectedExecutionException when a node shuts down - } - } - parent.uncaughtException(t, e); - } - } - private static String threadName(Thread t) { return "Thread[" + "id=" + t.getId() +