From a34df7df0878c7ca1cd4d35a2e1d750951f073a4 Mon Sep 17 00:00:00 2001 From: ittaiz Date: Sun, 29 Dec 2013 09:54:55 +0200 Subject: [PATCH 1/2] Fixed issue where cancelling a scheduled future while it is running did not stop it from running again --- .../concurrent/DeterministicScheduler.java | 2 +- .../DeterministicSchedulerTests.java | 40 ++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/org/jmock/lib/concurrent/DeterministicScheduler.java b/src/org/jmock/lib/concurrent/DeterministicScheduler.java index 0dce41c90..2663ec709 100644 --- a/src/org/jmock/lib/concurrent/DeterministicScheduler.java +++ b/src/org/jmock/lib/concurrent/DeterministicScheduler.java @@ -65,7 +65,7 @@ public void runNextPendingCommand() { scheduledTask.run(); - if (scheduledTask.repeats()) { + if (!scheduledTask.isCancelled() && scheduledTask.repeats()) { deltaQueue.add(scheduledTask.repeatDelay, scheduledTask); } } diff --git a/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java b/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java index 684c7d14f..c67d9148c 100644 --- a/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java +++ b/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java @@ -1,15 +1,13 @@ package org.jmock.test.unit.lib.concurrent; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertThat; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicInteger; import org.jmock.Expectations; import org.jmock.Sequence; @@ -208,7 +206,37 @@ public void testCanCancelScheduledCommands() { scheduler.tick(2, TimeUnit.SECONDS); } - + + public void testCancellingARunningCommandStopsItFromRunningAgain() { + DeterministicScheduler deterministicScheduler = new DeterministicScheduler(); + SchedulerContainer schedulerContainer = new SchedulerContainer(deterministicScheduler); + schedulerContainer.scheduleSelfCancellingTask(1,TimeUnit.SECONDS); + deterministicScheduler.tick(2,TimeUnit.SECONDS); + assertThat("cancelling runnable run count",schedulerContainer.runCount(), is(1)); + } + public static class SchedulerContainer { + + private final ScheduledExecutorService scheduler; + private ScheduledFuture scheduledFuture; + private final AtomicInteger counter = new AtomicInteger(); + public SchedulerContainer(ScheduledExecutorService scheduler) { + this.scheduler = scheduler; + } + public void scheduleSelfCancellingTask(int interval, TimeUnit unit){ + scheduledFuture = scheduler.scheduleAtFixedRate( + new CancellingRunnable(), interval,interval,unit); + } + public int runCount(){ + return counter.get(); + } + private class CancellingRunnable implements Runnable{ + @Override + public void run() { + scheduledFuture.cancel(true); + counter.incrementAndGet(); + } + } + } static final int TIMEOUT_IGNORED = 1000; public void testCanScheduleCallablesAndGetTheirResultAfterTheyHaveBeenExecuted() throws Exception { From 4e31c8b4bc0a9fda4a2e3c3224823f15e6aab206 Mon Sep 17 00:00:00 2001 From: ittaiz Date: Mon, 30 Dec 2013 15:00:52 +0200 Subject: [PATCH 2/2] renamed SchedulerContainer to ObjectThatCancelsARepeatingTask (better describes what it does) --- .../lib/concurrent/DeterministicSchedulerTests.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java b/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java index c67d9148c..cac4136b0 100644 --- a/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java +++ b/test/org/jmock/test/unit/lib/concurrent/DeterministicSchedulerTests.java @@ -209,20 +209,20 @@ public void testCanCancelScheduledCommands() { public void testCancellingARunningCommandStopsItFromRunningAgain() { DeterministicScheduler deterministicScheduler = new DeterministicScheduler(); - SchedulerContainer schedulerContainer = new SchedulerContainer(deterministicScheduler); - schedulerContainer.scheduleSelfCancellingTask(1,TimeUnit.SECONDS); + ObjectThatCancelsARepeatingTask objectThatCancelsARepeatingTask = new ObjectThatCancelsARepeatingTask(deterministicScheduler); + objectThatCancelsARepeatingTask.scheduleATaskThatWillCancelItself(1, TimeUnit.SECONDS); deterministicScheduler.tick(2,TimeUnit.SECONDS); - assertThat("cancelling runnable run count",schedulerContainer.runCount(), is(1)); + assertThat("cancelling runnable run count", objectThatCancelsARepeatingTask.runCount(), is(1)); } - public static class SchedulerContainer { + public static class ObjectThatCancelsARepeatingTask { private final ScheduledExecutorService scheduler; private ScheduledFuture scheduledFuture; private final AtomicInteger counter = new AtomicInteger(); - public SchedulerContainer(ScheduledExecutorService scheduler) { + public ObjectThatCancelsARepeatingTask(ScheduledExecutorService scheduler) { this.scheduler = scheduler; } - public void scheduleSelfCancellingTask(int interval, TimeUnit unit){ + public void scheduleATaskThatWillCancelItself(int interval, TimeUnit unit){ scheduledFuture = scheduler.scheduleAtFixedRate( new CancellingRunnable(), interval,interval,unit); }