Merge pull request #3388 from durban/timerLongOverflow

typelevel · Jan 29, 2023 · 20deae5 · 20deae5
2 parents be6325d + 3f2c730
commit 20deae5
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 14 deletions.
diff --git a/core/jvm/src/main/scala/cats/effect/unsafe/SleepCallback.scala b/core/jvm/src/main/scala/cats/effect/unsafe/SleepCallback.scala
@@ -75,5 +75,5 @@ private object SleepCallback {
   }
 
   implicit val sleepCallbackReverseOrdering: Ordering[SleepCallback] =
-    Ordering.fromLessThan(_.triggerTime > _.triggerTime)
+    Ordering.fromLessThan(_.triggerTime - _.triggerTime > 0)
 }
diff --git a/core/jvm/src/main/scala/cats/effect/unsafe/WorkerThread.scala b/core/jvm/src/main/scala/cats/effect/unsafe/WorkerThread.scala
@@ -401,7 +401,7 @@ private final class WorkerThread(
         while (cont) {
           val head = sleepers.head()
 
-          if (head.triggerTime <= now) {
+          if (head.triggerTime - now <= 0) {
             if (head.get()) {
               head.callback(RightUnit)
             }

diff --git a/tests/jvm/src/test/scala/cats/effect/unsafe/SleepCallbackSpec.scala b/tests/jvm/src/test/scala/cats/effect/unsafe/SleepCallbackSpec.scala
@@ -18,6 +18,7 @@ package cats.effect.unsafe
 
 import org.specs2.mutable.Specification
 
+import scala.annotation.tailrec
 import scala.concurrent.duration._
 
 class SleepCallbackSpec extends Specification {
@@ -33,6 +34,19 @@ class SleepCallbackSpec extends Specification {
       scb.triggerTime mustEqual expected
     }
 
+    def dequeueAll(sleepers: SleepersQueue): List[SleepCallback] = {
+      @tailrec
+      def loop(sleepers: SleepersQueue, acc: List[SleepCallback]): List[SleepCallback] =
+        if (sleepers.isEmpty) acc.reverse
+        else {
+          val head = sleepers.head()
+          sleepers.popHead()
+          loop(sleepers, head :: acc)
+        }
+
+      loop(sleepers, Nil)
+    }
+
     "be ordered according to the trigger time" in {
       val sleepers = SleepersQueue.empty
 
@@ -71,18 +85,6 @@ class SleepCallbackSpec extends Specification {
 
       sleepers.isEmpty must beFalse
 
-      def dequeueAll(sleepers: SleepersQueue): List[SleepCallback] = {
-        def loop(sleepers: SleepersQueue, acc: List[SleepCallback]): List[SleepCallback] =
-          if (sleepers.isEmpty) acc.reverse
-          else {
-            val head = sleepers.head()
-            sleepers.popHead()
-            loop(sleepers, head :: acc)
-          }
-
-        loop(sleepers, Nil)
-      }
-
       val ordering = dequeueAll(sleepers)
       val expectedOrdering = List(scb2, scb3, scb1)
 
@@ -91,6 +93,37 @@ class SleepCallbackSpec extends Specification {
       sleepers.isEmpty must beTrue
     }
 
+    "be ordered correctly even if Long overflows" in {
+      val sleepers = SleepersQueue.empty
+
+      val now1 = Long.MaxValue - 20L
+      val delay1 = 10.nanos
+      val expected1 = Long.MaxValue - 10L // no overflow yet
+
+      val now2 = Long.MaxValue - 5L
+      val delay2 = 10.nanos
+      val expected2 = Long.MinValue + 4L // overflow
+
+      val scb1 = SleepCallback.create(delay1, _ => (), now1, sleepers)
+      val scb2 = SleepCallback.create(delay2, _ => (), now2, sleepers)
+
+      scb1.triggerTime mustEqual expected1
+      scb2.triggerTime mustEqual expected2
+
+      (expected1 - expected2) must be lessThan 0
+      scb1 must be greaterThan scb2 // uses the reverse `Ordering` instance
+
+      sleepers += scb1
+      sleepers += scb2
+
+      val ordering = dequeueAll(sleepers)
+      val expectedOrdering = List(scb1, scb2)
+
+      ordering mustEqual expectedOrdering
+      ordering.map(_.triggerTime) mustEqual List(expected1, expected2)
+      sleepers.isEmpty must beTrue
+    }
+
     "summon the implicit ordering evidence" in {
       val _ = implicitly[Ordering[SleepCallback]]
       ok