Swiftdiff options
| author | Nick Hudson <nick.hudson@isode.com> | 2012-01-16 15:43:40 (GMT) |
|---|---|---|
| committer | Kevin Smith <git@kismith.co.uk> | 2012-01-16 15:54:31 (GMT) |
| commit | 3adee817bfcbd54fd13c4d946bedadeca661e9b1 (patch) | |
| tree | 57787bd6a45c1a207f776f883f6192f04c00d832 | |
| parent | 1c107332343c7ce4dadb02d69148d4232cebf965 (diff) | |
| download | stroke-3adee817bfcbd54fd13c4d946bedadeca661e9b1.zip stroke-3adee817bfcbd54fd13c4d946bedadeca661e9b1.tar.bz2 | |
Update JavaTimer class to guard against premature timer expiration
The JavaTimer class uses Thead.sleep() to wait a specified number of
milliseconds. Thread.sleep() requires the caller catch
InterruptedException, but in the original implementation, such an
exception would result in the code assuming that the specified time
had been reached.
So as things stood, if you e.g. set a timer for 60 seconds, then the
timer might generate its "onTick" signal before that 60 seconds had
elapsed.
This patch changes the code so that the method will wait until the
specified time has been reached.
The "milliseconds" parameters are also changed to "long", which is the
type used by the rest of the java library for millisecond values.
Added a bit of javadoc and a toString() method as well.
Note there is still a "FIXME" in the code which I've not addressed.
Test-information:
Tested in debugging setup; things seem to be working as expected.
| -rw-r--r-- | src/com/isode/stroke/network/JavaTimer.java | 54 | ||||
| -rw-r--r-- | src/com/isode/stroke/network/JavaTimerFactory.java | 4 | ||||
| -rw-r--r-- | src/com/isode/stroke/network/TimerFactory.java | 4 |
3 files changed, 44 insertions, 18 deletions
diff --git a/src/com/isode/stroke/network/JavaTimer.java b/src/com/isode/stroke/network/JavaTimer.java index bcd37d5..ac9e219 100644 --- a/src/com/isode/stroke/network/JavaTimer.java +++ b/src/com/isode/stroke/network/JavaTimer.java @@ -9,6 +9,8 @@ */ package com.isode.stroke.network; +import java.util.Date; + import com.isode.stroke.eventloop.Event; import com.isode.stroke.eventloop.EventLoop; @@ -18,28 +20,33 @@ class JavaTimer extends Timer { boolean running_ = true; private final EventLoop eventLoop_; - private final int milliseconds_; + private final long milliseconds_; - public TimerRunnable(EventLoop eventLoop, int milliseconds) { + public TimerRunnable(EventLoop eventLoop, long milliseconds) { eventLoop_ = eventLoop; milliseconds_ = milliseconds; } public void run() { - while (shouldEmit()) { + long endTime = new Date().getTime() + milliseconds_; + while (shouldEmit() && new Date().getTime() < endTime) { try { - Thread.sleep(milliseconds_); + long timeToWait = endTime - new Date().getTime(); + if (timeToWait > 0) { + Thread.sleep(milliseconds_); + } } catch (InterruptedException ex) { - /* If we were interrupted, either emit or don't, based on whether stop was called.*/ - } - if (shouldEmit()) { - eventLoop_.postEvent(new Event.Callback() { - public void run() { - onTick.emit(); - } - }); + // Needs to be caught, but won't break out of the loop + // unless end time reached or stop() has been called. } - } + } + if (shouldEmit()) { + eventLoop_.postEvent(new Event.Callback() { + public void run() { + onTick.emit(); + } + }); + } } @@ -52,10 +59,19 @@ class JavaTimer extends Timer { } } - public JavaTimer(EventLoop eventLoop, int milliseconds) { + /** + * Create a new JavaTimer + * @param eventLoop the caller's EventLoop. Should not be null. + * @param milliseconds length of delay. + */ + public JavaTimer(EventLoop eventLoop, long milliseconds) { timer_ = new TimerRunnable(eventLoop, milliseconds); } + /** + * Start the timer running. The timer will expire and generate a signal + * after the specified delay, unless {@link #stop()} has been called. + */ @Override public void start() { Thread thread = (new Thread(timer_)); @@ -63,10 +79,20 @@ class JavaTimer extends Timer { thread.start(); } + /** + * Cancel the timer. No signal will be generated. + */ @Override public void stop() { timer_.stop(); //FIXME: This needs to clear any remaining events out of the EventLoop queue. } + + @Override + public String toString() { + return "JavaTimer for " + timer_.milliseconds_ + + " milliseconds " + + (timer_.running_ ? "running" : "not running"); + } private final TimerRunnable timer_; } diff --git a/src/com/isode/stroke/network/JavaTimerFactory.java b/src/com/isode/stroke/network/JavaTimerFactory.java index 9326f76..7586e74 100644 --- a/src/com/isode/stroke/network/JavaTimerFactory.java +++ b/src/com/isode/stroke/network/JavaTimerFactory.java @@ -4,7 +4,7 @@ * See Documentation/Licenses/GPLv3.txt for more information. */ /* - * Copyright (c) 2010, Isode Limited, London, England. + * Copyright (c) 2010-2012, Isode Limited, London, England. * All rights reserved. */ @@ -18,7 +18,7 @@ public class JavaTimerFactory implements TimerFactory { eventLoop_ = eventLoop; } - public Timer createTimer(int milliseconds) { + public Timer createTimer(long milliseconds) { return new JavaTimer(eventLoop_, milliseconds); } diff --git a/src/com/isode/stroke/network/TimerFactory.java b/src/com/isode/stroke/network/TimerFactory.java index d3325ac..ac53e48 100644 --- a/src/com/isode/stroke/network/TimerFactory.java +++ b/src/com/isode/stroke/network/TimerFactory.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, Isode Limited, London, England. + * Copyright (c) 2010-2012, Isode Limited, London, England. * All rights reserved. */ /* @@ -11,5 +11,5 @@ package com.isode.stroke.network; public interface TimerFactory { - Timer createTimer(int milliseconds); + Timer createTimer(long milliseconds); } |