Fix potential race conditions when cancelling read/write idle timers (#720)

Author: gjcairo

Sources/AsyncHTTPClient/ConnectionPool/HTTP1/HTTP1ClientChannelHandler.swift

@@ -60,17 +60,13 @@ final class HTTP1ClientChannelHandler: ChannelDuplexHandler {
     private var idleReadTimeoutStateMachine: IdleReadStateMachine?
     private var idleReadTimeoutTimer: Scheduled<Void>?
 
-    /// Cancelling a task in NIO does *not* guarantee that the task will not execute under certain race conditions.
-    /// We therefore give each timer an ID and increase the ID every time we reset or cancel it.
-    /// We check in the task if the timer ID has changed in the meantime and do not execute any action if has changed.
-    private var currentIdleReadTimeoutTimerID: Int = 0
-
     private var idleWriteTimeoutStateMachine: IdleWriteStateMachine?
     private var idleWriteTimeoutTimer: Scheduled<Void>?
 
     /// Cancelling a task in NIO does *not* guarantee that the task will not execute under certain race conditions.
     /// We therefore give each timer an ID and increase the ID every time we reset or cancel it.
     /// We check in the task if the timer ID has changed in the meantime and do not execute any action if has changed.
+    private var currentIdleReadTimeoutTimerID: Int = 0
     private var currentIdleWriteTimeoutTimerID: Int = 0
 
     private let backgroundLogger: Logger

Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2ClientRequestHandler.swift

@@ -57,6 +57,12 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {
     private var idleWriteTimeoutStateMachine: IdleWriteStateMachine?
     private var idleWriteTimeoutTimer: Scheduled<Void>?
 
+    /// Cancelling a task in NIO does *not* guarantee that the task will not execute under certain race conditions.
+    /// We therefore give each timer an ID and increase the ID every time we reset or cancel it.
+    /// We check in the task if the timer ID has changed in the meantime and do not execute any action if has changed.
+    private var currentIdleReadTimeoutTimerID: Int = 0
+    private var currentIdleWriteTimeoutTimerID: Int = 0
+
     init(eventLoop: EventLoop) {
         self.eventLoop = eventLoop
     }
@@ -295,8 +301,9 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {
         case .startIdleReadTimeoutTimer(let timeAmount):
             assert(self.idleReadTimeoutTimer == nil, "Expected there is no timeout timer so far.")
 
+            let timerID = self.currentIdleReadTimeoutTimerID
             self.idleReadTimeoutTimer = self.eventLoop.scheduleTask(in: timeAmount) {
-                guard self.idleReadTimeoutTimer != nil else { return }
+                guard self.currentIdleReadTimeoutTimerID == timerID else { return }
                 let action = self.state.idleReadTimeoutTriggered()
                 self.run(action, context: context)
             }
@@ -306,14 +313,17 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {
                 oldTimer.cancel()
             }
 
+            self.currentIdleReadTimeoutTimerID &+= 1
+            let timerID = self.currentIdleReadTimeoutTimerID
             self.idleReadTimeoutTimer = self.eventLoop.scheduleTask(in: timeAmount) {
-                guard self.idleReadTimeoutTimer != nil else { return }
+                guard self.currentIdleReadTimeoutTimerID == timerID else { return }
                 let action = self.state.idleReadTimeoutTriggered()
                 self.run(action, context: context)
             }
         case .clearIdleReadTimeoutTimer:
             if let oldTimer = self.idleReadTimeoutTimer {
                 self.idleReadTimeoutTimer = nil
+                self.currentIdleReadTimeoutTimerID &+= 1
                 oldTimer.cancel()
             }
 
@@ -327,8 +337,9 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {
         case .startIdleWriteTimeoutTimer(let timeAmount):
             assert(self.idleWriteTimeoutTimer == nil, "Expected there is no timeout timer so far.")
 
+            let timerID = self.currentIdleWriteTimeoutTimerID
             self.idleWriteTimeoutTimer = self.eventLoop.scheduleTask(in: timeAmount) {
-                guard self.idleWriteTimeoutTimer != nil else { return }
+                guard self.currentIdleWriteTimeoutTimerID == timerID else { return }
                 let action = self.state.idleWriteTimeoutTriggered()
                 self.run(action, context: context)
             }
@@ -337,14 +348,17 @@ final class HTTP2ClientRequestHandler: ChannelDuplexHandler {
                 oldTimer.cancel()
             }
 
+            self.currentIdleWriteTimeoutTimerID &+= 1
+            let timerID = self.currentIdleWriteTimeoutTimerID
             self.idleWriteTimeoutTimer = self.eventLoop.scheduleTask(in: timeAmount) {
-                guard self.idleWriteTimeoutTimer != nil else { return }
+                guard self.currentIdleWriteTimeoutTimerID == timerID else { return }
                 let action = self.state.idleWriteTimeoutTriggered()
                 self.run(action, context: context)
             }
         case .clearIdleWriteTimeoutTimer:
             if let oldTimer = self.idleWriteTimeoutTimer {
                 self.idleWriteTimeoutTimer = nil
+                self.currentIdleWriteTimeoutTimerID &+= 1
                 oldTimer.cancel()
             }
         case .none: