From d32305d0d301e44228b8202a0cd2e3f4bf393cf6 Mon Sep 17 00:00:00 2001
From: FranzBusch <f.busch@apple.com>
Date: Mon, 27 Dec 2021 12:15:29 +0100
Subject: [PATCH] Guarantee ordering of `scheduleTask` with same deadline

### Motivation:

When scheduling tasks with the same deadline the current order of execution is undefined. Fixes https://github.com/apple/swift-nio/issues/1541

### Modifications:

In my previous PR https://github.com/apple/swift-nio/pull/2010, I added an internal id to every `ScheduledTask` to give them an identity for cancellation purposes. In this PR, I am now using the same id to also ensure that the execution of tasks with the same deadline is the same as the order they were scheduled in.

### Result:

`ScheduledTask`s are now executed in their scheduled order when they have the same deadline.
---
 .../MultiThreadedEventLoopGroup.swift         | 11 +++++++++-
 .../NIOPosixTests/EventLoopTest+XCTest.swift  |  1 +
 Tests/NIOPosixTests/EventLoopTest.swift       | 20 +++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift
index 7ec29197b3..5bd05833b9 100644
--- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift
+++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift
@@ -339,6 +339,11 @@ extension MultiThreadedEventLoopGroup: CustomStringConvertible {
 
 @usableFromInline
 internal struct ScheduledTask {
+    /// The id of the scheduled task.
+    ///
+    /// - Important: This id has two purposes. First, it is used to give this struct an identity so that we can implement ``Equatable``
+    ///     Second, it is used to give the tasks an order which we use to execute them.
+    ///     This means, the ids need to be unique for a given ``SelectableEventLoop`` and they need to be in ascending order.
     @usableFromInline
     let id: UInt64
     let task: () -> Void
@@ -376,7 +381,11 @@ extension ScheduledTask: CustomStringConvertible {
 extension ScheduledTask: Comparable {
     @usableFromInline
     static func < (lhs: ScheduledTask, rhs: ScheduledTask) -> Bool {
-        return lhs._readyTime < rhs._readyTime
+        if lhs._readyTime == rhs._readyTime {
+            return lhs.id < rhs.id
+        } else {
+            return lhs._readyTime < rhs._readyTime
+        }
     }
 
     @usableFromInline
diff --git a/Tests/NIOPosixTests/EventLoopTest+XCTest.swift b/Tests/NIOPosixTests/EventLoopTest+XCTest.swift
index b8cf6453e3..f855aba744 100644
--- a/Tests/NIOPosixTests/EventLoopTest+XCTest.swift
+++ b/Tests/NIOPosixTests/EventLoopTest+XCTest.swift
@@ -34,6 +34,7 @@ extension EventLoopTest {
                 ("testFlatScheduleCancelled", testFlatScheduleCancelled),
                 ("testScheduleRepeatedTask", testScheduleRepeatedTask),
                 ("testScheduledTaskThatIsImmediatelyCancelledNeverFires", testScheduledTaskThatIsImmediatelyCancelledNeverFires),
+                ("testScheduledTasksAreOrdered", testScheduledTasksAreOrdered),
                 ("testFlatScheduledTaskThatIsImmediatelyCancelledNeverFires", testFlatScheduledTaskThatIsImmediatelyCancelledNeverFires),
                 ("testRepeatedTaskThatIsImmediatelyCancelledNeverFires", testRepeatedTaskThatIsImmediatelyCancelledNeverFires),
                 ("testScheduleRepeatedTaskCancelFromDifferentThread", testScheduleRepeatedTaskCancelFromDifferentThread),
diff --git a/Tests/NIOPosixTests/EventLoopTest.swift b/Tests/NIOPosixTests/EventLoopTest.swift
index ec5080aa95..6bb5fdb86a 100644
--- a/Tests/NIOPosixTests/EventLoopTest.swift
+++ b/Tests/NIOPosixTests/EventLoopTest.swift
@@ -184,6 +184,26 @@ public final class EventLoopTest : XCTestCase {
         XCTAssertEqual(error as? EventLoopError, .cancelled)
     }
 
+    public func testScheduledTasksAreOrdered() throws {
+        let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
+        defer {
+            XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully())
+        }
+
+        let eventLoop = eventLoopGroup.next()
+        let now = NIODeadline.now()
+
+        var result = [Int]()
+        var lastScheduled: Scheduled<Void>?
+        for i in 0...100 {
+            lastScheduled = eventLoop.scheduleTask(deadline: now) {
+                result.append(i)
+            }
+        }
+        try lastScheduled?.futureResult.wait()
+        XCTAssertEqual(result, Array(0...100))
+    }
+
     public func testFlatScheduledTaskThatIsImmediatelyCancelledNeverFires() throws {
         let eventLoop = EmbeddedEventLoop()
         let scheduled = eventLoop.flatScheduleTask(in: .seconds(1)) {