Skip to content

Commit c915843

Browse files
mbonneaudavidwdan
authored andcommitted
Fix EventLoopScheduler issue (#186)
* Corrects EventLoopScheduler issue. Fixes #184 * Give test a little more breathing room.
1 parent cc84689 commit c915843

File tree

2 files changed

+50
-26
lines changed

2 files changed

+50
-26
lines changed

src/Scheduler/EventLoopScheduler.php

+21-14
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,27 @@ function ($ms, $callable) use ($timerCallableOrLoop) {
4242
});
4343
}
4444

45+
private function scheduleStartup()
46+
{
47+
if ($this->insideInvoke) {
48+
return;
49+
}
50+
$this->currentTimer->dispose();
51+
$this->nextTimer = $this->getClock();
52+
$this->currentTimer = call_user_func($this->delayCallback, 0, [$this, 'start']);
53+
}
54+
4555
public function scheduleAbsoluteWithState($state, int $dueTime, callable $action): DisposableInterface
4656
{
47-
$disp = parent::scheduleAbsoluteWithState($state, $dueTime, $action);
57+
$disp = new CompositeDisposable([
58+
parent::scheduleAbsoluteWithState($state, $dueTime, $action),
59+
new CallbackDisposable(function () use ($dueTime) {
60+
if ($dueTime > $this->nextTimer) {
61+
return;
62+
}
63+
$this->scheduleStartup();
64+
})
65+
]);
4866

4967
if ($this->insideInvoke) {
5068
return $disp;
@@ -54,20 +72,9 @@ public function scheduleAbsoluteWithState($state, int $dueTime, callable $action
5472
return $disp;
5573
}
5674

57-
$this->nextTimer = $this->getClock();
58-
59-
$this->currentTimer->dispose();
60-
$this->currentTimer = call_user_func($this->delayCallback, 0, [$this, 'start']);
75+
$this->scheduleStartup();
6176

62-
return new CompositeDisposable([
63-
$disp,
64-
new CallbackDisposable(function () {
65-
if (!$this->insideInvoke) {
66-
$this->currentTimer->dispose();
67-
$this->currentTimer = call_user_func($this->delayCallback, 0, [$this, 'start']);
68-
}
69-
})
70-
]);
77+
return $disp;
7178
}
7279

7380
public function start()

test/Rx/Scheduler/EventLoopSchedulerTest.php

+29-12
Original file line numberDiff line numberDiff line change
@@ -152,22 +152,20 @@ public function testScheduledItemsFromOutsideOfSchedulerDontCreateExtraTimers()
152152
});
153153
});
154154

155-
$scheduler->schedule(function () {}, 20);
155+
$scheduler->schedule(function () {}, 40);
156156

157-
$scheduler->schedule(function () {}, 15)->dispose();
158-
$scheduler->schedule(function () {}, 14)->dispose();
159-
$scheduler->schedule(function () {}, 13)->dispose();
160-
$scheduler->schedule(function () {}, 12)->dispose();
157+
$scheduler->schedule(function () {}, 35)->dispose();
158+
$scheduler->schedule(function () {}, 34)->dispose();
161159

162-
$scheduler->schedule(function () {}, 10);
160+
$scheduler->schedule(function () {}, 20);
163161

164162
$loop->run();
165163

166-
$this->assertEquals($timersCreated, 3);
167-
$this->assertEquals($timersExecuted, 3);
164+
$this->assertLessThanOrEqual(3, $timersCreated);
165+
$this->assertLessThanOrEqual(3, $timersExecuted);
168166
}
169167

170-
public function testMultipleSchedulersFromOutsideInSameTickDontCreateExtraTimers()
168+
public function testMultipleSchedulesFromOutsideInSameTickDontCreateExtraTimers()
171169
{
172170
$timersCreated = 0;
173171
$timersExecuted = 0;
@@ -190,13 +188,13 @@ public function testMultipleSchedulersFromOutsideInSameTickDontCreateExtraTimers
190188
$scheduler->schedule(function () {}, 25)->dispose();
191189
$scheduler->schedule(function () {}, 24)->dispose();
192190
$scheduler->schedule(function () {}, 23)->dispose();
193-
$scheduler->schedule(function () {}, 22)->dispose();
191+
$scheduler->schedule(function () {}, 25)->dispose();
194192
});
195193

196194
$loop->run();
197195

198-
$this->assertEquals($timersCreated, 3);
199-
$this->assertEquals($timersExecuted, 3);
196+
$this->assertEquals(3, $timersCreated);
197+
$this->assertEquals(3, $timersExecuted);
200198
}
201199

202200
public function testThatStuffScheduledWayInTheFutureDoesntKeepTheLoopRunningIfDisposed()
@@ -244,4 +242,23 @@ public function testThatDisposalOfSingleScheduledItemOutsideOfInvokeCancelsTimer
244242

245243
$this->assertLessThan(2, $endTime - $startTime);
246244
}
245+
246+
public function testScheduledItemPastNextScheduledItemKillsItOwnTimerIfItBecomesTheNextOneAndIsDisposed()
247+
{
248+
$loop = Factory::create();
249+
$scheduler = new EventLoopScheduler($loop);
250+
251+
$startTime = microtime(true);
252+
253+
$scheduler->schedule(function () {}, 30);
254+
$disp = $scheduler->schedule(function () {}, 3000);
255+
$loop->addTimer(0.050, function () use ($disp) {
256+
$disp->dispose();
257+
});
258+
259+
$loop->run();
260+
$endTime = microtime(true);
261+
262+
$this->assertLessThan(2, $endTime - $startTime);
263+
}
247264
}

0 commit comments

Comments
 (0)