Skip to content

Commit 168b8b6

Browse files
committed
EngineTimer: fix misuse of epsilon when firing timer (#399)
Due to rounding errors with floating-point numbers, we use an EPSILON value for error margin, but it should be used only when comparing input parameters, not when firing a timer. Closes #399. Change-Id: I8f056848c61b48dd59d91f44c09c9caf17c7fa58
1 parent f9e18fa commit 168b8b6

File tree

2 files changed

+9
-4
lines changed

2 files changed

+9
-4
lines changed

lib/ClusterShell/Engine/Engine.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ def fire_expired(self):
311311
# timer is fired, but only once per call.
312312
expired_timercases = []
313313
now = time.time()
314-
while self.timers and (self.timers[0].fire_date - now) <= EPSILON:
314+
while self.timers and self.timers[0].fire_date <= now:
315315
expired_timercases.append(heapq.heappop(self.timers))
316316
self._dequeue_disarmed()
317317

tests/TaskTimerTest.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -428,17 +428,22 @@ def testForceDelayedRepeaterAutoClose(self):
428428
INTERVAL = 0.25
429429
task = task_self()
430430
teh = self.__class__.TForceDelayedRepeaterAutoCloseChecker()
431-
bootstrap = task.shell("sleep %f" % INTERVAL)
431+
bootstrap = task.shell("sleep %f" % (2 * INTERVAL))
432432
repeater1 = task.timer(INTERVAL, teh, INTERVAL, autoclose=True)
433433
repeater2 = task.timer(INTERVAL, teh, INTERVAL, autoclose=True)
434434
task.resume()
435435
# Expected behavior: both timers will fire after INTERVAL, the first
436436
# one will block thread for INTERVAL+0.1, the second one will also
437437
# block for INTERVAL+0.1 more time. Then at next runloop the engine
438438
# will see our shell command termination so will unregister associated
439-
# worker client. At this point, only autoclosing timers remain
439+
# worker client. At this point, only non-autoclosing timers remain
440440
# registered, so timer firing will be skipped and runloop will exit.
441-
self.assertEqual(teh.count, 2)
441+
#
442+
# Why two possible values below? This has changed due to #399:
443+
# count will be 1 if one timer is fired first (most likely without
444+
# epsilon batching effect fixed in #399)
445+
# count will be 2 if both timers are fired during the same runloop
446+
self.assertTrue(teh.count in (1, 2))
442447

443448
def testMultipleAddSameTimerPrivate(self):
444449
"""test multiple add() of same timer [private]"""

0 commit comments

Comments
 (0)