Skip to content

Commit

Permalink
Fixed Timer tasks being run after cancellation.
Browse files Browse the repository at this point in the history
  • Loading branch information
@NathanSweet
NathanSweet committed Apr 28, 2024
1 parent b0a3e8b commit bba4270
Show file tree
Hide file tree
Showing 2 changed files with 242 additions and 25 deletions.
85 changes: 63 additions & 22 deletions gdx/src/com/badlogic/gdx/utils/Timer.java
View file
Expand Up @@ -27,6 +27,7 @@ public class Timer {
// TimerThread access is synchronized using threadLock.
// Timer access is synchronized using the Timer instance.
// Task access is synchronized using the Task instance.
// Posted tasks are synchronized using TimerThread#postedTasks.

static final Object threadLock = new Object();
static TimerThread thread;
Expand Down Expand Up @@ -118,15 +119,20 @@ public void start () {
}

/** Cancels all tasks. */
public synchronized void clear () {
for (int i = 0, n = tasks.size; i < n; i++) {
Task task = tasks.get(i);
synchronized (task) {
task.executeTimeMillis = 0;
task.timer = null;
public void clear () {
synchronized (threadLock) {
TimerThread thread = thread();
synchronized (this) {
synchronized (thread.postedTasks) {
for (int i = 0, n = tasks.size; i < n; i++) {
Task task = tasks.get(i);
thread.removePostedTask(task);
task.reset();
}
}
tasks.clear();
}
}
tasks.clear();
}

/** Returns true if the timer has no tasks in the queue. Note that this can change at any time. Synchronize on the timer
Expand All @@ -135,7 +141,7 @@ public synchronized boolean isEmpty () {
return tasks.size == 0;
}

synchronized long update (long timeMillis, long waitMillis) {
synchronized long update (TimerThread thread, long timeMillis, long waitMillis) {
for (int i = 0, n = tasks.size; i < n; i++) {
Task task = tasks.get(i);
synchronized (task) {
Expand All @@ -153,7 +159,7 @@ synchronized long update (long timeMillis, long waitMillis) {
waitMillis = Math.min(waitMillis, task.intervalMillis);
if (task.repeatCount > 0) task.repeatCount--;
}
task.app.postRunnable(task);
thread.addPostedTask(task);
}
}
return waitMillis;
Expand Down Expand Up @@ -212,23 +218,24 @@ public Task () {

/** Cancels the task. It will not be executed until it is scheduled again. This method can be called at any time. */
public void cancel () {
Timer timer = this.timer;
if (timer != null) {
synchronized (timer) {
synchronized (this) {
executeTimeMillis = 0;
this.timer = null;
synchronized (threadLock) {
thread().removePostedTask(this);
Timer timer = this.timer;
if (timer != null) {
synchronized (timer) {
timer.tasks.removeValue(this, true);
reset();
}
}
} else {
synchronized (this) {
executeTimeMillis = 0;
this.timer = null;
}
} else
reset();
}
}

synchronized void reset () {
executeTimeMillis = 0;
this.timer = null;
}

/** Returns true if this task is scheduled to be executed in the future by a timer. The execution time may be reached at any
* time after calling this method, which may change the scheduled state. To prevent the scheduled state from changing,
* synchronize on this task object, eg:
Expand Down Expand Up @@ -258,6 +265,13 @@ static class TimerThread implements Runnable, LifecycleListener {
Timer instance;
long pauseTimeMillis;

final Array<Task> postedTasks = new Array(2);
private final Runnable runPostedTasks = new Runnable() {
public void run () {
runPostedTasks();
}
};

public TimerThread () {
files = Gdx.files;
app = Gdx.app;
Expand All @@ -279,7 +293,7 @@ public void run () {
long timeMillis = System.nanoTime() / 1000000;
for (int i = 0, n = instances.size; i < n; i++) {
try {
waitMillis = instances.get(i).update(timeMillis, waitMillis);
waitMillis = instances.get(i).update(this, timeMillis, waitMillis);
} catch (Throwable ex) {
throw new GdxRuntimeException("Task failed: " + instances.get(i).getClass().getName(), ex);
}
Expand All @@ -297,6 +311,30 @@ public void run () {
dispose();
}

void runPostedTasks () {
synchronized (postedTasks) {
Object[] items = postedTasks.items;
for (int i = 0, n = postedTasks.size; i < n; i++)
((Task)items[i]).run();
postedTasks.clear();
}
}

void addPostedTask (Task task) {
synchronized (postedTasks) {
if (postedTasks.isEmpty()) task.app.postRunnable(runPostedTasks);
postedTasks.add(task);
}
}

void removePostedTask (Task task) {
synchronized (postedTasks) {
Object[] items = postedTasks.items;
for (int i = postedTasks.size - 1; i >= 0; i--)
if (items[i] == task) postedTasks.removeIndex(i);
}
}

public void resume () {
synchronized (threadLock) {
long delayMillis = System.nanoTime() / 1000000 - pauseTimeMillis;
Expand All @@ -316,6 +354,9 @@ public void pause () {

public void dispose () { // OK to call multiple times.
synchronized (threadLock) {
synchronized (postedTasks) {
postedTasks.clear();
}
if (thread == this) thread = null;
instances.clear();
threadLock.notifyAll();
Expand Down
182 changes: 179 additions & 3 deletions tests/gdx-tests/src/com/badlogic/gdx/tests/TimerTest.java
View file
Expand Up @@ -22,16 +22,192 @@
import com.badlogic.gdx.utils.Timer.Task;

public class TimerTest extends GdxTest {
static final int expectedTests = 10;

@Override
public void create () {
Gdx.app.log("TimerTest", "Starting tests...");
testOnce();
testRepeat();
testCancelPosted();
testCancelPostedTwice();
testCancelPostedReschedule();
testStop();
testStopStart();
testDelay();
testClear();
}

float time;
int testCount;
Task repeatTask;

public void render () {
if (time < 2) {
time += Gdx.graphics.getDeltaTime();
if (time >= 2) {
if (testCount != expectedTests)
throw new RuntimeException("Some tests did not run: " + testCount + " != " + expectedTests);
Gdx.app.log("TimerTest", "SUCCESS! " + testCount + " tests passed.");
repeatTask.cancel();
}
}
}

void testOnce () {
Task task = Timer.schedule(new Task() {
@Override
public void run () {
Gdx.app.log("TimerTest", "testOnce");
testCount++;
}
}, 1);
assertScheduled(task);
}

void testRepeat () {
repeatTask = Timer.schedule(new Task() {
int count;

@Override
public void run () {
Gdx.app.log("TimerTest", "testRepeat");
if (++count <= 2) testCount++;
}
}, 1, 1);
assertScheduled(repeatTask);
}

void testCancelPosted () {
Task task = Timer.schedule(new Task() {
@Override
public void run () {
throw new RuntimeException("Cancelled task should not run.");
}
}, 0.01f);
assertScheduled(task);
sleep(200); // Sleep so the task is posted.
assertNotScheduled(task);
task.cancel(); // The posted task should not execute.
assertNotScheduled(task);
Gdx.app.log("TimerTest", "testCancelPosted");
testCount++;
}

void testCancelPostedTwice () {
Task task = new Task() {
@Override
public void run () {
throw new RuntimeException("Cancelled task should not run.");
}
};
Timer.schedule(task, 0.01f);
assertScheduled(task);
sleep(200); // Sleep so the task is posted.
assertNotScheduled(task);
Timer.schedule(task, 0.01f);
assertScheduled(task);
sleep(200); // Sleep so the task is posted.
assertNotScheduled(task);
task.cancel(); // The twice posted task should not execute.
assertNotScheduled(task);
Gdx.app.log("TimerTest", "testCancelPostedTwice");
testCount++;
}

void testCancelPostedReschedule () {
Task task = Timer.schedule(new Task() {
int count;

@Override
public void run () {
if (++count != 1) throw new RuntimeException("Rescheduled task should only run once.");
Gdx.app.log("TimerTest", "testCancelPostedReschedule");
testCount++;
}
}, 0.01f);
assertScheduled(task);
sleep(200); // Sleep so the task is posted.
assertNotScheduled(task);
task.cancel(); // The posted task should not execute.
assertNotScheduled(task);
Timer.schedule(task, 0.01f); // Schedule it again.
assertScheduled(task);
}

void testStop () {
Timer timer = new Timer();
Task task = timer.scheduleTask(new Task() {
@Override
public void run () {
Gdx.app.log("TimerTest", "ping");
throw new RuntimeException("Stopped timer should not run tasks.");
}
}, 1, 1);
}, 1);
assertScheduled(task);
timer.stop();
Gdx.app.log("TimerTest", "testStop");
testCount++;
}

void testStopStart () {
Timer timer = new Timer();
Task task = timer.scheduleTask(new Task() {
@Override
public void run () {
Gdx.app.log("TimerTest", "testStopStart");
testCount++;
}
}, 0.200f);
assertScheduled(task);
timer.stop();
sleep(300); // Sleep longer than task delay while stopped.
timer.start();
sleep(100);
assertScheduled(task); // Shouldn't have happened yet.
}

void testDelay () {
Timer timer = new Timer();
Task task = timer.scheduleTask(new Task() {
@Override
public void run () {
Gdx.app.log("TimerTest", "testDelay");
testCount++;
}
}, 0.200f);
assertScheduled(task);
timer.delay(200);
sleep(300); // Sleep longer than task delay.
assertScheduled(task); // Shouldn't have happened yet.
}

void testClear () {
Timer timer = new Timer();
Task task = timer.scheduleTask(new Task() {
@Override
public void run () {
throw new RuntimeException("Cleared timer should not run tasks.");
}
}, 0.200f);
assertScheduled(task);
timer.clear();
assertNotScheduled(task);
Gdx.app.log("TimerTest", "testClear");
testCount++;
}

void assertScheduled (Task task) {
if (!task.isScheduled()) throw new RuntimeException("Should be scheduled.");
}

void assertNotScheduled (Task task) {
if (task.isScheduled()) throw new RuntimeException("Should not be scheduled.");
}

Gdx.app.log("TimerTest", "is task scheduled: " + String.valueOf(task.isScheduled()));
void sleep (long millis) {
try {
Thread.sleep(millis);
} catch (InterruptedException ignored) {
}
}
}

0 comments on commit bba4270

Please sign in to comment.