Repository: geode Updated Branches: refs/heads/develop cf0694499 -> 8ace5128f
GEODE-2228 FutureResult.get() does not check for cancellation prior to waiting for a result Adding cancellation checks to the get() methods in FutureResult. This prevents the TXLockService from hanging waiting for a FutureResult from another thread when none will ever appear. Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/8ace5128 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/8ace5128 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/8ace5128 Branch: refs/heads/develop Commit: 8ace5128f4c33db4df3acef888d975cb08a3601f Parents: cf06944 Author: Bruce Schuchardt <bschucha...@pivotal.io> Authored: Mon Dec 19 14:01:13 2016 -0800 Committer: Bruce Schuchardt <bschucha...@pivotal.io> Committed: Mon Dec 19 14:02:51 2016 -0800 ---------------------------------------------------------------------- .../internal/util/concurrent/FutureResult.java | 8 ++ .../util/concurrent/FutureResultJUnitTest.java | 78 ++++++++++++++++++++ 2 files changed, 86 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/8ace5128/geode-core/src/main/java/org/apache/geode/internal/util/concurrent/FutureResult.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/util/concurrent/FutureResult.java b/geode-core/src/main/java/org/apache/geode/internal/util/concurrent/FutureResult.java index 06c394e..463b3fb 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/util/concurrent/FutureResult.java +++ b/geode-core/src/main/java/org/apache/geode/internal/util/concurrent/FutureResult.java @@ -59,6 +59,10 @@ public class FutureResult implements Future { public Object get() throws InterruptedException { if (Thread.interrupted()) throw new InterruptedException(); // check in case latch is null + if (this.isCancelled) { + throw new CancellationException( + LocalizedStrings.FutureResult_FUTURE_WAS_CANCELLED.toLocalizedString()); + } if (this.latch != null) this.latch.await(); if (this.isCancelled) { @@ -71,6 +75,10 @@ public class FutureResult implements Future { public Object get(long timeout, TimeUnit unit) throws InterruptedException, TimeoutException { if (Thread.interrupted()) throw new InterruptedException(); // check in case latch is null + if (this.isCancelled) { + throw new CancellationException( + LocalizedStrings.FutureResult_FUTURE_WAS_CANCELLED.toLocalizedString()); + } if (this.latch != null) { if (!this.latch.await(unit.toMillis(timeout))) { throw new TimeoutException(); http://git-wip-us.apache.org/repos/asf/geode/blob/8ace5128/geode-core/src/test/java/org/apache/geode/internal/util/concurrent/FutureResultJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/util/concurrent/FutureResultJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/util/concurrent/FutureResultJUnitTest.java new file mode 100644 index 0000000..196438c --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/internal/util/concurrent/FutureResultJUnitTest.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF 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.apache.geode.internal.util.concurrent; + +import static org.junit.Assert.fail; + +import org.apache.geode.CancelCriterion; +import org.apache.geode.distributed.DistributedSystemDisconnectedException; +import org.apache.geode.test.junit.categories.UnitTest; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.util.concurrent.CancellationException; +import java.util.concurrent.TimeUnit; + +/** + * Unit tests for {@link FutureResult}. + + */ +@Category(UnitTest.class) +public class FutureResultJUnitTest { + + @Test + public void cancelledFutureResultThrowsExceptionOnGet() throws Exception { + TestCancelCriterion cancelCriterion = new TestCancelCriterion(); + FutureResult futureResult = new FutureResult(cancelCriterion); + cancelCriterion.cancelReason = "cancelling for test"; + try { + futureResult.get(1000, TimeUnit.MILLISECONDS); + fail("expected a DistributedSystemDisconnectedException to be thrown"); + } catch (DistributedSystemDisconnectedException e) { + // expected - thrown by the StoppableCountDownLatch in the FutureResult + } + cancelCriterion.cancelReason = ""; + futureResult = new FutureResult(cancelCriterion); + futureResult.cancel(false); + try { + futureResult.get(1000, TimeUnit.MILLISECONDS); + fail("expected a CancellationException to be thrown"); + } catch (CancellationException e) { + // expected - thrown by the FutureResult + } + } + + static class TestCancelCriterion extends CancelCriterion { + String cancelReason = ""; + @Override + public String cancelInProgress() { + return cancelReason; + } + + @Override + public RuntimeException generateCancelledException(Throwable e) { + String reason = cancelInProgress(); + if (reason == null) { + return null; + } else { + if (e == null) { + return new DistributedSystemDisconnectedException(reason); + } else { + return new DistributedSystemDisconnectedException(reason, e); + } + } + } + } +}
