geode-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <>
Subject [jira] [Commented] (GEODE-2109) calling submit on ExecutionService can cause exceptions to be lost
Date Thu, 01 Dec 2016 18:08:59 GMT


ASF GitHub Bot commented on GEODE-2109:

Github user kirklund commented on the issue:
    We've been requiring unit tests or integration tests for all bug fixes. 
    At a minimum, I would recommend adding a new test that fails due to at least of the conditions
reported by GEODE-2109 and then passes with your changes. Example:
    import static org.assertj.core.api.Assertions.assertThat; 
    import org.apache.geode.test.junit.categories.UnitTest;
    import org.junit.Rule;
    import org.junit.Test;
    import org.junit.experimental.categories.Category;
    public class FederatingManagerSubmitTaskWithFailureTest {
      private FederatingManager manager;
      public SystemErrRule systemErrRule = new SystemErrRule().enableLog();
      public void submittedTaskShouldLogFailure() throws Exception {
        manager.submitTask(() -> {throw new Exception("error message");});
    The above test would require a couple things:
    1) some sort of @Before method which instantiates manager
    2) change FederatingManager.submitTask(...) to package scope so the unit test can call
it (right now FederatingManager is not a test-friendly class)

> calling submit on ExecutionService can cause exceptions to be lost
> ------------------------------------------------------------------
>                 Key: GEODE-2109
>                 URL:
>             Project: Geode
>          Issue Type: Bug
>          Components: regions
>            Reporter: Darrel Schneider
> Geode has a number of places that call submit on ExecutionService. The submit method
returns a Future object. If the caller makes sure it calls "get" on the Future then all is
well. But in many places geode is not calling get. In that case if the Runnable that was submitted
throws an exception it gets stored in the get and never logged. This can make it very hard
to diagnose problems.
> If the caller does not want to call get on the returned Future then it should instead
call the "execute" method. In that case the exception will be unhandled and the unhandled
exception handler code we have on the LoggingThreadGroup class will cause the exception to
be logged.
> Here are the places that should be changed to use execute instead of submit:
> org.apache.geode.internal.util.concurrent.CustomEntryConcurrentHashMap.clear()
> org.apache.geode.internal.cache.DiskStoreImpl.executeDiskStoreTask(Runnable)
> org.apache.geode.internal.cache.lru.HeapEvictor.onEvent(MemoryEvent)
> org.apache.geode.distributed.internal.InternalLocator.restartWithDS(InternalDistributedSystem,
> org.apache.geode.distributed.internal.FunctionExecutionPooledExecutor.FunctionExecutionPooledExecutor(BlockingQueue<Runnable>,
int, PoolStatHelper, ThreadFactory, int, boolean)
> org.apache.geode.internal.cache.PRHARedundancyProvider.scheduleCreateMissingBuckets()
> org.apache.geode.distributed.internal.InternalLocator.startSharedConfigurationService(GemFireCacheImpl)
> org.apache.geode.cache.client.internal.SingleHopClientExecutor.submitTask(Runnable)

This message was sent by Atlassian JIRA

View raw message