-
Notifications
You must be signed in to change notification settings - Fork 24.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Handle throws on tasks submitted to thread pools
When we submit a task to a thread pool for asynchronous execution, we are returned a future. Since we submitted to go asynchronous, these futures are not inspected for failure (we would have to block a thread to do that). While we have on failure handlers for exceptions that are thrown during execution, we do not handle throwables that are not exceptions and these end up silently lost. This commit adds a check after the runnable returns that inspects the status of the future. If an unhandled throwable occurred during execution, this throwable is propogated out where it will land in the uncaught exception handler. Relates #28667
- Loading branch information
1 parent
3e17185
commit 3e846ab
Showing
2 changed files
with
142 additions
and
0 deletions.
There are no files selected for viewing
105 changes: 105 additions & 0 deletions
105
qa/evil-tests/src/test/java/org/elasticsearch/threadpool/EvilThreadPoolTests.java
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
package org.elasticsearch.threadpool; | ||
|
||
import org.elasticsearch.test.ESTestCase; | ||
import org.junit.After; | ||
import org.junit.Before; | ||
|
||
import java.util.Optional; | ||
import java.util.concurrent.CountDownLatch; | ||
import java.util.concurrent.atomic.AtomicReference; | ||
import java.util.function.Consumer; | ||
import java.util.function.Supplier; | ||
|
||
import static org.hamcrest.Matchers.containsString; | ||
import static org.hamcrest.Matchers.hasToString; | ||
import static org.hamcrest.Matchers.instanceOf; | ||
|
||
public class EvilThreadPoolTests extends ESTestCase { | ||
|
||
private ThreadPool threadPool; | ||
|
||
@Before | ||
public void setUpThreadPool() { | ||
threadPool = new TestThreadPool(EvilThreadPoolTests.class.getName()); | ||
} | ||
|
||
@After | ||
public void tearDownThreadPool() throws InterruptedException { | ||
terminate(threadPool); | ||
} | ||
|
||
public void testExecutionException() throws InterruptedException { | ||
runExecutionExceptionTest( | ||
() -> { | ||
throw new Error("future error"); | ||
}, | ||
true, | ||
o -> { | ||
assertTrue(o.isPresent()); | ||
assertThat(o.get(), instanceOf(Error.class)); | ||
assertThat(o.get(), hasToString(containsString("future error"))); | ||
}); | ||
runExecutionExceptionTest( | ||
() -> { | ||
throw new IllegalStateException("future exception"); | ||
}, | ||
false, | ||
o -> assertFalse(o.isPresent())); | ||
} | ||
|
||
private void runExecutionExceptionTest( | ||
final Runnable runnable, | ||
final boolean expectThrowable, | ||
final Consumer<Optional<Throwable>> consumer) throws InterruptedException { | ||
final AtomicReference<Throwable> throwableReference = new AtomicReference<>(); | ||
final Thread.UncaughtExceptionHandler uncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler(); | ||
final CountDownLatch uncaughtExceptionHandlerLatch = new CountDownLatch(1); | ||
|
||
try { | ||
Thread.setDefaultUncaughtExceptionHandler((t, e) -> { | ||
assertTrue(expectThrowable); | ||
throwableReference.set(e); | ||
uncaughtExceptionHandlerLatch.countDown(); | ||
}); | ||
|
||
final CountDownLatch supplierLatch = new CountDownLatch(1); | ||
|
||
threadPool.generic().submit(() -> { | ||
try { | ||
runnable.run(); | ||
} finally { | ||
supplierLatch.countDown(); | ||
} | ||
}); | ||
|
||
supplierLatch.await(); | ||
|
||
if (expectThrowable) { | ||
uncaughtExceptionHandlerLatch.await(); | ||
} | ||
consumer.accept(Optional.ofNullable(throwableReference.get())); | ||
} finally { | ||
Thread.setDefaultUncaughtExceptionHandler(uncaughtExceptionHandler); | ||
} | ||
} | ||
|
||
} |
This file contains 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