Skip to content

Commit 00ffe2b

Browse files
committed
Handle negative delay times
Prior to this commit, `JavaFxScheduler` asserted that delay time values must non-negative and less than `Integer.MAX_VALUE` for compatibility with the JavaFX timer. This resulted in the possibility of false negative test failures for `JavaFxSchedulerTest#testPeriodicScheduling`, because when the system is under load (e.g. when CPUs are maxed out), negative delay time values get passed to `#schedule`. These negative values are perfectly valid, indicating no delay (and are documented as such--see `Worker#schedule` Javadoc). This commit removes the assertion of non-negative values, and instead simply adapts any negative value to zero, which keeps the JavaFX timing infrastructure happy. It also removes the upper boundary of `Integer.MAX_VALUE` for delay times, as it does not appear that JavaFX actually imposes any such limitation. JavaFxSchedulerTest#testInvalidDelayValues has been removed entirely, as there is essentially no such thing as an "invalid delay value" at this point. All remaining tests now typically succeed save for very infrequent failures of `JavaFxSchedulerTest#testNestedActions`. The reason for these non-deterministic failures is as yet unknown, but because they appear to be unrelated to the changes addressed in this commit, no attempt is made here to diagnose and fix them. Note that this branch includes @Lestard's pull request #4, which will indeed be necessary to fix "display not found" errors under Travis CI's headless environment. i.e. if that commit isn't included, this changes in this commit won't have a chance of succeeding.
1 parent ad77fcb commit 00ffe2b

File tree

2 files changed

+3
-27
lines changed

2 files changed

+3
-27
lines changed

src/main/java/rx/schedulers/JavaFxScheduler.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030

3131
import java.util.concurrent.TimeUnit;
3232

33+
import static java.lang.Math.max;
34+
3335
/**
3436
* Executes work on the JavaFx UI thread.
3537
* This scheduler should only be used with actions that execute quickly.
@@ -44,12 +46,6 @@ public static JavaFxScheduler getInstance() {
4446
return INSTANCE;
4547
}
4648

47-
private static void assertThatTheDelayIsValidForTheJavaFxTimer(long delay) {
48-
if (delay < 0 || delay > Integer.MAX_VALUE) {
49-
throw new IllegalArgumentException(String.format("The JavaFx timer only accepts non-negative delays up to %d milliseconds.", Integer.MAX_VALUE));
50-
}
51-
}
52-
5349
@Override
5450
public Worker createWorker() {
5551
return new InnerJavaFxScheduler();
@@ -71,10 +67,9 @@ public boolean isUnsubscribed() {
7167

7268
@Override
7369
public Subscription schedule(final Action0 action, long delayTime, TimeUnit unit) {
74-
long delay = unit.toMillis(delayTime);
75-
assertThatTheDelayIsValidForTheJavaFxTimer(delay);
7670
final BooleanSubscription s = BooleanSubscription.create();
7771

72+
final long delay = unit.toMillis(max(delayTime, 0));
7873
final Timeline timeline = new Timeline(new KeyFrame(Duration.millis(delay), new EventHandler<ActionEvent>() {
7974

8075
@Override

src/test/java/rx/schedulers/JavaFxSchedulerTest.java

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -73,25 +73,6 @@ public void run() {
7373
t.start();
7474
}
7575

76-
@Test
77-
public void testInvalidDelayValues() {
78-
final JavaFxScheduler scheduler = new JavaFxScheduler();
79-
final Worker inner = scheduler.createWorker();
80-
final Action0 action = mock(Action0.class);
81-
82-
exception.expect(IllegalArgumentException.class);
83-
inner.schedulePeriodically(action, -1L, 100L, TimeUnit.SECONDS);
84-
85-
exception.expect(IllegalArgumentException.class);
86-
inner.schedulePeriodically(action, 100L, -1L, TimeUnit.SECONDS);
87-
88-
exception.expect(IllegalArgumentException.class);
89-
inner.schedulePeriodically(action, 1L + Integer.MAX_VALUE, 100L, TimeUnit.MILLISECONDS);
90-
91-
exception.expect(IllegalArgumentException.class);
92-
inner.schedulePeriodically(action, 100L, 1L + Integer.MAX_VALUE / 1000, TimeUnit.SECONDS);
93-
}
94-
9576
@Test
9677
public void testPeriodicScheduling() throws Exception {
9778
final JavaFxScheduler scheduler = new JavaFxScheduler();

0 commit comments

Comments
 (0)