From 5f7065ffeb0dbc7286f3806b7a0b22d61cfa5ad1 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 28 Jun 2024 15:21:12 +0100 Subject: [PATCH 01/37] benchmarks: Add benchmark for MTELG.scheduleTask(in:_:) --- .../NIOPosixBenchmarks/Benchmarks.swift | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 45ddfc2a3f..42c5e3f662 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -14,6 +14,7 @@ import Benchmark import NIOPosix +import NIOCore private let eventLoop = MultiThreadedEventLoopGroup.singleton.next() @@ -64,4 +65,22 @@ let benchmarks = { ) } #endif + + Benchmark( + "MTELG.scheduleTask(in:_:)", + configuration: Benchmark.Configuration( + metrics: [.mallocCountTotal, .cpuTotal], + scalingFactor: .kilo + ) + ) { benchmark in + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { try! group.syncShutdownGracefully() } + let loop = group.next() + + benchmark.startMeasurement() + for _ in benchmark.scaledIterations { + loop.scheduleTask(in: .hours(1), {}) + } + benchmark.stopMeasurement() + } } From 33e82e0d5657f4bdddf899cce4cfe31d0db0eb21 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 28 Jun 2024 15:21:12 +0100 Subject: [PATCH 02/37] api: Add NIOTimer, NIOTimerHandler, and EventLoop.setTimer(for:_:) --- Sources/NIOCore/EventLoop.swift | 8 + Sources/NIOCore/NIOTimer.swift | 94 ++++++++++++ .../NIOEmbedded/AsyncTestingEventLoop.swift | 8 + Sources/NIOEmbedded/Embedded.swift | 8 + .../MultiThreadedEventLoopGroup.swift | 23 ++- Sources/NIOPosix/SelectableEventLoop.swift | 17 ++- Tests/NIOPosixTests/NIOTimerTests.swift | 139 ++++++++++++++++++ 7 files changed, 288 insertions(+), 9 deletions(-) create mode 100644 Sources/NIOCore/NIOTimer.swift create mode 100644 Tests/NIOPosixTests/NIOTimerTests.swift diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 50b90ed925..1ffc6bf0ab 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -356,6 +356,14 @@ public protocol EventLoop: EventLoopGroup { /// It is valid for an `EventLoop` not to implement any of the two `_promise` functions. If either of them are implemented, /// however, both of them should be implemented. func _promiseCompleted(futureIdentifier: _NIOEventLoopFutureIdentifier) -> (file: StaticString, line: UInt)? + + /// Set a timer that will call a handler at the given time. + @discardableResult + func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer + + /// Set a timer that will call a handler after a given amount of time. + @discardableResult + func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer } extension EventLoop { diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift new file mode 100644 index 0000000000..f4cef38113 --- /dev/null +++ b/Sources/NIOCore/NIOTimer.swift @@ -0,0 +1,94 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftNIO open source project +// +// Copyright (c) 2024 Apple Inc. and the SwiftNIO project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of SwiftNIO project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +/// A type that handles timer callbacks scheduled with ``EventLoop/setTimer(for:_:)-5e37g``. +/// +/// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. +public protocol NIOTimerHandler { + func timerFired(loop: any EventLoop) +} + +/// An opaque handle that can be used to cancel a timer. +/// +/// Users cannot create an instance of this type; it is returned by ``EventLoop/setTimer(for:_:)-5e37g``. +/// +/// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. +public struct NIOTimer { + @usableFromInline + enum Backing { + /// A task created using `EventLoop.scheduleTask(deadline:_:)`, used by default for `EventLoop` implementations. + case scheduledTask(Scheduled) + /// An identifier for a timer, used by `EventLoop` implementations that conform to `CustomTimerImplementation`. + case custom(eventLoop: any NIOCustomTimerImplementation, id: UInt64) + } + + @usableFromInline + var backing: Backing + + fileprivate init(_ scheduled: Scheduled) { + self.backing = .scheduledTask(scheduled) + } + + @inlinable + init(_ eventLoop: any NIOCustomTimerImplementation, id: UInt64) { + self.backing = .custom(eventLoop: eventLoop, id: id) + } + + /// Cancel the timer associated with this handle. + @inlinable + public func cancel() { + switch self.backing { + case .scheduledTask(let scheduled): + scheduled.cancel() + case .custom(let eventLoop, let id): + eventLoop.cancelTimer(id) + } + } +} + +/// Default implementation of `setSimpleTimer(for deadline:_:)`, backed by `EventLoop.scheduleTask`. +extension EventLoop { + @discardableResult + public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { + NIOTimer(self.scheduleTask(deadline: deadline) { handler.timerFired(loop: self) }) + } +} + +/// Default implementation of `setSimpleTimer(for duration:_:)`, delegating to `setSimpleTimer(for deadline:_:)`. +extension EventLoop { + @discardableResult + @inlinable + public func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer { + self.setTimer(for: .now() + duration, handler) + } +} + +/// Extension point for `EventLoop` implementations implement a custom timer. +public protocol NIOCustomTimerImplementation: EventLoop { + /// Set a timer that calls handler at the given time. + /// + /// Implementations must return an integer identifier that uniquely identifies the timer. + func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> UInt64 + + /// Cancel a timer with a given timer identifier. + func cancelTimer(_ id: UInt64) +} + +/// Default implementation of `setSimpleTimer(for deadline:_:)` for `EventLoop` types that opted in to `CustomeTimerImplementation`. +extension EventLoop where Self: NIOCustomTimerImplementation { + @inlinable + public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { + NIOTimer(self, id: self.setTimer(for: deadline, handler)) + } +} diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index bdc9423c85..806ec12958 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -171,6 +171,14 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { return self.scheduleTask(deadline: self.now + `in`, task) } + @discardableResult + public func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer { + /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot + /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for + /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self.now`. + self.setTimer(for: self.now + duration, handler) + } + /// On an `NIOAsyncTestingEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. Unlike with the other operations, this will /// immediately execute, to eliminate a common class of bugs. public func execute(_ task: @escaping () -> Void) { diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index 4a8ce0d218..0a6d39e71b 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -127,6 +127,14 @@ public final class EmbeddedEventLoop: EventLoop { return scheduleTask(deadline: self._now + `in`, task) } + @discardableResult + public func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer { + /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot + /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for + /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self._now`. + self.setTimer(for: self._now + duration, handler) + } + /// On an `EmbeddedEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. This means that /// `task` will be run the next time you call `EmbeddedEventLoop.run`. public func execute(_ task: @escaping () -> Void) { diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index b9e73b16b5..e6d1fd4974 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -460,21 +460,32 @@ internal struct ScheduledTask { /// 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 - private let failFn: (Error) -> Void + @usableFromInline internal let readyTime: NIODeadline + @usableFromInline + enum Kind { + case task(task: () -> Void, failFn: (Error) -> Void) + case timer(any NIOTimerHandler) + } + + @usableFromInline + let kind: Kind + + // TODO: Should these be .init() or should they be static functions? @usableFromInline init(id: UInt64, _ task: @escaping () -> Void, _ failFn: @escaping (Error) -> Void, _ time: NIODeadline) { self.id = id - self.task = task - self.failFn = failFn self.readyTime = time + self.kind = .task(task: task, failFn: failFn) } - func fail(_ error: Error) { - failFn(error) + @usableFromInline + init(id: UInt64, _ handler: any NIOTimerHandler, _ time: NIODeadline) { + self.id = id + self.readyTime = time + self.kind = .timer(handler) } } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index d262cf56ad..5aa7ba07f2 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -474,6 +474,8 @@ Further information: fatalError("Tried to run an UnownedJob without runtime support") } #endif + case .timer(let handler): + handler.timerFired(loop: self) } } } @@ -565,7 +567,10 @@ Further information: if moreScheduledTasksToConsider, tasksCopy.count < tasksCopyBatchSize, let task = scheduledTasks.peek() { if task.readyTime.readyIn(now) <= .nanoseconds(0) { scheduledTasks.pop() - tasksCopy.append(.function(task.task)) + switch task.kind { + case .task(let task, _): tasksCopy.append(.function(task)) + case .timer(let handler): tasksCopy.append(.timer(handler)) + } } else { nextScheduledTaskDeadline = task.readyTime moreScheduledTasksToConsider = false @@ -658,9 +663,14 @@ Further information: for task in immediateTasksCopy { self.run(task) } - // Fail all the scheduled tasks. + // Fail all the scheduled tasks (timers have no failFn and can just be dropped). for task in scheduledTasksCopy { - task.fail(EventLoopError.shutdown) + switch task.kind { + case .task(_, let failFn): + failFn(EventLoopError.shutdown) + case .timer: + break + } } iterations += 1 @@ -869,6 +879,7 @@ enum UnderlyingTask { #if compiler(>=5.9) case unownedJob(ErasedUnownedJob) #endif + case timer(any NIOTimerHandler) } @usableFromInline diff --git a/Tests/NIOPosixTests/NIOTimerTests.swift b/Tests/NIOPosixTests/NIOTimerTests.swift new file mode 100644 index 0000000000..68a59e445d --- /dev/null +++ b/Tests/NIOPosixTests/NIOTimerTests.swift @@ -0,0 +1,139 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftNIO open source project +// +// Copyright (c) 2024 Apple Inc. and the SwiftNIO project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of SwiftNIO project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import NIOCore +import NIOEmbedded +import NIOPosix +import XCTest + +final class EmbeddedTimerTests: XCTestCase { + + func testFired() async { + let loop = EmbeddedEventLoop() + + let handler = MockTimerHandler() + XCTAssertEqual(handler.firedCount, 0) + + _ = loop.setTimer(for: .milliseconds(42), handler) + XCTAssertEqual(handler.firedCount, 0) + + loop.advanceTime(by: .milliseconds(41)) + XCTAssertEqual(handler.firedCount, 0) + + loop.advanceTime(by: .milliseconds(1)) + XCTAssertEqual(handler.firedCount, 1) + + loop.advanceTime(by: .milliseconds(1)) + XCTAssertEqual(handler.firedCount, 1) + } + + func testCancelled() async { + let loop = EmbeddedEventLoop() + let handler = MockTimerHandler() + let handle = loop.setTimer(for: .milliseconds(42), handler) + + handle.cancel() + XCTAssertEqual(handler.firedCount, 0) + + loop.advanceTime(by: .milliseconds(43)) + XCTAssertEqual(handler.firedCount, 0) + } +} + +final class NIOAsyncTestingEventLoopTimerTests: XCTestCase { + + func testFired() async { + let loop = NIOAsyncTestingEventLoop() + + let handler = MockTimerHandler() + XCTAssertEqual(handler.firedCount, 0) + + _ = loop.setTimer(for: .milliseconds(42), handler) + XCTAssertEqual(handler.firedCount, 0) + + await loop.advanceTime(by: .milliseconds(41)) + XCTAssertEqual(handler.firedCount, 0) + + await loop.advanceTime(by: .milliseconds(1)) + XCTAssertEqual(handler.firedCount, 1) + + await loop.advanceTime(by: .milliseconds(1)) + XCTAssertEqual(handler.firedCount, 1) + } + + func testCancelled() async { + let loop = NIOAsyncTestingEventLoop() + let handler = MockTimerHandler() + let handle = loop.setTimer(for: .milliseconds(42), handler) + + handle.cancel() + XCTAssertEqual(handler.firedCount, 0) + + await loop.advanceTime(by: .milliseconds(43)) + XCTAssertEqual(handler.firedCount, 0) + } +} + +final class MTELGTimerTests: XCTestCase { + + func testFired() async throws { + let loop = MultiThreadedEventLoopGroup.singleton.next() + + let handler = MockTimerHandler() + XCTAssertEqual(handler.firedCount, 0) + + _ = loop.setTimer(for: .milliseconds(1), handler) + + await fulfillment(of: [handler.timerDidFire], timeout: 0.01) + XCTAssertEqual(handler.firedCount, 1) + } + + func testCancel() async throws { + let loop = MultiThreadedEventLoopGroup.singleton.next() + + let handler = MockTimerHandler() + handler.timerDidFire.isInverted = true + + let handle = loop.setTimer(for: .milliseconds(1), handler) + handle.cancel() + + await fulfillment(of: [handler.timerDidFire], timeout: 0.01) + XCTAssertEqual(handler.firedCount, 0) + } + + func testShutdown() async throws { + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + let loop = group.next() + defer { try! group.syncShutdownGracefully() } + + let handler = MockTimerHandler() + handler.timerDidFire.isInverted = true + + _ = loop.setTimer(for: .milliseconds(1), handler) + try await group.shutdownGracefully() + + await fulfillment(of: [handler.timerDidFire], timeout: 0.01) + XCTAssertEqual(handler.firedCount, 0) + } +} + +fileprivate final class MockTimerHandler: NIOTimerHandler { + var firedCount = 0 + var timerDidFire = XCTestExpectation(description: "Timer fired") + + func timerFired(loop: any EventLoop) { + self.firedCount += 1 + self.timerDidFire.fulfill() + } +} From b59e31d63c1c7b5741032d774191217ebdf40297 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 28 Jun 2024 15:21:12 +0100 Subject: [PATCH 03/37] benchmarks: Add benchmark for MTELG.setTimer(for:_:) --- .../NIOPosixBenchmarks/Benchmarks.swift | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 42c5e3f662..2f3beee0c6 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -13,8 +13,8 @@ //===----------------------------------------------------------------------===// import Benchmark -import NIOPosix import NIOCore +import NIOPosix private let eventLoop = MultiThreadedEventLoopGroup.singleton.next() @@ -83,4 +83,27 @@ let benchmarks = { } benchmark.stopMeasurement() } + + Benchmark( + "MTELG.setTimer(for:_:)", + configuration: Benchmark.Configuration( + metrics: [.mallocCountTotal, .cpuTotal], + scalingFactor: .kilo + ) + ) { benchmark in + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + defer { try! group.syncShutdownGracefully() } + let loop = group.next() + + final class Timer: NIOTimerHandler { + func timerFired(loop: any EventLoop) {} + } + let timer = Timer() + + benchmark.startMeasurement() + for _ in benchmark.scaledIterations { + let handle = loop.setTimer(for: .hours(1), timer) + } + benchmark.stopMeasurement() + } } From 17cf105d3d2c17f072d4398c12ce7c9970c75f06 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 28 Jun 2024 15:21:12 +0100 Subject: [PATCH 04/37] internal: Add NIOCustomTimerImplementation conformance to SelectableEventLoop --- Sources/NIOPosix/SelectableEventLoop.swift | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 5aa7ba07f2..98f73dfe20 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -894,3 +894,20 @@ internal func assertExpression(_ body: () -> Bool) { return body() }()) } + +extension SelectableEventLoop: NIOCustomTimerImplementation { + @inlinable + internal func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> UInt64 { + let taskId = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) + let task = ScheduledTask(id: taskId, handler, deadline) + try! self._schedule0(.scheduled(task)) + return taskId + } + + @inlinable + internal func cancelTimer(_ id: UInt64) { + self._tasksLock.withLock { + self._scheduledTasks.removeFirst(where: { $0.id == id }) + } + } +} From 0afb3a261b23e09457d204daab9e4a0422f41e4b Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 1 Jul 2024 10:12:35 +0100 Subject: [PATCH 05/37] test: Add Linux pre-5.9.2 backport for fulfillment(of:timeout:enforceOrder:) --- Tests/NIOPosixTests/NIOTimerTests.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Tests/NIOPosixTests/NIOTimerTests.swift b/Tests/NIOPosixTests/NIOTimerTests.swift index 68a59e445d..bd9f9fa9fd 100644 --- a/Tests/NIOPosixTests/NIOTimerTests.swift +++ b/Tests/NIOPosixTests/NIOTimerTests.swift @@ -137,3 +137,15 @@ fileprivate final class MockTimerHandler: NIOTimerHandler { self.timerDidFire.fulfill() } } + +#if !canImport(Darwin) && swift(<5.9.2) +extension XCTestCase { + func fulfillment( + of expectations: [XCTestExpectation], + timeout seconds: TimeInterval, + enforceOrder enforceOrderOfFulfillment: Bool = false + ) async { + wait(for: expectations, timeout: seconds) + } +} +#endif From df3cdb1c1f1f3ce8fb66fd81cdfa0cef3318c984 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 1 Jul 2024 17:29:27 +0100 Subject: [PATCH 06/37] test: Increase timer used in shutdown test --- Tests/NIOPosixTests/NIOTimerTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/NIOPosixTests/NIOTimerTests.swift b/Tests/NIOPosixTests/NIOTimerTests.swift index bd9f9fa9fd..2734e110eb 100644 --- a/Tests/NIOPosixTests/NIOTimerTests.swift +++ b/Tests/NIOPosixTests/NIOTimerTests.swift @@ -120,7 +120,7 @@ final class MTELGTimerTests: XCTestCase { let handler = MockTimerHandler() handler.timerDidFire.isInverted = true - _ = loop.setTimer(for: .milliseconds(1), handler) + _ = loop.setTimer(for: .milliseconds(100), handler) try await group.shutdownGracefully() await fulfillment(of: [handler.timerDidFire], timeout: 0.01) From b0d3f66cd8f337a4531b1e287958e76a216b76f1 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 5 Jul 2024 09:10:27 +0100 Subject: [PATCH 07/37] feedback: Make NIOTimer Sendable --- Sources/NIOCore/NIOTimer.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index f4cef38113..85b513f84d 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -24,9 +24,9 @@ public protocol NIOTimerHandler { /// Users cannot create an instance of this type; it is returned by ``EventLoop/setTimer(for:_:)-5e37g``. /// /// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. -public struct NIOTimer { +public struct NIOTimer: Sendable { @usableFromInline - enum Backing { + enum Backing: Sendable { /// A task created using `EventLoop.scheduleTask(deadline:_:)`, used by default for `EventLoop` implementations. case scheduledTask(Scheduled) /// An identifier for a timer, used by `EventLoop` implementations that conform to `CustomTimerImplementation`. @@ -79,9 +79,13 @@ public protocol NIOCustomTimerImplementation: EventLoop { /// Set a timer that calls handler at the given time. /// /// Implementations must return an integer identifier that uniquely identifies the timer. + /// + /// Implementations should tolerate this call being made off the event loop. func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> UInt64 /// Cancel a timer with a given timer identifier. + /// + /// Implementations should tolerate this call being made off the event loop. func cancelTimer(_ id: UInt64) } From 907c08702c06e56ae1959ad8fe51dc5a9d923837 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 09:13:23 +0100 Subject: [PATCH 08/37] feedback: Rename timerFired(loop:) to timerFired(eventLoop:) --- Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift | 2 +- Sources/NIOCore/NIOTimer.swift | 4 ++-- Sources/NIOPosix/SelectableEventLoop.swift | 2 +- Tests/NIOPosixTests/NIOTimerTests.swift | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 2f3beee0c6..af947bf967 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -96,7 +96,7 @@ let benchmarks = { let loop = group.next() final class Timer: NIOTimerHandler { - func timerFired(loop: any EventLoop) {} + func timerFired(eventLoop: any EventLoop) {} } let timer = Timer() diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index 85b513f84d..7c3b8dcd61 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -16,7 +16,7 @@ /// /// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. public protocol NIOTimerHandler { - func timerFired(loop: any EventLoop) + func timerFired(eventLoop: any EventLoop) } /// An opaque handle that can be used to cancel a timer. @@ -61,7 +61,7 @@ public struct NIOTimer: Sendable { extension EventLoop { @discardableResult public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { - NIOTimer(self.scheduleTask(deadline: deadline) { handler.timerFired(loop: self) }) + NIOTimer(self.scheduleTask(deadline: deadline) { handler.timerFired(eventLoop: self) }) } } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 98f73dfe20..0c9b087a4e 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -475,7 +475,7 @@ Further information: } #endif case .timer(let handler): - handler.timerFired(loop: self) + handler.timerFired(eventLoop: self) } } } diff --git a/Tests/NIOPosixTests/NIOTimerTests.swift b/Tests/NIOPosixTests/NIOTimerTests.swift index 2734e110eb..0cdd2f2cd7 100644 --- a/Tests/NIOPosixTests/NIOTimerTests.swift +++ b/Tests/NIOPosixTests/NIOTimerTests.swift @@ -132,7 +132,7 @@ fileprivate final class MockTimerHandler: NIOTimerHandler { var firedCount = 0 var timerDidFire = XCTestExpectation(description: "Timer fired") - func timerFired(loop: any EventLoop) { + func timerFired(eventLoop: any EventLoop) { self.firedCount += 1 self.timerDidFire.fulfill() } From b3e49036c6d5ef8f1f7dcbe0da49bce46c91e49f Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 09:52:51 +0100 Subject: [PATCH 09/37] feedback(attempted): Store a closure instead of UInt64 --- Sources/NIOCore/NIOTimer.swift | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index 7c3b8dcd61..63f2167549 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -30,20 +30,25 @@ public struct NIOTimer: Sendable { /// A task created using `EventLoop.scheduleTask(deadline:_:)`, used by default for `EventLoop` implementations. case scheduledTask(Scheduled) /// An identifier for a timer, used by `EventLoop` implementations that conform to `CustomTimerImplementation`. - case custom(eventLoop: any NIOCustomTimerImplementation, id: UInt64) + case custom(eventLoop: any NIOCustomTimerImplementation, onCancel: @Sendable () -> Void) } @usableFromInline var backing: Backing + @inlinable + init(_ backing: Backing) { + self.backing = backing + } + fileprivate init(_ scheduled: Scheduled) { self.backing = .scheduledTask(scheduled) } - @inlinable - init(_ eventLoop: any NIOCustomTimerImplementation, id: UInt64) { - self.backing = .custom(eventLoop: eventLoop, id: id) - } +// @inlinable +// init(_ eventLoop: any NIOCustomTimerImplementation, onCancel: @Sendable @escaping () -> Void) { +// self.backing = .custom(eventLoop: eventLoop, onCancel: onCancel) +// } /// Cancel the timer associated with this handle. @inlinable @@ -51,8 +56,8 @@ public struct NIOTimer: Sendable { switch self.backing { case .scheduledTask(let scheduled): scheduled.cancel() - case .custom(let eventLoop, let id): - eventLoop.cancelTimer(id) + case .custom(let eventLoop, let onCancel): + onCancel() } } } @@ -93,6 +98,7 @@ public protocol NIOCustomTimerImplementation: EventLoop { extension EventLoop where Self: NIOCustomTimerImplementation { @inlinable public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { - NIOTimer(self, id: self.setTimer(for: deadline, handler)) + let id = self.setTimer(for: deadline, handler) + return NIOTimer(.custom(eventLoop: self, onCancel: { self.cancelTimer(id) })) } } From fb5e835366d7bd679ec593779bf41c73a049cb46 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 09:53:01 +0100 Subject: [PATCH 10/37] feedback(reverted, allocates): Store a closure instead of UInt64 This reverts commit 0d9a6f9d6bb4add42e02127daddd01e00d0e6b6d. --- Sources/NIOCore/NIOTimer.swift | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index 63f2167549..7c3b8dcd61 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -30,25 +30,20 @@ public struct NIOTimer: Sendable { /// A task created using `EventLoop.scheduleTask(deadline:_:)`, used by default for `EventLoop` implementations. case scheduledTask(Scheduled) /// An identifier for a timer, used by `EventLoop` implementations that conform to `CustomTimerImplementation`. - case custom(eventLoop: any NIOCustomTimerImplementation, onCancel: @Sendable () -> Void) + case custom(eventLoop: any NIOCustomTimerImplementation, id: UInt64) } @usableFromInline var backing: Backing - @inlinable - init(_ backing: Backing) { - self.backing = backing - } - fileprivate init(_ scheduled: Scheduled) { self.backing = .scheduledTask(scheduled) } -// @inlinable -// init(_ eventLoop: any NIOCustomTimerImplementation, onCancel: @Sendable @escaping () -> Void) { -// self.backing = .custom(eventLoop: eventLoop, onCancel: onCancel) -// } + @inlinable + init(_ eventLoop: any NIOCustomTimerImplementation, id: UInt64) { + self.backing = .custom(eventLoop: eventLoop, id: id) + } /// Cancel the timer associated with this handle. @inlinable @@ -56,8 +51,8 @@ public struct NIOTimer: Sendable { switch self.backing { case .scheduledTask(let scheduled): scheduled.cancel() - case .custom(let eventLoop, let onCancel): - onCancel() + case .custom(let eventLoop, let id): + eventLoop.cancelTimer(id) } } } @@ -98,7 +93,6 @@ public protocol NIOCustomTimerImplementation: EventLoop { extension EventLoop where Self: NIOCustomTimerImplementation { @inlinable public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { - let id = self.setTimer(for: deadline, handler) - return NIOTimer(.custom(eventLoop: self, onCancel: { self.cancelTimer(id) })) + NIOTimer(self, id: self.setTimer(for: deadline, handler)) } } From 89c57ec15e25992577ee4ed999cc5b71954b5e3c Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 11:21:07 +0100 Subject: [PATCH 11/37] feedback(unsure): Replace extra protocol with runtime checks --- Sources/NIOCore/EventLoop.swift | 3 + Sources/NIOCore/NIOTimer.swift | 76 +++++++++++----------- Sources/NIOPosix/SelectableEventLoop.swift | 11 ++-- 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 1ffc6bf0ab..ec5232a3ea 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -364,6 +364,9 @@ public protocol EventLoop: EventLoopGroup { /// Set a timer that will call a handler after a given amount of time. @discardableResult func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer + + /// Cancel a timer. + func cancelTimer(_ timer: NIOTimer) } extension EventLoop { diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index 7c3b8dcd61..4bb5e76c7b 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -27,72 +27,70 @@ public protocol NIOTimerHandler { public struct NIOTimer: Sendable { @usableFromInline enum Backing: Sendable { - /// A task created using `EventLoop.scheduleTask(deadline:_:)`, used by default for `EventLoop` implementations. - case scheduledTask(Scheduled) - /// An identifier for a timer, used by `EventLoop` implementations that conform to `CustomTimerImplementation`. - case custom(eventLoop: any NIOCustomTimerImplementation, id: UInt64) + /// A task created using `EventLoop.scheduleTask(deadline:_:)` by the default event loop timer implementation. + case `default`(_ task: Scheduled) + /// A custom timer identifier, used by event loops that want to provide a custom timer implementation. + case custom(id: UInt64) } + @usableFromInline + var eventLoop: any EventLoop + @usableFromInline var backing: Backing - fileprivate init(_ scheduled: Scheduled) { - self.backing = .scheduledTask(scheduled) + /// This initializer is only for the default implementations and is fileprivate to avoid tempting EL implementations. + fileprivate init(_ eventLoop: any EventLoop, _ task: Scheduled) { + self.eventLoop = eventLoop + self.backing = .default(task) } + /// This initializer is for event loop implementations. End users should use ``EventLoop/setTimer(for:_:)-5e37g``. + /// + /// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. @inlinable - init(_ eventLoop: any NIOCustomTimerImplementation, id: UInt64) { - self.backing = .custom(eventLoop: eventLoop, id: id) + public init(_ eventLoop: any EventLoop, id: UInt64) { + self.eventLoop = eventLoop + self.backing = .custom(id: id) } /// Cancel the timer associated with this handle. @inlinable public func cancel() { - switch self.backing { - case .scheduledTask(let scheduled): - scheduled.cancel() - case .custom(let eventLoop, let id): - eventLoop.cancelTimer(id) - } + self.eventLoop.cancelTimer(self) + } + + /// The custom timer identifier, if this timer uses a custom timer implementation; nil otherwise. + @inlinable + public var customTimerID: UInt64? { + guard case .custom(let id) = backing else { return nil } + return id } } -/// Default implementation of `setSimpleTimer(for deadline:_:)`, backed by `EventLoop.scheduleTask`. extension EventLoop { + /// Default implementation of `setTimer(for deadline:_:)`, backed by `EventLoop.scheduleTask`. @discardableResult public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { - NIOTimer(self.scheduleTask(deadline: deadline) { handler.timerFired(eventLoop: self) }) + let task = self.scheduleTask(deadline: deadline) { handler.timerFired(eventLoop: self) } + return NIOTimer(self, task) } -} -/// Default implementation of `setSimpleTimer(for duration:_:)`, delegating to `setSimpleTimer(for deadline:_:)`. -extension EventLoop { + /// Default implementation of `setTimer(for duration:_:)`, delegating to `setTimer(for deadline:_:)`. @discardableResult @inlinable public func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer { self.setTimer(for: .now() + duration, handler) } -} -/// Extension point for `EventLoop` implementations implement a custom timer. -public protocol NIOCustomTimerImplementation: EventLoop { - /// Set a timer that calls handler at the given time. - /// - /// Implementations must return an integer identifier that uniquely identifies the timer. - /// - /// Implementations should tolerate this call being made off the event loop. - func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> UInt64 - - /// Cancel a timer with a given timer identifier. - /// - /// Implementations should tolerate this call being made off the event loop. - func cancelTimer(_ id: UInt64) -} - -/// Default implementation of `setSimpleTimer(for deadline:_:)` for `EventLoop` types that opted in to `CustomeTimerImplementation`. -extension EventLoop where Self: NIOCustomTimerImplementation { + /// Default implementation of `cancelTimer(_:)`, for cancelling timers set with the default timer implementation. @inlinable - public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { - NIOTimer(self, id: self.setTimer(for: deadline, handler)) + public func cancelTimer(_ timer: NIOTimer) { + switch timer.backing { + case .default(let task): + task.cancel() + case .custom: + preconditionFailure("EventLoop missing custom implementation of cancelTimer(_:)") + } } } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 0c9b087a4e..3730d90622 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -895,17 +895,20 @@ internal func assertExpression(_ body: () -> Bool) { }()) } -extension SelectableEventLoop: NIOCustomTimerImplementation { +extension SelectableEventLoop { @inlinable - internal func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> UInt64 { + func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { let taskId = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) let task = ScheduledTask(id: taskId, handler, deadline) try! self._schedule0(.scheduled(task)) - return taskId + return NIOTimer(self, id: taskId) } @inlinable - internal func cancelTimer(_ id: UInt64) { + func cancelTimer(_ timer: NIOTimer) { + guard let id = timer.customTimerID else { + preconditionFailure("No custom ID for timer") + } self._tasksLock.withLock { self._scheduledTasks.removeFirst(where: { $0.id == id }) } From 59a04de86f1d7ccdd41aa6dbc1e53590fe7ce287 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 13:28:29 +0100 Subject: [PATCH 12/37] feedback(unsure): Generic timerFired protocol witness --- Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift | 2 +- Sources/NIOCore/NIOTimer.swift | 2 +- Tests/NIOPosixTests/NIOTimerTests.swift | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index af947bf967..33641b7149 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -96,7 +96,7 @@ let benchmarks = { let loop = group.next() final class Timer: NIOTimerHandler { - func timerFired(eventLoop: any EventLoop) {} + func timerFired(eventLoop: some EventLoop) {} } let timer = Timer() diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index 4bb5e76c7b..f8d1080ad0 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -16,7 +16,7 @@ /// /// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. public protocol NIOTimerHandler { - func timerFired(eventLoop: any EventLoop) + func timerFired(eventLoop: some EventLoop) } /// An opaque handle that can be used to cancel a timer. diff --git a/Tests/NIOPosixTests/NIOTimerTests.swift b/Tests/NIOPosixTests/NIOTimerTests.swift index 0cdd2f2cd7..955d879050 100644 --- a/Tests/NIOPosixTests/NIOTimerTests.swift +++ b/Tests/NIOPosixTests/NIOTimerTests.swift @@ -132,7 +132,7 @@ fileprivate final class MockTimerHandler: NIOTimerHandler { var firedCount = 0 var timerDidFire = XCTestExpectation(description: "Timer fired") - func timerFired(eventLoop: any EventLoop) { + func timerFired(eventLoop: some EventLoop) { self.firedCount += 1 self.timerDidFire.fulfill() } From 06a4ce76a788bf5a7bfa7b2a5faf8fd2dcf2e8d4 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 13:34:13 +0100 Subject: [PATCH 13/37] feedback(unsure): Make setTimer generic over the handler --- Sources/NIOCore/EventLoop.swift | 4 ++-- Sources/NIOCore/NIOTimer.swift | 4 ++-- Sources/NIOEmbedded/AsyncTestingEventLoop.swift | 2 +- Sources/NIOEmbedded/Embedded.swift | 2 +- Sources/NIOPosix/MultiThreadedEventLoopGroup.swift | 2 +- Sources/NIOPosix/SelectableEventLoop.swift | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index ec5232a3ea..5c1ea50187 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -359,11 +359,11 @@ public protocol EventLoop: EventLoopGroup { /// Set a timer that will call a handler at the given time. @discardableResult - func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer + func setTimer(for deadline: NIODeadline, _ handler: some NIOTimerHandler) -> NIOTimer /// Set a timer that will call a handler after a given amount of time. @discardableResult - func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer + func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer /// Cancel a timer. func cancelTimer(_ timer: NIOTimer) diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index f8d1080ad0..d23a6be516 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -71,7 +71,7 @@ public struct NIOTimer: Sendable { extension EventLoop { /// Default implementation of `setTimer(for deadline:_:)`, backed by `EventLoop.scheduleTask`. @discardableResult - public func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { + public func setTimer(for deadline: NIODeadline, _ handler: some NIOTimerHandler) -> NIOTimer { let task = self.scheduleTask(deadline: deadline) { handler.timerFired(eventLoop: self) } return NIOTimer(self, task) } @@ -79,7 +79,7 @@ extension EventLoop { /// Default implementation of `setTimer(for duration:_:)`, delegating to `setTimer(for deadline:_:)`. @discardableResult @inlinable - public func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer { + public func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer { self.setTimer(for: .now() + duration, handler) } diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index 806ec12958..66e3043772 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -172,7 +172,7 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { } @discardableResult - public func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer { + public func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer { /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self.now`. diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index 0a6d39e71b..fe7789f5e1 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -128,7 +128,7 @@ public final class EmbeddedEventLoop: EventLoop { } @discardableResult - public func setTimer(for duration: TimeAmount, _ handler: any NIOTimerHandler) -> NIOTimer { + public func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer { /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self._now`. diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index e6d1fd4974..2cc65474a7 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -482,7 +482,7 @@ internal struct ScheduledTask { } @usableFromInline - init(id: UInt64, _ handler: any NIOTimerHandler, _ time: NIODeadline) { + init(id: UInt64, _ handler: some NIOTimerHandler, _ time: NIODeadline) { self.id = id self.readyTime = time self.kind = .timer(handler) diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 3730d90622..36bd03d8f0 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -897,7 +897,7 @@ internal func assertExpression(_ body: () -> Bool) { extension SelectableEventLoop { @inlinable - func setTimer(for deadline: NIODeadline, _ handler: any NIOTimerHandler) -> NIOTimer { + func setTimer(for deadline: NIODeadline, _ handler: some NIOTimerHandler) -> NIOTimer { let taskId = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) let task = ScheduledTask(id: taskId, handler, deadline) try! self._schedule0(.scheduled(task)) From 950db0c84ba06626eaa0f244ddcb847fa324904a Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 13:58:30 +0100 Subject: [PATCH 14/37] feedback: Use labelled parameter for handler --- .../Benchmarks/NIOPosixBenchmarks/Benchmarks.swift | 2 +- Sources/NIOCore/EventLoop.swift | 4 ++-- Sources/NIOCore/NIOTimer.swift | 6 +++--- Sources/NIOEmbedded/AsyncTestingEventLoop.swift | 4 ++-- Sources/NIOEmbedded/Embedded.swift | 4 ++-- Sources/NIOPosix/SelectableEventLoop.swift | 2 +- Tests/NIOPosixTests/NIOTimerTests.swift | 14 +++++++------- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 33641b7149..82f4519ac4 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -102,7 +102,7 @@ let benchmarks = { benchmark.startMeasurement() for _ in benchmark.scaledIterations { - let handle = loop.setTimer(for: .hours(1), timer) + let handle = loop.setTimer(for: .hours(1), handler: timer) } benchmark.stopMeasurement() } diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 5c1ea50187..3cf66ac29b 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -359,11 +359,11 @@ public protocol EventLoop: EventLoopGroup { /// Set a timer that will call a handler at the given time. @discardableResult - func setTimer(for deadline: NIODeadline, _ handler: some NIOTimerHandler) -> NIOTimer + func setTimer(for deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer /// Set a timer that will call a handler after a given amount of time. @discardableResult - func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer + func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer /// Cancel a timer. func cancelTimer(_ timer: NIOTimer) diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index d23a6be516..f826f62a22 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -71,7 +71,7 @@ public struct NIOTimer: Sendable { extension EventLoop { /// Default implementation of `setTimer(for deadline:_:)`, backed by `EventLoop.scheduleTask`. @discardableResult - public func setTimer(for deadline: NIODeadline, _ handler: some NIOTimerHandler) -> NIOTimer { + public func setTimer(for deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { let task = self.scheduleTask(deadline: deadline) { handler.timerFired(eventLoop: self) } return NIOTimer(self, task) } @@ -79,8 +79,8 @@ extension EventLoop { /// Default implementation of `setTimer(for duration:_:)`, delegating to `setTimer(for deadline:_:)`. @discardableResult @inlinable - public func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer { - self.setTimer(for: .now() + duration, handler) + public func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer { + self.setTimer(for: .now() + duration, handler: handler) } /// Default implementation of `cancelTimer(_:)`, for cancelling timers set with the default timer implementation. diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index 66e3043772..d9b7366f3b 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -172,11 +172,11 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { } @discardableResult - public func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer { + public func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer { /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self.now`. - self.setTimer(for: self.now + duration, handler) + self.setTimer(for: self.now + duration, handler: handler) } /// On an `NIOAsyncTestingEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. Unlike with the other operations, this will diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index fe7789f5e1..a0a6cc14b9 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -128,11 +128,11 @@ public final class EmbeddedEventLoop: EventLoop { } @discardableResult - public func setTimer(for duration: TimeAmount, _ handler: some NIOTimerHandler) -> NIOTimer { + public func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer { /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self._now`. - self.setTimer(for: self._now + duration, handler) + self.setTimer(for: self._now + duration, handler: handler) } /// On an `EmbeddedEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. This means that diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 36bd03d8f0..568d4f1382 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -897,7 +897,7 @@ internal func assertExpression(_ body: () -> Bool) { extension SelectableEventLoop { @inlinable - func setTimer(for deadline: NIODeadline, _ handler: some NIOTimerHandler) -> NIOTimer { + func setTimer(for deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { let taskId = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) let task = ScheduledTask(id: taskId, handler, deadline) try! self._schedule0(.scheduled(task)) diff --git a/Tests/NIOPosixTests/NIOTimerTests.swift b/Tests/NIOPosixTests/NIOTimerTests.swift index 955d879050..d39f219a3c 100644 --- a/Tests/NIOPosixTests/NIOTimerTests.swift +++ b/Tests/NIOPosixTests/NIOTimerTests.swift @@ -25,7 +25,7 @@ final class EmbeddedTimerTests: XCTestCase { let handler = MockTimerHandler() XCTAssertEqual(handler.firedCount, 0) - _ = loop.setTimer(for: .milliseconds(42), handler) + _ = loop.setTimer(for: .milliseconds(42), handler: handler) XCTAssertEqual(handler.firedCount, 0) loop.advanceTime(by: .milliseconds(41)) @@ -41,7 +41,7 @@ final class EmbeddedTimerTests: XCTestCase { func testCancelled() async { let loop = EmbeddedEventLoop() let handler = MockTimerHandler() - let handle = loop.setTimer(for: .milliseconds(42), handler) + let handle = loop.setTimer(for: .milliseconds(42), handler: handler) handle.cancel() XCTAssertEqual(handler.firedCount, 0) @@ -59,7 +59,7 @@ final class NIOAsyncTestingEventLoopTimerTests: XCTestCase { let handler = MockTimerHandler() XCTAssertEqual(handler.firedCount, 0) - _ = loop.setTimer(for: .milliseconds(42), handler) + _ = loop.setTimer(for: .milliseconds(42), handler: handler) XCTAssertEqual(handler.firedCount, 0) await loop.advanceTime(by: .milliseconds(41)) @@ -75,7 +75,7 @@ final class NIOAsyncTestingEventLoopTimerTests: XCTestCase { func testCancelled() async { let loop = NIOAsyncTestingEventLoop() let handler = MockTimerHandler() - let handle = loop.setTimer(for: .milliseconds(42), handler) + let handle = loop.setTimer(for: .milliseconds(42), handler: handler) handle.cancel() XCTAssertEqual(handler.firedCount, 0) @@ -93,7 +93,7 @@ final class MTELGTimerTests: XCTestCase { let handler = MockTimerHandler() XCTAssertEqual(handler.firedCount, 0) - _ = loop.setTimer(for: .milliseconds(1), handler) + _ = loop.setTimer(for: .milliseconds(1), handler: handler) await fulfillment(of: [handler.timerDidFire], timeout: 0.01) XCTAssertEqual(handler.firedCount, 1) @@ -105,7 +105,7 @@ final class MTELGTimerTests: XCTestCase { let handler = MockTimerHandler() handler.timerDidFire.isInverted = true - let handle = loop.setTimer(for: .milliseconds(1), handler) + let handle = loop.setTimer(for: .milliseconds(1), handler: handler) handle.cancel() await fulfillment(of: [handler.timerDidFire], timeout: 0.01) @@ -120,7 +120,7 @@ final class MTELGTimerTests: XCTestCase { let handler = MockTimerHandler() handler.timerDidFire.isInverted = true - _ = loop.setTimer(for: .milliseconds(100), handler) + _ = loop.setTimer(for: .milliseconds(100), handler: handler) try await group.shutdownGracefully() await fulfillment(of: [handler.timerDidFire], timeout: 0.01) From d6ae472c7db437642f14cd6b2d319b32d5229673 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 14:33:12 +0100 Subject: [PATCH 15/37] feedback: Use separate prepositions for TimeAmount and NIODeadline APIs --- Sources/NIOCore/EventLoop.swift | 2 +- Sources/NIOCore/NIOTimer.swift | 4 ++-- Sources/NIOEmbedded/AsyncTestingEventLoop.swift | 2 +- Sources/NIOEmbedded/Embedded.swift | 2 +- Sources/NIOPosix/SelectableEventLoop.swift | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 3cf66ac29b..b78c383f24 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -359,7 +359,7 @@ public protocol EventLoop: EventLoopGroup { /// Set a timer that will call a handler at the given time. @discardableResult - func setTimer(for deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer + func setTimer(at deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer /// Set a timer that will call a handler after a given amount of time. @discardableResult diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index f826f62a22..64bda65c82 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -71,7 +71,7 @@ public struct NIOTimer: Sendable { extension EventLoop { /// Default implementation of `setTimer(for deadline:_:)`, backed by `EventLoop.scheduleTask`. @discardableResult - public func setTimer(for deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { + public func setTimer(at deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { let task = self.scheduleTask(deadline: deadline) { handler.timerFired(eventLoop: self) } return NIOTimer(self, task) } @@ -80,7 +80,7 @@ extension EventLoop { @discardableResult @inlinable public func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer { - self.setTimer(for: .now() + duration, handler: handler) + self.setTimer(at: .now() + duration, handler: handler) } /// Default implementation of `cancelTimer(_:)`, for cancelling timers set with the default timer implementation. diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index d9b7366f3b..4df3bfc9c9 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -176,7 +176,7 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self.now`. - self.setTimer(for: self.now + duration, handler: handler) + self.setTimer(at: self.now + duration, handler: handler) } /// On an `NIOAsyncTestingEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. Unlike with the other operations, this will diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index a0a6cc14b9..bdd3bea7c9 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -132,7 +132,7 @@ public final class EmbeddedEventLoop: EventLoop { /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self._now`. - self.setTimer(for: self._now + duration, handler: handler) + self.setTimer(at: self._now + duration, handler: handler) } /// On an `EmbeddedEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. This means that diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 568d4f1382..9250ab5808 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -897,7 +897,7 @@ internal func assertExpression(_ body: () -> Bool) { extension SelectableEventLoop { @inlinable - func setTimer(for deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { + func setTimer(at deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { let taskId = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) let task = ScheduledTask(id: taskId, handler, deadline) try! self._schedule0(.scheduled(task)) From 4439ac692cca04d57bb65d566c7e8587a9fc89cf Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 14:35:39 +0100 Subject: [PATCH 16/37] Remove DocC disambiguation for now until API is decided --- Sources/NIOCore/NIOTimer.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index 64bda65c82..c7d7f1d6cd 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -12,18 +12,18 @@ // //===----------------------------------------------------------------------===// -/// A type that handles timer callbacks scheduled with ``EventLoop/setTimer(for:_:)-5e37g``. +/// A type that handles timer callbacks scheduled with `EventLoop.setTimer(for:_:)`. /// -/// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. +/// - Seealso: `EventLoop.setTimer(for:_:)`. public protocol NIOTimerHandler { func timerFired(eventLoop: some EventLoop) } /// An opaque handle that can be used to cancel a timer. /// -/// Users cannot create an instance of this type; it is returned by ``EventLoop/setTimer(for:_:)-5e37g``. +/// Users cannot create an instance of this type; it is returned by `EventLoop.setTimer(for:_:)`. /// -/// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. +/// - Seealso: `EventLoop.setTimer(for:_:)`. public struct NIOTimer: Sendable { @usableFromInline enum Backing: Sendable { @@ -45,9 +45,9 @@ public struct NIOTimer: Sendable { self.backing = .default(task) } - /// This initializer is for event loop implementations. End users should use ``EventLoop/setTimer(for:_:)-5e37g``. + /// This initializer is for event loop implementations. End users should use `EventLoop.setTimer(for:_:)`. /// - /// - Seealso: ``EventLoop/setTimer(for:_:)-5e37g``. + /// - Seealso: `EventLoop.setTimer(for:_:)`. @inlinable public init(_ eventLoop: any EventLoop, id: UInt64) { self.eventLoop = eventLoop From abd4b286abb277550728e5e72f526631ee36134a Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 14:36:12 +0100 Subject: [PATCH 17/37] feedback: Add documentation to NIOTimerHandler.timerFired protocol requirement --- Sources/NIOCore/NIOTimer.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift index c7d7f1d6cd..ca7eb7a4cf 100644 --- a/Sources/NIOCore/NIOTimer.swift +++ b/Sources/NIOCore/NIOTimer.swift @@ -16,6 +16,9 @@ /// /// - Seealso: `EventLoop.setTimer(for:_:)`. public protocol NIOTimerHandler { + /// Called when the timer expires, unless the timer is cancelled. + /// + /// - Parameter eventLoop: The event loop associated with the timer, on which this function will be called. func timerFired(eventLoop: some EventLoop) } From ceabc7b28c80578dbdfece27df7acba5188bbcbe Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 8 Jul 2024 15:44:05 +0100 Subject: [PATCH 18/37] feedback: Change API terms from setTimer to scheduleCallback --- .../NIOPosixBenchmarks/Benchmarks.swift | 8 +- Sources/NIOCore/EventLoop.swift | 24 +++- Sources/NIOCore/NIOScheduledCallback.swift | 107 ++++++++++++++++++ Sources/NIOCore/NIOTimer.swift | 99 ---------------- .../NIOEmbedded/AsyncTestingEventLoop.swift | 5 +- Sources/NIOEmbedded/Embedded.swift | 5 +- .../MultiThreadedEventLoopGroup.swift | 6 +- Sources/NIOPosix/SelectableEventLoop.swift | 22 ++-- ....swift => NIOScheduledCallbackTests.swift} | 24 ++-- 9 files changed, 161 insertions(+), 139 deletions(-) create mode 100644 Sources/NIOCore/NIOScheduledCallback.swift delete mode 100644 Sources/NIOCore/NIOTimer.swift rename Tests/NIOPosixTests/{NIOTimerTests.swift => NIOScheduledCallbackTests.swift} (81%) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 82f4519ac4..e3525aa54b 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -85,7 +85,7 @@ let benchmarks = { } Benchmark( - "MTELG.setTimer(for:_:)", + "MTELG.scheduleCallback(in:_:)", configuration: Benchmark.Configuration( metrics: [.mallocCountTotal, .cpuTotal], scalingFactor: .kilo @@ -95,14 +95,14 @@ let benchmarks = { defer { try! group.syncShutdownGracefully() } let loop = group.next() - final class Timer: NIOTimerHandler { - func timerFired(eventLoop: some EventLoop) {} + final class Timer: NIOScheduledCallbackHandler { + func onSchedule(eventLoop: some EventLoop) {} } let timer = Timer() benchmark.startMeasurement() for _ in benchmark.scaledIterations { - let handle = loop.setTimer(for: .hours(1), handler: timer) + let handle = loop.scheduleCallback(in: .hours(1), handler: timer) } benchmark.stopMeasurement() } diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index b78c383f24..0dd6f5f312 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -357,16 +357,28 @@ public protocol EventLoop: EventLoopGroup { /// however, both of them should be implemented. func _promiseCompleted(futureIdentifier: _NIOEventLoopFutureIdentifier) -> (file: StaticString, line: UInt)? - /// Set a timer that will call a handler at the given time. + /// Schedule a callback at a given time. + /// + /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ + /// `sheduleCallback(at deadline:handler:)` _and_ `cancelScheduledCallback(_:)`. Failure to do so will + /// result in a runtime error. @discardableResult - func setTimer(at deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer + func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback - /// Set a timer that will call a handler after a given amount of time. + /// Schedule a callback after given time. + /// + /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ + /// `sheduleCallback(at deadline:handler:)` _and_ `cancelScheduledCallback(_:)`. Failure to do so will + /// result in a runtime error. @discardableResult - func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer + func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback - /// Cancel a timer. - func cancelTimer(_ timer: NIOTimer) + /// Cancel a scheduled callback. + /// + /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ + /// `sheduleCallback(at deadline:handler:)` _and_ `cancelScheduledCallback(_:)`. Failure to do so will + /// result in a runtime error. + func cancelScheduledCallback(_ scheduledCallback: NIOScheduledCallback) } extension EventLoop { diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift new file mode 100644 index 0000000000..1984c76b3f --- /dev/null +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -0,0 +1,107 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the SwiftNIO open source project +// +// Copyright (c) 2024 Apple Inc. and the SwiftNIO project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of SwiftNIO project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +/// A type that handles callbacks scheduled with `EventLoop.scheduleCallback(at:handler:)`. +/// +/// - Seealso: `EventLoop.scheduleCallback(at:handler:)`. +public protocol NIOScheduledCallbackHandler { + /// This function is called at the scheduled time, unless the scheduled callback is cancelled. + /// + /// - Parameter eventLoop: The event loop on which the callback was scheduled. + func onSchedule(eventLoop: some EventLoop) +} + +/// An opaque handle that can be used to cancel a scheduled callback. +/// +/// Users should not create an instance of this type; it is returned by `EventLoop.scheduleCallback(at:handler:)`. +/// +/// - Seealso: `EventLoop.scheduleCallback(at:handler:)`. +public struct NIOScheduledCallback: Sendable { + @usableFromInline + enum Backing: Sendable { + /// A task created using `EventLoop.scheduleTask(deadline:_:)` by the default implementation. + case `default`(_ task: Scheduled) + /// A custom callback identifier, used by event loops that provide a custom implementation. + case custom(id: UInt64) + } + + @usableFromInline + var eventLoop: any EventLoop + + @usableFromInline + var backing: Backing + + /// This initializer is only for the default implementation and is fileprivate to avoid use in EL implementations. + fileprivate init(_ eventLoop: any EventLoop, _ task: Scheduled) { + self.eventLoop = eventLoop + self.backing = .default(task) + } + + /// Create a handle for the scheduled callback with an opaque identifier managed by the event loop. + /// + /// - NOTE: This initializer is for event loop implementors only, end users should use `EventLoop.scheduleCallback`. + /// + /// - Seealso: `EventLoop.scheduleCallback(at:handler:)`. + @inlinable + public init(_ eventLoop: any EventLoop, id: UInt64) { + self.eventLoop = eventLoop + self.backing = .custom(id: id) + } + + /// Cancel the scheduled callback associated with this handle. + @inlinable + public func cancel() { + self.eventLoop.cancelScheduledCallback(self) + } + + /// The callback identifier, if the event loop uses a custom scheduled callback implementation; nil otherwise. + /// + /// - NOTE: This property is for event loop implementors only. + @inlinable + public var customCallbackID: UInt64? { + guard case .custom(let id) = backing else { return nil } + return id + } +} + +extension EventLoop { + /// Default implementation of `scheduleCallback(at deadline:handler:)`: backed by `EventLoop.scheduleTask`. + @discardableResult + public func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + let task = self.scheduleTask(deadline: deadline) { handler.onSchedule(eventLoop: self) } + return NIOScheduledCallback(self, task) + } + + /// Default implementation of `scheduleCallback(in amount:handler:)`: calls `scheduleCallback(at deadline:handler:)`. + @discardableResult + @inlinable + public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + self.scheduleCallback(at: .now() + amount, handler: handler) + } + + /// Default implementation of `cancelScheduledCallback(_:)`: only cancels callbacks scheduled by the default implementation of `scheduleCallback`. + /// + /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ + /// `sheduleCallback(at deadline:handler:)` _and_ `cancelScheduledCallback(_:)`. Failure to do so will + /// result in a runtime error. + @inlinable + public func cancelScheduledCallback(_ scheduledCallback: NIOScheduledCallback) { + switch scheduledCallback.backing { + case .default(let task): + task.cancel() + case .custom: + preconditionFailure("EventLoop missing custom implementation of cancelTimer(_:)") + } + } +} diff --git a/Sources/NIOCore/NIOTimer.swift b/Sources/NIOCore/NIOTimer.swift deleted file mode 100644 index ca7eb7a4cf..0000000000 --- a/Sources/NIOCore/NIOTimer.swift +++ /dev/null @@ -1,99 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the SwiftNIO open source project -// -// Copyright (c) 2024 Apple Inc. and the SwiftNIO project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// See CONTRIBUTORS.txt for the list of SwiftNIO project authors -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -/// A type that handles timer callbacks scheduled with `EventLoop.setTimer(for:_:)`. -/// -/// - Seealso: `EventLoop.setTimer(for:_:)`. -public protocol NIOTimerHandler { - /// Called when the timer expires, unless the timer is cancelled. - /// - /// - Parameter eventLoop: The event loop associated with the timer, on which this function will be called. - func timerFired(eventLoop: some EventLoop) -} - -/// An opaque handle that can be used to cancel a timer. -/// -/// Users cannot create an instance of this type; it is returned by `EventLoop.setTimer(for:_:)`. -/// -/// - Seealso: `EventLoop.setTimer(for:_:)`. -public struct NIOTimer: Sendable { - @usableFromInline - enum Backing: Sendable { - /// A task created using `EventLoop.scheduleTask(deadline:_:)` by the default event loop timer implementation. - case `default`(_ task: Scheduled) - /// A custom timer identifier, used by event loops that want to provide a custom timer implementation. - case custom(id: UInt64) - } - - @usableFromInline - var eventLoop: any EventLoop - - @usableFromInline - var backing: Backing - - /// This initializer is only for the default implementations and is fileprivate to avoid tempting EL implementations. - fileprivate init(_ eventLoop: any EventLoop, _ task: Scheduled) { - self.eventLoop = eventLoop - self.backing = .default(task) - } - - /// This initializer is for event loop implementations. End users should use `EventLoop.setTimer(for:_:)`. - /// - /// - Seealso: `EventLoop.setTimer(for:_:)`. - @inlinable - public init(_ eventLoop: any EventLoop, id: UInt64) { - self.eventLoop = eventLoop - self.backing = .custom(id: id) - } - - /// Cancel the timer associated with this handle. - @inlinable - public func cancel() { - self.eventLoop.cancelTimer(self) - } - - /// The custom timer identifier, if this timer uses a custom timer implementation; nil otherwise. - @inlinable - public var customTimerID: UInt64? { - guard case .custom(let id) = backing else { return nil } - return id - } -} - -extension EventLoop { - /// Default implementation of `setTimer(for deadline:_:)`, backed by `EventLoop.scheduleTask`. - @discardableResult - public func setTimer(at deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { - let task = self.scheduleTask(deadline: deadline) { handler.timerFired(eventLoop: self) } - return NIOTimer(self, task) - } - - /// Default implementation of `setTimer(for duration:_:)`, delegating to `setTimer(for deadline:_:)`. - @discardableResult - @inlinable - public func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer { - self.setTimer(at: .now() + duration, handler: handler) - } - - /// Default implementation of `cancelTimer(_:)`, for cancelling timers set with the default timer implementation. - @inlinable - public func cancelTimer(_ timer: NIOTimer) { - switch timer.backing { - case .default(let task): - task.cancel() - case .custom: - preconditionFailure("EventLoop missing custom implementation of cancelTimer(_:)") - } - } -} diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index 4df3bfc9c9..c87b55784c 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -172,11 +172,12 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { } @discardableResult - public func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer { + public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + // TODO: docs /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self.now`. - self.setTimer(at: self.now + duration, handler: handler) + self.scheduleCallback(at: self.now + amount, handler: handler) } /// On an `NIOAsyncTestingEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. Unlike with the other operations, this will diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index bdd3bea7c9..8cb6b82683 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -128,11 +128,12 @@ public final class EmbeddedEventLoop: EventLoop { } @discardableResult - public func setTimer(for duration: TimeAmount, handler: some NIOTimerHandler) -> NIOTimer { + public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + // TODO: docs /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self._now`. - self.setTimer(at: self._now + duration, handler: handler) + self.scheduleCallback(at: self._now + amount, handler: handler) } /// On an `EmbeddedEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. This means that diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index 2cc65474a7..06e7e2c5b9 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -467,7 +467,7 @@ internal struct ScheduledTask { @usableFromInline enum Kind { case task(task: () -> Void, failFn: (Error) -> Void) - case timer(any NIOTimerHandler) + case callback(any NIOScheduledCallbackHandler) } @usableFromInline @@ -482,10 +482,10 @@ internal struct ScheduledTask { } @usableFromInline - init(id: UInt64, _ handler: some NIOTimerHandler, _ time: NIODeadline) { + init(id: UInt64, _ handler: any NIOScheduledCallbackHandler, _ time: NIODeadline) { self.id = id self.readyTime = time - self.kind = .timer(handler) + self.kind = .callback(handler) } } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 9250ab5808..ed228300bd 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -474,8 +474,8 @@ Further information: fatalError("Tried to run an UnownedJob without runtime support") } #endif - case .timer(let handler): - handler.timerFired(eventLoop: self) + case .callback(let handler): + handler.onSchedule(eventLoop: self) } } } @@ -569,7 +569,7 @@ Further information: scheduledTasks.pop() switch task.kind { case .task(let task, _): tasksCopy.append(.function(task)) - case .timer(let handler): tasksCopy.append(.timer(handler)) + case .callback(let handler): tasksCopy.append(.callback(handler)) } } else { nextScheduledTaskDeadline = task.readyTime @@ -663,12 +663,12 @@ Further information: for task in immediateTasksCopy { self.run(task) } - // Fail all the scheduled tasks (timers have no failFn and can just be dropped). + // Fail all the scheduled tasks (callbacks have no failFn and can just be dropped). for task in scheduledTasksCopy { switch task.kind { case .task(_, let failFn): failFn(EventLoopError.shutdown) - case .timer: + case .callback: break } } @@ -879,7 +879,7 @@ enum UnderlyingTask { #if compiler(>=5.9) case unownedJob(ErasedUnownedJob) #endif - case timer(any NIOTimerHandler) + case callback(any NIOScheduledCallbackHandler) } @usableFromInline @@ -897,17 +897,17 @@ internal func assertExpression(_ body: () -> Bool) { extension SelectableEventLoop { @inlinable - func setTimer(at deadline: NIODeadline, handler: some NIOTimerHandler) -> NIOTimer { + func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { let taskId = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) let task = ScheduledTask(id: taskId, handler, deadline) try! self._schedule0(.scheduled(task)) - return NIOTimer(self, id: taskId) + return NIOScheduledCallback(self, id: taskId) } @inlinable - func cancelTimer(_ timer: NIOTimer) { - guard let id = timer.customTimerID else { - preconditionFailure("No custom ID for timer") + func cancelScheduledCallback(_ scheduledCallback: NIOScheduledCallback) { + guard let id = scheduledCallback.customCallbackID else { + preconditionFailure("No custom ID for callback") } self._tasksLock.withLock { self._scheduledTasks.removeFirst(where: { $0.id == id }) diff --git a/Tests/NIOPosixTests/NIOTimerTests.swift b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift similarity index 81% rename from Tests/NIOPosixTests/NIOTimerTests.swift rename to Tests/NIOPosixTests/NIOScheduledCallbackTests.swift index d39f219a3c..3823b843ae 100644 --- a/Tests/NIOPosixTests/NIOTimerTests.swift +++ b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift @@ -17,7 +17,7 @@ import NIOEmbedded import NIOPosix import XCTest -final class EmbeddedTimerTests: XCTestCase { +final class EmbeddedScheduledCallbackTests: XCTestCase { func testFired() async { let loop = EmbeddedEventLoop() @@ -25,7 +25,7 @@ final class EmbeddedTimerTests: XCTestCase { let handler = MockTimerHandler() XCTAssertEqual(handler.firedCount, 0) - _ = loop.setTimer(for: .milliseconds(42), handler: handler) + _ = loop.scheduleCallback(in: .milliseconds(42), handler: handler) XCTAssertEqual(handler.firedCount, 0) loop.advanceTime(by: .milliseconds(41)) @@ -41,7 +41,7 @@ final class EmbeddedTimerTests: XCTestCase { func testCancelled() async { let loop = EmbeddedEventLoop() let handler = MockTimerHandler() - let handle = loop.setTimer(for: .milliseconds(42), handler: handler) + let handle = loop.scheduleCallback(in: .milliseconds(42), handler: handler) handle.cancel() XCTAssertEqual(handler.firedCount, 0) @@ -51,7 +51,7 @@ final class EmbeddedTimerTests: XCTestCase { } } -final class NIOAsyncTestingEventLoopTimerTests: XCTestCase { +final class NIOAsyncTestingEventLoopScheduledCallbackTests: XCTestCase { func testFired() async { let loop = NIOAsyncTestingEventLoop() @@ -59,7 +59,7 @@ final class NIOAsyncTestingEventLoopTimerTests: XCTestCase { let handler = MockTimerHandler() XCTAssertEqual(handler.firedCount, 0) - _ = loop.setTimer(for: .milliseconds(42), handler: handler) + _ = loop.scheduleCallback(in: .milliseconds(42), handler: handler) XCTAssertEqual(handler.firedCount, 0) await loop.advanceTime(by: .milliseconds(41)) @@ -75,7 +75,7 @@ final class NIOAsyncTestingEventLoopTimerTests: XCTestCase { func testCancelled() async { let loop = NIOAsyncTestingEventLoop() let handler = MockTimerHandler() - let handle = loop.setTimer(for: .milliseconds(42), handler: handler) + let handle = loop.scheduleCallback(in: .milliseconds(42), handler: handler) handle.cancel() XCTAssertEqual(handler.firedCount, 0) @@ -85,7 +85,7 @@ final class NIOAsyncTestingEventLoopTimerTests: XCTestCase { } } -final class MTELGTimerTests: XCTestCase { +final class MTELGScheduledCallbackTests: XCTestCase { func testFired() async throws { let loop = MultiThreadedEventLoopGroup.singleton.next() @@ -93,7 +93,7 @@ final class MTELGTimerTests: XCTestCase { let handler = MockTimerHandler() XCTAssertEqual(handler.firedCount, 0) - _ = loop.setTimer(for: .milliseconds(1), handler: handler) + _ = loop.scheduleCallback(in: .milliseconds(1), handler: handler) await fulfillment(of: [handler.timerDidFire], timeout: 0.01) XCTAssertEqual(handler.firedCount, 1) @@ -105,7 +105,7 @@ final class MTELGTimerTests: XCTestCase { let handler = MockTimerHandler() handler.timerDidFire.isInverted = true - let handle = loop.setTimer(for: .milliseconds(1), handler: handler) + let handle = loop.scheduleCallback(in: .milliseconds(1), handler: handler) handle.cancel() await fulfillment(of: [handler.timerDidFire], timeout: 0.01) @@ -120,7 +120,7 @@ final class MTELGTimerTests: XCTestCase { let handler = MockTimerHandler() handler.timerDidFire.isInverted = true - _ = loop.setTimer(for: .milliseconds(100), handler: handler) + _ = loop.scheduleCallback(in: .milliseconds(100), handler: handler) try await group.shutdownGracefully() await fulfillment(of: [handler.timerDidFire], timeout: 0.01) @@ -128,11 +128,11 @@ final class MTELGTimerTests: XCTestCase { } } -fileprivate final class MockTimerHandler: NIOTimerHandler { +fileprivate final class MockTimerHandler: NIOScheduledCallbackHandler { var firedCount = 0 var timerDidFire = XCTestExpectation(description: "Timer fired") - func timerFired(eventLoop: some EventLoop) { + func onSchedule(eventLoop: some EventLoop) { self.firedCount += 1 self.timerDidFire.fulfill() } From 80d91f68d08356ef9073cc5c9d23f6ad47aeb49d Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Wed, 10 Jul 2024 08:26:16 +0100 Subject: [PATCH 19/37] feedback: Local variable rename: taskId -> taskID --- Sources/NIOPosix/SelectableEventLoop.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index ed228300bd..7a6b90fc97 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -898,10 +898,10 @@ internal func assertExpression(_ body: () -> Bool) { extension SelectableEventLoop { @inlinable func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - let taskId = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) - let task = ScheduledTask(id: taskId, handler, deadline) + let taskID = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) + let task = ScheduledTask(id: taskID, handler, deadline) try! self._schedule0(.scheduled(task)) - return NIOScheduledCallback(self, id: taskId) + return NIOScheduledCallback(self, id: taskID) } @inlinable From 2ec55431f8bddf3cdf4e209079da75beb9d76ed6 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Wed, 10 Jul 2024 08:50:59 +0100 Subject: [PATCH 20/37] feedback: Reanme NIOScheduledCallbackHandler.onSchedule to handleScheduledCallback --- Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift | 2 +- Sources/NIOCore/NIOScheduledCallback.swift | 4 ++-- Sources/NIOPosix/SelectableEventLoop.swift | 2 +- Tests/NIOPosixTests/NIOScheduledCallbackTests.swift | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index e3525aa54b..4f8a6bec33 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -96,7 +96,7 @@ let benchmarks = { let loop = group.next() final class Timer: NIOScheduledCallbackHandler { - func onSchedule(eventLoop: some EventLoop) {} + func handleScheduledCallback(eventLoop: some EventLoop) {} } let timer = Timer() diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift index 1984c76b3f..c8097b4c4d 100644 --- a/Sources/NIOCore/NIOScheduledCallback.swift +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -19,7 +19,7 @@ public protocol NIOScheduledCallbackHandler { /// This function is called at the scheduled time, unless the scheduled callback is cancelled. /// /// - Parameter eventLoop: The event loop on which the callback was scheduled. - func onSchedule(eventLoop: some EventLoop) + func handleScheduledCallback(eventLoop: some EventLoop) } /// An opaque handle that can be used to cancel a scheduled callback. @@ -79,7 +79,7 @@ extension EventLoop { /// Default implementation of `scheduleCallback(at deadline:handler:)`: backed by `EventLoop.scheduleTask`. @discardableResult public func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - let task = self.scheduleTask(deadline: deadline) { handler.onSchedule(eventLoop: self) } + let task = self.scheduleTask(deadline: deadline) { handler.handleScheduledCallback(eventLoop: self) } return NIOScheduledCallback(self, task) } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 7a6b90fc97..dc79281afb 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -475,7 +475,7 @@ Further information: } #endif case .callback(let handler): - handler.onSchedule(eventLoop: self) + handler.handleScheduledCallback(eventLoop: self) } } } diff --git a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift index 3823b843ae..6969c96f5d 100644 --- a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift +++ b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift @@ -132,7 +132,7 @@ fileprivate final class MockTimerHandler: NIOScheduledCallbackHandler { var firedCount = 0 var timerDidFire = XCTestExpectation(description: "Timer fired") - func onSchedule(eventLoop: some EventLoop) { + func handleScheduledCallback(eventLoop: some EventLoop) { self.firedCount += 1 self.timerDidFire.fulfill() } From 5d6ac17a89528c41224872e3012a66661e4798ba Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Wed, 10 Jul 2024 09:08:06 +0100 Subject: [PATCH 21/37] feedback: Update protocol requirement documentation comments to use DocC refs --- Sources/NIOCore/EventLoop.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 0dd6f5f312..247f7b4d4d 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -360,7 +360,7 @@ public protocol EventLoop: EventLoopGroup { /// Schedule a callback at a given time. /// /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ - /// `sheduleCallback(at deadline:handler:)` _and_ `cancelScheduledCallback(_:)`. Failure to do so will + /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will /// result in a runtime error. @discardableResult func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback @@ -368,7 +368,7 @@ public protocol EventLoop: EventLoopGroup { /// Schedule a callback after given time. /// /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ - /// `sheduleCallback(at deadline:handler:)` _and_ `cancelScheduledCallback(_:)`. Failure to do so will + /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will /// result in a runtime error. @discardableResult func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback @@ -376,7 +376,7 @@ public protocol EventLoop: EventLoopGroup { /// Cancel a scheduled callback. /// /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ - /// `sheduleCallback(at deadline:handler:)` _and_ `cancelScheduledCallback(_:)`. Failure to do so will + /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will /// result in a runtime error. func cancelScheduledCallback(_ scheduledCallback: NIOScheduledCallback) } From bbf8f9f816ffded170e7b06d7bf2ebfcdd589de8 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 11 Jul 2024 13:52:28 +0100 Subject: [PATCH 22/37] feedback: Remove explicit benchmark.stopMeasurement calls --- Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 4f8a6bec33..367ea250e0 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -81,7 +81,6 @@ let benchmarks = { for _ in benchmark.scaledIterations { loop.scheduleTask(in: .hours(1), {}) } - benchmark.stopMeasurement() } Benchmark( @@ -104,6 +103,5 @@ let benchmarks = { for _ in benchmark.scaledIterations { let handle = loop.scheduleCallback(in: .hours(1), handler: timer) } - benchmark.stopMeasurement() } } From 21fd1cca526df5ae9f3fb33d8585ac6f5527a766 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 11 Jul 2024 13:54:20 +0100 Subject: [PATCH 23/37] feedback: Remove TODO following discussion about internal inits --- Sources/NIOPosix/MultiThreadedEventLoopGroup.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index 06e7e2c5b9..1f9c473d20 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -473,7 +473,6 @@ internal struct ScheduledTask { @usableFromInline let kind: Kind - // TODO: Should these be .init() or should they be static functions? @usableFromInline init(id: UInt64, _ task: @escaping () -> Void, _ failFn: @escaping (Error) -> Void, _ time: NIODeadline) { self.id = id From 9bf65f610fec8929dc73406f4c20a3b1facb1e27 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 11 Jul 2024 15:23:50 +0100 Subject: [PATCH 24/37] feedback: Make scheduleCallback throwing --- Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift | 2 +- Sources/NIOCore/EventLoop.swift | 4 ++-- Sources/NIOCore/NIOScheduledCallback.swift | 4 ++-- Sources/NIOPosix/SelectableEventLoop.swift | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 367ea250e0..7ef114c0db 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -101,7 +101,7 @@ let benchmarks = { benchmark.startMeasurement() for _ in benchmark.scaledIterations { - let handle = loop.scheduleCallback(in: .hours(1), handler: timer) + let handle = try! loop.scheduleCallback(in: .hours(1), handler: timer) } } } diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 247f7b4d4d..4bdcb41126 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -363,7 +363,7 @@ public protocol EventLoop: EventLoopGroup { /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will /// result in a runtime error. @discardableResult - func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback + func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback /// Schedule a callback after given time. /// @@ -371,7 +371,7 @@ public protocol EventLoop: EventLoopGroup { /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will /// result in a runtime error. @discardableResult - func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback + func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback /// Cancel a scheduled callback. /// diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift index c8097b4c4d..71e866b1bd 100644 --- a/Sources/NIOCore/NIOScheduledCallback.swift +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -86,8 +86,8 @@ extension EventLoop { /// Default implementation of `scheduleCallback(in amount:handler:)`: calls `scheduleCallback(at deadline:handler:)`. @discardableResult @inlinable - public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - self.scheduleCallback(at: .now() + amount, handler: handler) + public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { + try self.scheduleCallback(at: .now() + amount, handler: handler) } /// Default implementation of `cancelScheduledCallback(_:)`: only cancels callbacks scheduled by the default implementation of `scheduleCallback`. diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index dc79281afb..f848700790 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -897,10 +897,10 @@ internal func assertExpression(_ body: () -> Bool) { extension SelectableEventLoop { @inlinable - func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { let taskID = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) let task = ScheduledTask(id: taskID, handler, deadline) - try! self._schedule0(.scheduled(task)) + try self._schedule0(.scheduled(task)) return NIOScheduledCallback(self, id: taskID) } From dbba9e3dc98162f7493b3c94858f410be1505555 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 11 Jul 2024 15:27:02 +0100 Subject: [PATCH 25/37] feedback: Add a missing explicit self --- Sources/NIOCore/NIOScheduledCallback.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift index 71e866b1bd..9054f25644 100644 --- a/Sources/NIOCore/NIOScheduledCallback.swift +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -70,7 +70,7 @@ public struct NIOScheduledCallback: Sendable { /// - NOTE: This property is for event loop implementors only. @inlinable public var customCallbackID: UInt64? { - guard case .custom(let id) = backing else { return nil } + guard case .custom(let id) = self.backing else { return nil } return id } } From b16a575a46520b4f2879a369623d69d569df7f81 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 12 Jul 2024 14:31:00 +0100 Subject: [PATCH 26/37] Remove broken DocC disambiguatoin and fix test calls --- Sources/NIOCore/EventLoop.swift | 14 +++++--------- .../NIOPosixTests/NIOScheduledCallbackTests.swift | 8 ++++---- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 67813c6cd5..731ab1302f 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -359,25 +359,21 @@ public protocol EventLoop: EventLoopGroup { /// Schedule a callback at a given time. /// - /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ - /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will - /// result in a runtime error. + /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** also implement + /// `cancelScheduledCallback`. Failure to do so will result in a runtime error. @discardableResult func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback /// Schedule a callback after given time. /// - /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ - /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will - /// result in a runtime error. + /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** also implement + /// `cancelScheduledCallback`. Failure to do so will result in a runtime error. @discardableResult func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback /// Cancel a scheduled callback. /// - /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** implement _both_ - /// ``scheduleCallback(at:handler:)-5ryox`` and ``cancelScheduledCallback(_:)-1lfz0``. Failure to do so will - /// result in a runtime error. + /// - NOTE: Event loops only need to implemented this if they provide a custom scheduled callback implementation. func cancelScheduledCallback(_ scheduledCallback: NIOScheduledCallback) } diff --git a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift index 6969c96f5d..c8061634c7 100644 --- a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift +++ b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift @@ -93,7 +93,7 @@ final class MTELGScheduledCallbackTests: XCTestCase { let handler = MockTimerHandler() XCTAssertEqual(handler.firedCount, 0) - _ = loop.scheduleCallback(in: .milliseconds(1), handler: handler) + _ = try loop.scheduleCallback(in: .milliseconds(1), handler: handler) await fulfillment(of: [handler.timerDidFire], timeout: 0.01) XCTAssertEqual(handler.firedCount, 1) @@ -105,7 +105,7 @@ final class MTELGScheduledCallbackTests: XCTestCase { let handler = MockTimerHandler() handler.timerDidFire.isInverted = true - let handle = loop.scheduleCallback(in: .milliseconds(1), handler: handler) + let handle = try loop.scheduleCallback(in: .milliseconds(1), handler: handler) handle.cancel() await fulfillment(of: [handler.timerDidFire], timeout: 0.01) @@ -115,12 +115,12 @@ final class MTELGScheduledCallbackTests: XCTestCase { func testShutdown() async throws { let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) let loop = group.next() - defer { try! group.syncShutdownGracefully() } + defer { group.shutdownGracefully { error in XCTAssertNil(error) } } let handler = MockTimerHandler() handler.timerDidFire.isInverted = true - _ = loop.scheduleCallback(in: .milliseconds(100), handler: handler) + _ = try loop.scheduleCallback(in: .milliseconds(100), handler: handler) try await group.shutdownGracefully() await fulfillment(of: [handler.timerDidFire], timeout: 0.01) From 4e71ffb36ec388aa85b281f58142a22c3ca91294 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 15 Jul 2024 12:06:52 +0100 Subject: [PATCH 27/37] Update implementation comments in EmbeddedEventLoop and AsyncTestingEventLoop --- Sources/NIOEmbedded/AsyncTestingEventLoop.swift | 7 +++---- Sources/NIOEmbedded/Embedded.swift | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index c87b55784c..10c08f2a51 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -173,10 +173,9 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { @discardableResult public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - // TODO: docs - /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot - /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for - /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self.now`. + /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it has a manual clock so + /// we cannot rely on the default implemntation of `setTimer(for:handler:)`, which computes the deadline as an + /// offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self.now`. self.scheduleCallback(at: self.now + amount, handler: handler) } diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index 8cb6b82683..384c66dc72 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -129,10 +129,9 @@ public final class EmbeddedEventLoop: EventLoop { @discardableResult public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - // TODO: docs - /// Even though this type does not conform to `CustomTimerImplemenation`, it has a manual clock so we cannot - /// rely on the default implemntation of `setTimer(for duration:_:)`, which computes the deadline for - /// `setTimer(for deadline:_:)` naively using `NIODeadline.now`, but we must use `self._now`. + /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it has a manual clock so + /// we cannot rely on the default implemntation of `setTimer(for:handler:)`, which computes the deadline as an + /// offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self._now`. self.scheduleCallback(at: self._now + amount, handler: handler) } From b909365d969b34f6e33ba0f87d8d43963ea0c43a Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 15 Jul 2024 13:40:34 +0100 Subject: [PATCH 28/37] feedback: Fix preconditionFailure message --- Sources/NIOCore/NIOScheduledCallback.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift index 9054f25644..278c71bf91 100644 --- a/Sources/NIOCore/NIOScheduledCallback.swift +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -101,7 +101,7 @@ extension EventLoop { case .default(let task): task.cancel() case .custom: - preconditionFailure("EventLoop missing custom implementation of cancelTimer(_:)") + preconditionFailure("EventLoop missing custom implementation of cancelScheduledCallback(_:)") } } } From 555977211725958405474514ea6d9fad344b7e63 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 15 Jul 2024 13:42:21 +0100 Subject: [PATCH 29/37] feedback: Move SheduledTask.Kind and kind above other properties --- .../NIOPosix/MultiThreadedEventLoopGroup.swift | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index 55d930ec3f..cec4b20d36 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -453,6 +453,15 @@ struct ErasedUnownedJob { @usableFromInline internal struct ScheduledTask { + @usableFromInline + enum Kind { + case task(task: () -> Void, failFn: (Error) -> Void) + case callback(any NIOScheduledCallbackHandler) + } + + @usableFromInline + let kind: Kind + /// 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`` @@ -464,15 +473,6 @@ internal struct ScheduledTask { @usableFromInline internal let readyTime: NIODeadline - @usableFromInline - enum Kind { - case task(task: () -> Void, failFn: (Error) -> Void) - case callback(any NIOScheduledCallbackHandler) - } - - @usableFromInline - let kind: Kind - @usableFromInline init(id: UInt64, _ task: @escaping () -> Void, _ failFn: @escaping (Error) -> Void, _ time: NIODeadline) { self.id = id From 7f159be1195001b90e992be9055e8bfb421aecb0 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 15 Jul 2024 13:44:34 +0100 Subject: [PATCH 30/37] feedback: Measure .instructions instead of .cpuTotal --- Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index 7ef114c0db..a8ec84f4a2 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -69,7 +69,7 @@ let benchmarks = { Benchmark( "MTELG.scheduleTask(in:_:)", configuration: Benchmark.Configuration( - metrics: [.mallocCountTotal, .cpuTotal], + metrics: [.mallocCountTotal, .instructions], scalingFactor: .kilo ) ) { benchmark in @@ -86,7 +86,7 @@ let benchmarks = { Benchmark( "MTELG.scheduleCallback(in:_:)", configuration: Benchmark.Configuration( - metrics: [.mallocCountTotal, .cpuTotal], + metrics: [.mallocCountTotal, .instructions], scalingFactor: .kilo ) ) { benchmark in From 3ad56477e96c57058eaf8927f9045e96e3b3c4aa Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Mon, 15 Jul 2024 13:49:13 +0100 Subject: [PATCH 31/37] feedback: Remove vestigial references to setTimer in impl comments --- Sources/NIOEmbedded/AsyncTestingEventLoop.swift | 6 +++--- Sources/NIOEmbedded/Embedded.swift | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index 10c08f2a51..0048e3f589 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -173,9 +173,9 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { @discardableResult public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it has a manual clock so - /// we cannot rely on the default implemntation of `setTimer(for:handler:)`, which computes the deadline as an - /// offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self.now`. + /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it uses a manual clock so + /// it cannot rely on the default implementation of `scheduleCallback(in:handler:)`, which computes the deadline + /// as an offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self.now`. self.scheduleCallback(at: self.now + amount, handler: handler) } diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index 384c66dc72..96c89cb676 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -129,9 +129,9 @@ public final class EmbeddedEventLoop: EventLoop { @discardableResult public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it has a manual clock so - /// we cannot rely on the default implemntation of `setTimer(for:handler:)`, which computes the deadline as an - /// offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self._now`. + /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it uses a manual clock so + /// it cannot rely on the default implementation of `scheduleCallback(in:handler:)`, which computes the deadline + /// as an offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self._now`. self.scheduleCallback(at: self._now + amount, handler: handler) } From 6af561d95d53c4c3d06e5bc86dbb464e4d65a892 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Fri, 19 Jul 2024 10:30:59 +0100 Subject: [PATCH 32/37] Add cancellation callback and implicitly cancel on shutdown --- Sources/NIOCore/NIOScheduledCallback.swift | 53 ++- .../NIOEmbedded/AsyncTestingEventLoop.swift | 22 +- Sources/NIOPosix/SelectableEventLoop.swift | 16 +- Sources/_NIODataStructures/Heap.swift | 9 +- .../_NIODataStructures/PriorityQueue.swift | 3 +- .../NIOScheduledCallbackTests.swift | 344 +++++++++++++----- 6 files changed, 352 insertions(+), 95 deletions(-) diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift index 278c71bf91..d428de516b 100644 --- a/Sources/NIOCore/NIOScheduledCallback.swift +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -20,6 +20,19 @@ public protocol NIOScheduledCallbackHandler { /// /// - Parameter eventLoop: The event loop on which the callback was scheduled. func handleScheduledCallback(eventLoop: some EventLoop) + + /// This function is called if the scheduled callback is cancelled. + /// + /// The callback could be cancelled explictily, by the user calling ``NIOScheduledCallback/cancel()``, or + /// implicitly, if it was still pending when the event loop was shut down. + /// + /// - Parameter eventLoop: The event loop on which the callback was scheduled. + func onCancelScheduledCallback(eventLoop: some EventLoop) +} + +extension NIOScheduledCallbackHandler { + /// Default implementation of `onCancelScheduledCallback(eventLoop:)`: does nothing. + public func onCancelScheduledCallback(eventLoop: some EventLoop) {} } /// An opaque handle that can be used to cancel a scheduled callback. @@ -76,11 +89,47 @@ public struct NIOScheduledCallback: Sendable { } extension EventLoop { + /* package */ public func _scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + let task = self.scheduleTask(deadline: deadline) { handler.handleScheduledCallback(eventLoop: self) } + task.futureResult.whenFailure { error in + if case .cancelled = error as? EventLoopError { + handler.onCancelScheduledCallback(eventLoop: self) + } + } + return NIOScheduledCallback(self, task) + } + /// Default implementation of `scheduleCallback(at deadline:handler:)`: backed by `EventLoop.scheduleTask`. + /// + /// Ideally the scheduled callback handler should be called exactly once for each call to `scheduleCallback`: + /// either the callback handler, or the cancellation handler. + /// + /// In order to support cancellation in the default implementation, we hook the future of the scheduled task + /// backing the scheduled callback. This requires two calls to the event loop: `EventLoop.scheduleTask`, and + /// `EventLoopFuture.whenFailure`, both of which queue onto the event loop if called from off the event loop. + /// + /// This can present a challenge during event loop shutdown, where typically: + /// 1. Scheduled work that is past its deadline gets run. + /// 2. Scheduled future work gets cancelled. + /// 3. New work resulting from (1) and (2) gets handled differently depending on the EL: + /// a. `SelectableEventLoop` runs new work recursively and crashes if not quiesced in some number of ticks. + /// b. `EmbeddedEventLoop` and `NIOAsyncTestingEventLoop` will fail incoming work. + /// + /// `SelectableEventLoop` has a custom implementation for scheduled callbacks so warrants no further discussion. + /// + /// As a practical matter, the `EmbeddedEventLoop` is OK because it shares the thread of the caller, but for + /// other event loops (including any outside this repo), it's possible that the call to shutdown interleaves + /// with the call to create the scheduled task and the call to hook the task future. + /// + /// Because this API is synchronous and we cannot block the calling thread, users of event loops with this + /// default implementation will have cancellation callbacks delivered on a best-effort basis when the event loop + /// is shutdown and depends on how the event loop deals with newly scheduled tasks during shutdown. + /// + /// The implementation of this default conformance has been further factored out so we can use it in + /// `NIOAsyncTestingEventLoop`, where the use of `wait()` is _less bad_. @discardableResult public func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { - let task = self.scheduleTask(deadline: deadline) { handler.handleScheduledCallback(eventLoop: self) } - return NIOScheduledCallback(self, task) + self._scheduleCallback(at: deadline, handler: handler) } /// Default implementation of `scheduleCallback(in amount:handler:)`: calls `scheduleCallback(at deadline:handler:)`. diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index 0f45aa5ed9..54e2bd5550 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -191,12 +191,30 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { self.scheduleTask(deadline: self.now + `in`, task) } + public func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { + /// The default implementation of `scheduledCallback(at:handler)` makes two calls to the event loop because it + /// needs to hook the future of the backing scheduled task, which can lead to lost cancellation callbacks when + /// callbacks are scheduled close to event loop shutdown. + /// + /// We work around this here by using a blocking `wait()` if we are not on the event loop, which would be very + /// bad in areal event loop, but _less bad_ for testing. + /// + /// For more details, see the documentation attached to the default implementation. + if self.inEventLoop { + return self._scheduleCallback(at: deadline, handler: handler) + } else { + return try self.submit { + self._scheduleCallback(at: deadline, handler: handler) + }.wait() + } + } + @discardableResult - public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it uses a manual clock so /// it cannot rely on the default implementation of `scheduleCallback(in:handler:)`, which computes the deadline /// as an offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self.now`. - self.scheduleCallback(at: self.now + amount, handler: handler) + try self.scheduleCallback(at: self.now + amount, handler: handler) } /// On an `NIOAsyncTestingEventLoop`, `execute` will simply use `scheduleTask` with a deadline of _now_. Unlike with the other operations, this will diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 4fe476f869..8643e622f5 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -695,13 +695,14 @@ internal final class SelectableEventLoop: EventLoop { for task in immediateTasksCopy { self.run(task) } - // Fail all the scheduled tasks (callbacks have no failFn and can just be dropped). for task in scheduledTasksCopy { switch task.kind { + // Fail all the scheduled tasks. case .task(_, let failFn): failFn(EventLoopError._shutdown) - case .callback: - break + // Call the cancellation handler for all the scheduled callbacks. + case .callback(let handler): + handler.onCancelScheduledCallback(eventLoop: self) } } @@ -944,7 +945,14 @@ extension SelectableEventLoop { preconditionFailure("No custom ID for callback") } self._tasksLock.withLock { - self._scheduledTasks.removeFirst(where: { $0.id == id }) + guard let scheduledTask = self._scheduledTasks.removeFirst(where: { $0.id == id }) else { + // Must have been cancelled already. + return + } + guard case .callback(let handler) = scheduledTask.kind else { + preconditionFailure("Incorrect task kind for callback") + } + handler.onCancelScheduledCallback(eventLoop: self) } } } diff --git a/Sources/_NIODataStructures/Heap.swift b/Sources/_NIODataStructures/Heap.swift index 29a95a18c2..30473a6acc 100644 --- a/Sources/_NIODataStructures/Heap.swift +++ b/Sources/_NIODataStructures/Heap.swift @@ -124,17 +124,18 @@ internal struct Heap { } } + @discardableResult @inlinable - internal mutating func removeFirst(where shouldBeRemoved: (Element) throws -> Bool) rethrows { + internal mutating func removeFirst(where shouldBeRemoved: (Element) throws -> Bool) rethrows -> Element? { guard self.storage.count > 0 else { - return + return nil } guard let index = try self.storage.firstIndex(where: shouldBeRemoved) else { - return + return nil } - self._remove(index: index) + return self._remove(index: index) } @discardableResult diff --git a/Sources/_NIODataStructures/PriorityQueue.swift b/Sources/_NIODataStructures/PriorityQueue.swift index 765826166e..21d0aa38bd 100644 --- a/Sources/_NIODataStructures/PriorityQueue.swift +++ b/Sources/_NIODataStructures/PriorityQueue.swift @@ -26,8 +26,9 @@ public struct PriorityQueue { self._heap.remove(value: key) } + @discardableResult @inlinable - public mutating func removeFirst(where shouldBeRemoved: (Element) throws -> Bool) rethrows { + public mutating func removeFirst(where shouldBeRemoved: (Element) throws -> Bool) rethrows -> Element? { try self._heap.removeFirst(where: shouldBeRemoved) } diff --git a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift index c8061634c7..56a629ee3e 100644 --- a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift +++ b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift @@ -17,135 +17,315 @@ import NIOEmbedded import NIOPosix import XCTest -final class EmbeddedScheduledCallbackTests: XCTestCase { +protocol ScheduledCallbackTestRequirements { + // Some ELs are backed by an ELG. + var loop: (any EventLoop) { get } - func testFired() async { - let loop = EmbeddedEventLoop() + // Some ELs have a manual time ratchet. + func advanceTime(by amount: TimeAmount) async throws - let handler = MockTimerHandler() - XCTAssertEqual(handler.firedCount, 0) + // ELG-backed ELs need to be shutdown via the ELG. + func shutdownEventLoop() async throws - _ = loop.scheduleCallback(in: .milliseconds(42), handler: handler) - XCTAssertEqual(handler.firedCount, 0) + // This is here for NIOAsyncTestingEventLoop only. + func maybeInContext(_ body: @escaping @Sendable () throws -> R) async throws -> R +} + +final class MTELGScheduledCallbackTests: _BaseScheduledCallbackTests { + struct Requirements: ScheduledCallbackTestRequirements { + let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) + var loop: (any EventLoop) { self.group.next() } + + func advanceTime(by amount: TimeAmount) async throws { + try await Task.sleep(nanoseconds: UInt64(amount.nanoseconds)) + } - loop.advanceTime(by: .milliseconds(41)) - XCTAssertEqual(handler.firedCount, 0) + func shutdownEventLoop() async throws { + try await self.group.shutdownGracefully() + } - loop.advanceTime(by: .milliseconds(1)) - XCTAssertEqual(handler.firedCount, 1) + func maybeInContext(_ body: @escaping @Sendable () throws -> R) async throws -> R { + try body() + } + } - loop.advanceTime(by: .milliseconds(1)) - XCTAssertEqual(handler.firedCount, 1) + override func setUp() async throws { + self.requirements = Requirements() } +} - func testCancelled() async { - let loop = EmbeddedEventLoop() - let handler = MockTimerHandler() - let handle = loop.scheduleCallback(in: .milliseconds(42), handler: handler) +final class EmbeddedScheduledCallbackTests: _BaseScheduledCallbackTests { + struct Requirements: ScheduledCallbackTestRequirements { + let _loop = EmbeddedEventLoop() + var loop: (any EventLoop) { self._loop } - handle.cancel() - XCTAssertEqual(handler.firedCount, 0) + func advanceTime(by amount: TimeAmount) async throws { + self._loop.advanceTime(by: amount) + } + + func shutdownEventLoop() async throws { + try await self._loop.shutdownGracefully() + } - loop.advanceTime(by: .milliseconds(43)) - XCTAssertEqual(handler.firedCount, 0) + func maybeInContext(_ body: @escaping @Sendable () throws -> R) async throws -> R { + try body() + } + } + + override func setUp() async throws { + self.requirements = Requirements() } } -final class NIOAsyncTestingEventLoopScheduledCallbackTests: XCTestCase { +final class NIOAsyncTestingEventLoopScheduledCallbackTests: _BaseScheduledCallbackTests { + struct Requirements: ScheduledCallbackTestRequirements { + let _loop = NIOAsyncTestingEventLoop() + var loop: (any EventLoop) { self._loop } - func testFired() async { - let loop = NIOAsyncTestingEventLoop() + func advanceTime(by amount: TimeAmount) async throws { + await self._loop.advanceTime(by: amount) + } - let handler = MockTimerHandler() - XCTAssertEqual(handler.firedCount, 0) + func shutdownEventLoop() async throws { + await self._loop.shutdownGracefully() + } - _ = loop.scheduleCallback(in: .milliseconds(42), handler: handler) - XCTAssertEqual(handler.firedCount, 0) + func maybeInContext(_ body: @escaping @Sendable () throws -> R) async throws -> R { + try await self._loop.executeInContext(body) + } + } - await loop.advanceTime(by: .milliseconds(41)) - XCTAssertEqual(handler.firedCount, 0) + override func setUp() async throws { + self.requirements = Requirements() + } +} - await loop.advanceTime(by: .milliseconds(1)) - XCTAssertEqual(handler.firedCount, 1) +class _BaseScheduledCallbackTests: XCTestCase { + // EL-specific test requirements. + var requirements: (any ScheduledCallbackTestRequirements)! = nil - await loop.advanceTime(by: .milliseconds(1)) - XCTAssertEqual(handler.firedCount, 1) + override func setUp() async throws { + try XCTSkipIf(type(of: self) == _BaseScheduledCallbackTests.self, "This is the abstract base class") + preconditionFailure("Subclass should implement setup and initialise EL-specific `self.requirements`") } +} - func testCancelled() async { - let loop = NIOAsyncTestingEventLoop() - let handler = MockTimerHandler() - let handle = loop.scheduleCallback(in: .milliseconds(42), handler: handler) +// Provide pass through computed properties to the EL-specific test requirements. +extension _BaseScheduledCallbackTests { + var loop: (any EventLoop) { self.requirements.loop } - handle.cancel() - XCTAssertEqual(handler.firedCount, 0) + func advanceTime(by amount: TimeAmount) async throws { + try await self.requirements.advanceTime(by: amount) + } - await loop.advanceTime(by: .milliseconds(43)) - XCTAssertEqual(handler.firedCount, 0) + func shutdownEventLoop() async throws { + try await self.requirements.shutdownEventLoop() + } + + func maybeInContext(_ body: @escaping @Sendable () throws -> R) async throws -> R { + try await self.requirements.maybeInContext(body) } } -final class MTELGScheduledCallbackTests: XCTestCase { +// The tests, abstracted over any of the event loops. +extension _BaseScheduledCallbackTests { - func testFired() async throws { - let loop = MultiThreadedEventLoopGroup.singleton.next() + func testScheduledCallbackNotExecutedBeforeDeadline() async throws { + let handler = MockScheduledCallbackHandler() - let handler = MockTimerHandler() - XCTAssertEqual(handler.firedCount, 0) + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 0) } + + try await self.advanceTime(by: .microseconds(1)) + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 0) } + } - _ = try loop.scheduleCallback(in: .milliseconds(1), handler: handler) + func testSheduledCallbackExecutedAtDeadline() async throws { + let handler = MockScheduledCallbackHandler() - await fulfillment(of: [handler.timerDidFire], timeout: 0.01) - XCTAssertEqual(handler.firedCount, 1) + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + try await self.advanceTime(by: .milliseconds(1)) + try await handler.waitForCallback(timeout: .seconds(1)) + try await self.maybeInContext { handler.assert(callbackCount: 1, cancelCount: 0) } } - func testCancel() async throws { - let loop = MultiThreadedEventLoopGroup.singleton.next() + func testMultipleSheduledCallbacksUsingSameHandler() async throws { + let handler = MockScheduledCallbackHandler() - let handler = MockTimerHandler() - handler.timerDidFire.isInverted = true + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) - let handle = try loop.scheduleCallback(in: .milliseconds(1), handler: handler) - handle.cancel() + try await self.advanceTime(by: .milliseconds(1)) + try await handler.waitForCallback(timeout: .seconds(1)) + try await handler.waitForCallback(timeout: .seconds(1)) + try await self.maybeInContext { handler.assert(callbackCount: 2, cancelCount: 0) } + + _ = try self.loop.scheduleCallback(in: .milliseconds(2), handler: handler) + _ = try self.loop.scheduleCallback(in: .milliseconds(3), handler: handler) - await fulfillment(of: [handler.timerDidFire], timeout: 0.01) - XCTAssertEqual(handler.firedCount, 0) + try await self.advanceTime(by: .milliseconds(3)) + try await handler.waitForCallback(timeout: .seconds(1)) + try await handler.waitForCallback(timeout: .seconds(1)) + try await self.maybeInContext { handler.assert(callbackCount: 4, cancelCount: 0) } } - func testShutdown() async throws { - let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) - let loop = group.next() - defer { group.shutdownGracefully { error in XCTAssertNil(error) } } + func testMultipleSheduledCallbacksUsingDifferentHandlers() async throws { + let handlerA = MockScheduledCallbackHandler() + let handlerB = MockScheduledCallbackHandler() + + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handlerA) + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handlerB) - let handler = MockTimerHandler() - handler.timerDidFire.isInverted = true + try await self.advanceTime(by: .milliseconds(1)) + try await handlerA.waitForCallback(timeout: .seconds(1)) + try await handlerB.waitForCallback(timeout: .seconds(1)) + try await self.maybeInContext { handlerA.assert(callbackCount: 1, cancelCount: 0) } + try await self.maybeInContext { handlerB.assert(callbackCount: 1, cancelCount: 0) } + } + + func testCancelExecutesCancellationCallback() async throws { + let handler = MockScheduledCallbackHandler() + + let scheduledCallback = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + scheduledCallback.cancel() + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 1) } + } + + func testCancelAfterDeadlineDoesNotExecutesCancellationCallback() async throws { + let handler = MockScheduledCallbackHandler() + + let scheduledCallback = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + try await self.advanceTime(by: .milliseconds(1)) + try await handler.waitForCallback(timeout: .seconds(1)) + scheduledCallback.cancel() + try await self.maybeInContext { handler.assert(callbackCount: 1, cancelCount: 0) } + } - _ = try loop.scheduleCallback(in: .milliseconds(100), handler: handler) - try await group.shutdownGracefully() + func testCancelAfterCancelDoesNotCallCancellationCallbackAgain() async throws { + let handler = MockScheduledCallbackHandler() - await fulfillment(of: [handler.timerDidFire], timeout: 0.01) - XCTAssertEqual(handler.firedCount, 0) + let scheduledCallback = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + scheduledCallback.cancel() + scheduledCallback.cancel() + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 1) } + } + + func testCancelAfterShutdownDoesNotCallCancellationCallbackAgain() async throws { + let handler = MockScheduledCallbackHandler() + + let scheduledCallback = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + try await self.shutdownEventLoop() + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 1) } + + scheduledCallback.cancel() + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 1) } + } + + func testShutdownCancelsOutstandingScheduledCallbacks() async throws { + let handler = MockScheduledCallbackHandler() + + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + try await self.shutdownEventLoop() + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 1) } + } + + func testShutdownDoesNotCancelCancelledCallbacksAgain() async throws { + let handler = MockScheduledCallbackHandler() + + let handle = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + handle.cancel() + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 1) } + + try await self.shutdownEventLoop() + try await self.maybeInContext { handler.assert(callbackCount: 0, cancelCount: 1) } + } + + func testShutdownDoesNotCancelPastCallbacks() async throws { + let handler = MockScheduledCallbackHandler() + + _ = try self.loop.scheduleCallback(in: .milliseconds(1), handler: handler) + try await self.advanceTime(by: .milliseconds(1)) + try await handler.waitForCallback(timeout: .seconds(1)) + try await self.maybeInContext { handler.assert(callbackCount: 1, cancelCount: 0) } + + try await self.shutdownEventLoop() + try await self.maybeInContext { handler.assert(callbackCount: 1, cancelCount: 0) } } } -fileprivate final class MockTimerHandler: NIOScheduledCallbackHandler { - var firedCount = 0 - var timerDidFire = XCTestExpectation(description: "Timer fired") +fileprivate final class MockScheduledCallbackHandler: NIOScheduledCallbackHandler { + var callbackCount = 0 + var cancelCount = 0 + + let callbackStream: AsyncStream + private let callbackStreamContinuation: AsyncStream.Continuation + + init() { + (self.callbackStream, self.callbackStreamContinuation) = AsyncStream.makeStream(of: Void.self) + } + + deinit { + self.callbackStreamContinuation.finish() + } func handleScheduledCallback(eventLoop: some EventLoop) { - self.firedCount += 1 - self.timerDidFire.fulfill() + self.callbackCount += 1 + self.callbackStreamContinuation.yield() + } + + func onCancelScheduledCallback(eventLoop: some EventLoop) { + self.cancelCount += 1 + } + + func assert(callbackCount: Int, cancelCount: Int, file: StaticString = #file, line: UInt = #line) { + XCTAssertEqual(self.callbackCount, callbackCount, "Unexpected callback count", file: file, line: line) + XCTAssertEqual(self.cancelCount, cancelCount, "Unexpected cancel count", file: file, line: line) + } + + func waitForCallback(timeout: Duration, file: StaticString = #file, line: UInt = #line) async throws { + try await XCTWithTimeout(timeout, file: file, line: line) { await self.callbackStream.first { _ in true } } + } +} + +/// This function exists because there's no nice way of waiting in tests for something to happen in the handler +/// without an arbitrary sleep. +/// +/// Other options include setting `XCTestCase.allowedExecutionTime` in `setup()` but this doesn't work well because +/// (1), it rounds up to the nearest minute; and (2), it doesn't seem to work reliably. +/// +/// Another option is to install a timebomb in `XCTestCase.setup()` that will fail the test. This works, but you +/// don't get any information on where the test was when it fails. +/// +/// Alternatively, one can use expectations, but these cannot be awaited more than once so won't work for tests where +/// the same handler is used to schedule multiple callbacks. +/// +/// This function is probably a good balance of pragmatism and clarity. +func XCTWithTimeout( + _ timeout: Duration, + file: StaticString = #file, + line: UInt = #line, + operation: @escaping @Sendable () async throws -> Result +) async throws -> Result where Result: Sendable { + do { + return try await withTimeout(timeout, operation: operation) + } catch is CancellationError { + XCTFail("Timed out after \(timeout)", file: file, line: line) + throw CancellationError() } } -#if !canImport(Darwin) && swift(<5.9.2) -extension XCTestCase { - func fulfillment( - of expectations: [XCTestExpectation], - timeout seconds: TimeInterval, - enforceOrder enforceOrderOfFulfillment: Bool = false - ) async { - wait(for: expectations, timeout: seconds) +func withTimeout( + _ timeout: Duration, + operation: @escaping @Sendable () async throws -> Result +) async throws -> Result where Result: Sendable { + try await withThrowingTaskGroup(of: Result.self) { group in + group.addTask { + try await Task.sleep(for: timeout) + throw CancellationError() + } + group.addTask(operation: operation) + let result = try await group.next()! + group.cancelAll() + return result } } -#endif From 81cc415ee2603140548eb5e3d5d1a0890effa367 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 25 Jul 2024 16:12:09 +0100 Subject: [PATCH 33/37] format: Update for new format and lint rules --- Sources/NIOCore/EventLoop.swift | 10 ++++++++-- Sources/NIOCore/NIOScheduledCallback.swift | 16 +++++++++++++--- Sources/NIOEmbedded/AsyncTestingEventLoop.swift | 10 ++++++++-- Sources/NIOEmbedded/Embedded.swift | 5 ++++- Sources/NIOPosix/SelectableEventLoop.swift | 5 ++++- .../NIOScheduledCallbackTests.swift | 2 +- 6 files changed, 38 insertions(+), 10 deletions(-) diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index 3af92ea616..547e242f0d 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -363,14 +363,20 @@ public protocol EventLoop: EventLoopGroup { /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** also implement /// `cancelScheduledCallback`. Failure to do so will result in a runtime error. @discardableResult - func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback + func scheduleCallback( + at deadline: NIODeadline, + handler: some NIOScheduledCallbackHandler + ) throws -> NIOScheduledCallback /// Schedule a callback after given time. /// /// - NOTE: Event loops that provide a custom scheduled callback implementation **must** also implement /// `cancelScheduledCallback`. Failure to do so will result in a runtime error. @discardableResult - func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback + func scheduleCallback( + in amount: TimeAmount, + handler: some NIOScheduledCallbackHandler + ) throws -> NIOScheduledCallback /// Cancel a scheduled callback. /// diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift index d428de516b..9d9b07f44b 100644 --- a/Sources/NIOCore/NIOScheduledCallback.swift +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -89,7 +89,11 @@ public struct NIOScheduledCallback: Sendable { } extension EventLoop { - /* package */ public func _scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + // This could be package once we drop Swift 5.8. + public func _scheduleCallback( + at deadline: NIODeadline, + handler: some NIOScheduledCallbackHandler + ) -> NIOScheduledCallback { let task = self.scheduleTask(deadline: deadline) { handler.handleScheduledCallback(eventLoop: self) } task.futureResult.whenFailure { error in if case .cancelled = error as? EventLoopError { @@ -128,14 +132,20 @@ extension EventLoop { /// The implementation of this default conformance has been further factored out so we can use it in /// `NIOAsyncTestingEventLoop`, where the use of `wait()` is _less bad_. @discardableResult - public func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + public func scheduleCallback( + at deadline: NIODeadline, + handler: some NIOScheduledCallbackHandler + ) -> NIOScheduledCallback { self._scheduleCallback(at: deadline, handler: handler) } /// Default implementation of `scheduleCallback(in amount:handler:)`: calls `scheduleCallback(at deadline:handler:)`. @discardableResult @inlinable - public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { + public func scheduleCallback( + in amount: TimeAmount, + handler: some NIOScheduledCallbackHandler + ) throws -> NIOScheduledCallback { try self.scheduleCallback(at: .now() + amount, handler: handler) } diff --git a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift index 54e2bd5550..93c07faaf7 100644 --- a/Sources/NIOEmbedded/AsyncTestingEventLoop.swift +++ b/Sources/NIOEmbedded/AsyncTestingEventLoop.swift @@ -191,7 +191,10 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { self.scheduleTask(deadline: self.now + `in`, task) } - public func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { + public func scheduleCallback( + at deadline: NIODeadline, + handler: some NIOScheduledCallbackHandler + ) throws -> NIOScheduledCallback { /// The default implementation of `scheduledCallback(at:handler)` makes two calls to the event loop because it /// needs to hook the future of the backing scheduled task, which can lead to lost cancellation callbacks when /// callbacks are scheduled close to event loop shutdown. @@ -210,7 +213,10 @@ public final class NIOAsyncTestingEventLoop: EventLoop, @unchecked Sendable { } @discardableResult - public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { + public func scheduleCallback( + in amount: TimeAmount, + handler: some NIOScheduledCallbackHandler + ) throws -> NIOScheduledCallback { /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it uses a manual clock so /// it cannot rely on the default implementation of `scheduleCallback(in:handler:)`, which computes the deadline /// as an offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self.now`. diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index 2c6396abdc..8426136cab 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -156,7 +156,10 @@ public final class EmbeddedEventLoop: EventLoop { } @discardableResult - public func scheduleCallback(in amount: TimeAmount, handler: some NIOScheduledCallbackHandler) -> NIOScheduledCallback { + public func scheduleCallback( + in amount: TimeAmount, + handler: some NIOScheduledCallbackHandler + ) -> NIOScheduledCallback { /// Even though this type does not implement a custom `scheduleCallback(at:handler)`, it uses a manual clock so /// it cannot rely on the default implementation of `scheduleCallback(in:handler:)`, which computes the deadline /// as an offset from `NIODeadline.now`. This event loop needs the deadline to be offset from `self._now`. diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 8643e622f5..d5359cab5c 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -932,7 +932,10 @@ internal func assertExpression(_ body: () -> Bool) { extension SelectableEventLoop { @inlinable - func scheduleCallback(at deadline: NIODeadline, handler: some NIOScheduledCallbackHandler) throws -> NIOScheduledCallback { + func scheduleCallback( + at deadline: NIODeadline, + handler: some NIOScheduledCallbackHandler + ) throws -> NIOScheduledCallback { let taskID = self.scheduledTaskCounter.loadThenWrappingIncrement(ordering: .relaxed) let task = ScheduledTask(id: taskID, handler, deadline) try self._schedule0(.scheduled(task)) diff --git a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift index 56a629ee3e..8e5185601f 100644 --- a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift +++ b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift @@ -253,7 +253,7 @@ extension _BaseScheduledCallbackTests { } } -fileprivate final class MockScheduledCallbackHandler: NIOScheduledCallbackHandler { +private final class MockScheduledCallbackHandler: NIOScheduledCallbackHandler { var callbackCount = 0 var cancelCount = 0 From 4467728f9c4dda5e3a4c1d608043460e8bfecec8 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Tue, 30 Jul 2024 16:09:53 +0100 Subject: [PATCH 34/37] Remove use of Task.sleep(for:) in tests --- Tests/NIOPosixTests/NIOScheduledCallbackTests.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift index 8e5185601f..81a86c394f 100644 --- a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift +++ b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift @@ -282,7 +282,7 @@ private final class MockScheduledCallbackHandler: NIOScheduledCallbackHandler { XCTAssertEqual(self.cancelCount, cancelCount, "Unexpected cancel count", file: file, line: line) } - func waitForCallback(timeout: Duration, file: StaticString = #file, line: UInt = #line) async throws { + func waitForCallback(timeout: TimeAmount, file: StaticString = #file, line: UInt = #line) async throws { try await XCTWithTimeout(timeout, file: file, line: line) { await self.callbackStream.first { _ in true } } } } @@ -301,7 +301,7 @@ private final class MockScheduledCallbackHandler: NIOScheduledCallbackHandler { /// /// This function is probably a good balance of pragmatism and clarity. func XCTWithTimeout( - _ timeout: Duration, + _ timeout: TimeAmount, file: StaticString = #file, line: UInt = #line, operation: @escaping @Sendable () async throws -> Result @@ -315,12 +315,12 @@ func XCTWithTimeout( } func withTimeout( - _ timeout: Duration, + _ timeout: TimeAmount, operation: @escaping @Sendable () async throws -> Result ) async throws -> Result where Result: Sendable { try await withThrowingTaskGroup(of: Result.self) { group in group.addTask { - try await Task.sleep(for: timeout) + try await Task.sleep(nanoseconds: UInt64(timeout.nanoseconds)) throw CancellationError() } group.addTask(operation: operation) From c48b7aab902568ff97ead8c854199a3513aa02c8 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 15 Aug 2024 14:40:01 +0100 Subject: [PATCH 35/37] Update benchmark to use same config as other benchmarks --- .../NIOPosixBenchmarks/Benchmarks.swift | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift index ecd911ec47..3cc33b5492 100644 --- a/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift +++ b/Benchmarks/Benchmarks/NIOPosixBenchmarks/Benchmarks.swift @@ -71,31 +71,26 @@ let benchmarks = { Benchmark( "MTELG.scheduleTask(in:_:)", configuration: Benchmark.Configuration( - metrics: [.mallocCountTotal, .instructions], - scalingFactor: .kilo + metrics: defaultMetrics, + scalingFactor: .mega, + maxDuration: .seconds(10_000_000), + maxIterations: 5 ) ) { benchmark in - let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) - defer { try! group.syncShutdownGracefully() } - let loop = group.next() - - benchmark.startMeasurement() for _ in benchmark.scaledIterations { - loop.scheduleTask(in: .hours(1), {}) + eventLoop.scheduleTask(in: .hours(1), {}) } } Benchmark( "MTELG.scheduleCallback(in:_:)", configuration: Benchmark.Configuration( - metrics: [.mallocCountTotal, .instructions], - scalingFactor: .kilo + metrics: defaultMetrics, + scalingFactor: .mega, + maxDuration: .seconds(10_000_000), + maxIterations: 5 ) ) { benchmark in - let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) - defer { try! group.syncShutdownGracefully() } - let loop = group.next() - final class Timer: NIOScheduledCallbackHandler { func handleScheduledCallback(eventLoop: some EventLoop) {} } @@ -103,7 +98,7 @@ let benchmarks = { benchmark.startMeasurement() for _ in benchmark.scaledIterations { - let handle = try! loop.scheduleCallback(in: .hours(1), handler: timer) + let handle = try! eventLoop.scheduleCallback(in: .hours(1), handler: timer) } } } From 151c2c82217d4eb837edf3b58224b9329d97f2c1 Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 15 Aug 2024 14:51:57 +0100 Subject: [PATCH 36/37] Rename onCancelScheduledCallback to didCancelScheduledCallback --- Sources/NIOCore/NIOScheduledCallback.swift | 8 ++++---- Sources/NIOPosix/SelectableEventLoop.swift | 4 ++-- Tests/NIOPosixTests/NIOScheduledCallbackTests.swift | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/NIOCore/NIOScheduledCallback.swift b/Sources/NIOCore/NIOScheduledCallback.swift index 9d9b07f44b..84c9f8d202 100644 --- a/Sources/NIOCore/NIOScheduledCallback.swift +++ b/Sources/NIOCore/NIOScheduledCallback.swift @@ -27,12 +27,12 @@ public protocol NIOScheduledCallbackHandler { /// implicitly, if it was still pending when the event loop was shut down. /// /// - Parameter eventLoop: The event loop on which the callback was scheduled. - func onCancelScheduledCallback(eventLoop: some EventLoop) + func didCancelScheduledCallback(eventLoop: some EventLoop) } extension NIOScheduledCallbackHandler { - /// Default implementation of `onCancelScheduledCallback(eventLoop:)`: does nothing. - public func onCancelScheduledCallback(eventLoop: some EventLoop) {} + /// Default implementation of `didCancelScheduledCallback(eventLoop:)`: does nothing. + public func didCancelScheduledCallback(eventLoop: some EventLoop) {} } /// An opaque handle that can be used to cancel a scheduled callback. @@ -97,7 +97,7 @@ extension EventLoop { let task = self.scheduleTask(deadline: deadline) { handler.handleScheduledCallback(eventLoop: self) } task.futureResult.whenFailure { error in if case .cancelled = error as? EventLoopError { - handler.onCancelScheduledCallback(eventLoop: self) + handler.didCancelScheduledCallback(eventLoop: self) } } return NIOScheduledCallback(self, task) diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index d5359cab5c..79ba55b6c8 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -702,7 +702,7 @@ internal final class SelectableEventLoop: EventLoop { failFn(EventLoopError._shutdown) // Call the cancellation handler for all the scheduled callbacks. case .callback(let handler): - handler.onCancelScheduledCallback(eventLoop: self) + handler.didCancelScheduledCallback(eventLoop: self) } } @@ -955,7 +955,7 @@ extension SelectableEventLoop { guard case .callback(let handler) = scheduledTask.kind else { preconditionFailure("Incorrect task kind for callback") } - handler.onCancelScheduledCallback(eventLoop: self) + handler.didCancelScheduledCallback(eventLoop: self) } } } diff --git a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift index 81a86c394f..7038b7129d 100644 --- a/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift +++ b/Tests/NIOPosixTests/NIOScheduledCallbackTests.swift @@ -273,7 +273,7 @@ private final class MockScheduledCallbackHandler: NIOScheduledCallbackHandler { self.callbackStreamContinuation.yield() } - func onCancelScheduledCallback(eventLoop: some EventLoop) { + func didCancelScheduledCallback(eventLoop: some EventLoop) { self.cancelCount += 1 } From ba63fda1cf0ff7312b9beffc0488d6aa788c081b Mon Sep 17 00:00:00 2001 From: Si Beaumont Date: Thu, 3 Oct 2024 20:58:19 +0100 Subject: [PATCH 37/37] Make AsyncStream.makeStream backfill internal --- Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift b/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift index c425de8d81..98d8943598 100644 --- a/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift +++ b/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift @@ -1485,7 +1485,7 @@ final class AsyncChannelBootstrapTests: XCTestCase { @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) extension AsyncStream { - fileprivate static func makeStream( + static func makeStream( of elementType: Element.Type = Element.self, bufferingPolicy limit: Continuation.BufferingPolicy = .unbounded ) -> (stream: AsyncStream, continuation: AsyncStream.Continuation) {