Fix another NPE when canceling removed timer

armanbilge · Sep 22, 2023 · bc8fcf9 · bc8fcf9
1 parent 9d00ba0
commit bc8fcf9
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 2 deletions.
diff --git a/core/jvm/src/main/scala/cats/effect/unsafe/TimerHeap.scala b/core/jvm/src/main/scala/cats/effect/unsafe/TimerHeap.scala
@@ -133,6 +133,7 @@ private final class TimerHeap extends AtomicBoolean { needsPack =>
     val rootDeleted = root.isDeleted()
     val rootExpired = !rootDeleted && isExpired(root, now)
     if (rootDeleted || rootExpired) { // see if we can just replace the root
+      root.index = -1
       if (rootExpired) out(0) = root.getAndClear()
       val node = new Node(triggerTime, callback, 1)
       heap(1) = node
@@ -369,7 +370,6 @@ private final class TimerHeap extends AtomicBoolean { needsPack =>
         if (worker.ownsTimers(heap)) {
           // remove only if we are still in the heap
           if (index >= 0) heap.removeAt(index)
-          else ()
         } else // otherwise this heap will need packing
           needsPack.set(true)
       } else needsPack.set(true)

diff --git a/tests/shared/src/test/scala/cats/effect/IOSpec.scala b/tests/shared/src/test/scala/cats/effect/IOSpec.scala
@@ -1823,7 +1823,8 @@ class IOSpec extends BaseSpec with Discipline with IOPlatformSpecification {
         }
       }
 
-      "no-op when canceling an expired timer" in realWithRuntime { rt =>
+      "no-op when canceling an expired timer 1" in realWithRuntime { rt =>
+        // this one excercises a timer removed via `TimerHeap#pollFirstIfTriggered`
         IO(Promise[Unit]())
           .flatMap { p =>
             IO(rt.scheduler.sleep(1.nanosecond, () => p.success(()))).flatMap { cancel =>
@@ -1833,6 +1834,17 @@ class IOSpec extends BaseSpec with Discipline with IOPlatformSpecification {
           .as(ok)
       }
 
+      "no-op when canceling an expired timer 2" in realWithRuntime { rt =>
+        // this one excercises a timer removed via `TimerHeap#insert`
+        IO(Promise[Unit]())
+          .flatMap { p =>
+            IO(rt.scheduler.sleep(1.nanosecond, () => p.success(()))).flatMap { cancel =>
+              IO.sleep(1.nanosecond) *> IO.fromFuture(IO(p.future)) *> IO(cancel.run())
+            }
+          }
+          .as(ok)
+      }
+
       "no-op when canceling a timer twice" in realWithRuntime { rt =>
         IO(rt.scheduler.sleep(1.day, () => ()))
           .flatMap(cancel => IO(cancel.run()) *> IO(cancel.run()))