From 877263485c43454a460102f982265caab0a90b93 Mon Sep 17 00:00:00 2001 From: Franz Busch Date: Fri, 13 Oct 2023 10:05:16 +0100 Subject: [PATCH] George review --- Sources/ServiceLifecycle/ServiceGroup.swift | 61 ++++++---- .../ServiceGroupConfiguration.swift | 115 ++++++++---------- 2 files changed, 93 insertions(+), 83 deletions(-) diff --git a/Sources/ServiceLifecycle/ServiceGroup.swift b/Sources/ServiceLifecycle/ServiceGroup.swift index 6710fa2..01d2598 100644 --- a/Sources/ServiceLifecycle/ServiceGroup.swift +++ b/Sources/ServiceLifecycle/ServiceGroup.swift @@ -33,8 +33,10 @@ public actor ServiceGroup: Sendable { private let logger: Logger /// The logging configuration. private let loggingConfiguration: ServiceGroupConfiguration.LoggingConfiguration - /// The escalation configuration. - private let escalationConfiguration: ServiceGroupConfiguration.EscalationBehaviour + /// The maximum amount of time that graceful shutdown is allowed to take. + private var maximumGracefulShutdownDuration: (secondsComponent: Int64, attosecondsComponent: Int64)? + /// The maximum amount of time that task cancellation is allowed to take. + private var maximumCancellationDuration: (secondsComponent: Int64, attosecondsComponent: Int64)? /// The signals that lead to graceful shutdown. private let gracefulShutdownSignals: [UnixSignal] /// The signals that lead to cancellation. @@ -59,7 +61,8 @@ public actor ServiceGroup: Sendable { self.cancellationSignals = configuration.cancellationSignals self.logger = configuration.logger self.loggingConfiguration = configuration.logging - self.escalationConfiguration = configuration.escalation + self.maximumGracefulShutdownDuration = configuration._maximumCancellationDuration + self.maximumCancellationDuration = configuration._maximumCancellationDuration } /// Initializes a new ``ServiceGroup``. @@ -97,7 +100,8 @@ public actor ServiceGroup: Sendable { self.cancellationSignals = configuration.cancellationSignals self.logger = logger self.loggingConfiguration = configuration.logging - self.escalationConfiguration = configuration.escalation + self.maximumGracefulShutdownDuration = configuration._maximumCancellationDuration + self.maximumCancellationDuration = configuration._maximumCancellationDuration } /// Runs all the services by spinning up a child task per service. @@ -310,7 +314,7 @@ public actor ServiceGroup: Sendable { self.loggingConfiguration.keys.serviceKey: "\(service.service)", ] ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) return .failure(ServiceGroupError.serviceFinishedUnexpectedly()) case .gracefullyShutdownGroup: @@ -345,7 +349,7 @@ public actor ServiceGroup: Sendable { self.logger.debug( "All services finished." ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) return .success(()) } } @@ -360,7 +364,7 @@ public actor ServiceGroup: Sendable { self.loggingConfiguration.keys.errorKey: "\(serviceError)", ] ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) return .failure(serviceError) case .gracefullyShutdownGroup: @@ -400,7 +404,7 @@ public actor ServiceGroup: Sendable { "All services finished." ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) return .success(()) } } @@ -433,7 +437,7 @@ public actor ServiceGroup: Sendable { ] ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) } case .gracefulShutdownCaught: @@ -455,7 +459,7 @@ public actor ServiceGroup: Sendable { // We caught cancellation in our child task so we have to spawn // our cancellation timeout task if needed self.logger.debug("Caught cancellation.") - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) case .signalSequenceFinished, .gracefulShutdownFinished: // This can happen when we are either cancelling everything or @@ -491,9 +495,12 @@ public actor ServiceGroup: Sendable { fatalError("Unexpected state") } - if #available(macOS 13.0, *), let maximumGracefulShutdownDuration = self.escalationConfiguration.maximumGracefulShutdownDuration { + if #available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *), let maximumGracefulShutdownDuration = self.maximumGracefulShutdownDuration { group.addTask { - try await Task.sleep(for: maximumGracefulShutdownDuration) + try await Task.sleep(for: Duration( + secondsComponent: maximumGracefulShutdownDuration.secondsComponent, + attosecondsComponent: maximumGracefulShutdownDuration.attosecondsComponent + )) return .gracefulShutdownTimedOut } } @@ -549,7 +556,7 @@ public actor ServiceGroup: Sendable { ] ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) throw ServiceGroupError.serviceFinishedUnexpectedly() } @@ -601,7 +608,7 @@ public actor ServiceGroup: Sendable { ] ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) } case .gracefulShutdownTimedOut: @@ -613,13 +620,13 @@ public actor ServiceGroup: Sendable { self.loggingConfiguration.keys.serviceKey: "\(service.service)", ] ) - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) case .cancellationCaught: // We caught cancellation in our child task so we have to spawn // our cancellation timeout task if needed self.logger.debug("Caught cancellation.") - cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group) + self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask) case .signalSequenceFinished, .gracefulShutdownCaught, .gracefulShutdownFinished: // We just have to tolerate this since signals and parent graceful shutdowns downs can race. @@ -645,19 +652,29 @@ public actor ServiceGroup: Sendable { } private func cancelGroupAndSpawnTimeoutIfNeeded( - group: inout ThrowingTaskGroup - ) -> Task? { + group: inout ThrowingTaskGroup, + cancellationTimeoutTask: inout Task? + ) { + guard cancellationTimeoutTask == nil else { + // We already have a cancellation timeout task running. + return + } group.cancelAll() - if #available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *), let maximumCancellationDuration = self.escalationConfiguration.maximumCancellationDuration { + + + if #available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *), let maximumCancellationDuration = self.maximumCancellationDuration { // We have to spawn an unstructured task here because the call to our `run` // method might have already been cancelled and we need to protect the sleep // from being cancelled. - return Task { + cancellationTimeoutTask = Task { do { self.logger.debug( "Task cancellation timeout task started." ) - try await Task.sleep(for: maximumCancellationDuration) + try await Task.sleep(for: Duration( + secondsComponent: maximumCancellationDuration.secondsComponent, + attosecondsComponent: maximumCancellationDuration.attosecondsComponent + )) self.logger.debug( "Cancellation took longer than allowed by the configuration." ) @@ -667,7 +684,7 @@ public actor ServiceGroup: Sendable { } } } else { - return nil + cancellationTimeoutTask = nil } } } diff --git a/Sources/ServiceLifecycle/ServiceGroupConfiguration.swift b/Sources/ServiceLifecycle/ServiceGroupConfiguration.swift index b1a0f9d..6dcfb43 100644 --- a/Sources/ServiceLifecycle/ServiceGroupConfiguration.swift +++ b/Sources/ServiceLifecycle/ServiceGroupConfiguration.swift @@ -96,65 +96,6 @@ public struct ServiceGroupConfiguration: Sendable { } } - /// The group's escalation configuration. - public struct EscalationBehaviour: Sendable { - /// The maximum amount of time that graceful shutdown is allowed to take. - /// - /// After this time has elapsed graceful shutdown will be escalated to task cancellation. - @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) - public var maximumGracefulShutdownDuration: Duration? { - get { - if let maximumGracefulShutdownDuration = self._maximumGracefulShutdownDuration { - return .init( - secondsComponent: maximumGracefulShutdownDuration.secondsComponent, - attosecondsComponent: maximumGracefulShutdownDuration.attosecondsComponent - ) - } else { - return nil - } - } - set { - if let newValue = newValue { - self._maximumGracefulShutdownDuration = (newValue.components.seconds, newValue.components.attoseconds) - } else { - self._maximumCancellationDuration = nil - } - } - } - - /// The maximum amount of time that task cancellation is allowed to take. - /// - /// After this time has elapsed task cancellation will be escalated to a `fatalError`. - /// - /// - Important: This setting is useful to guarantee that your application will exit at some point and - /// should be used to identify APIs that are not properly implementing task cancellation. - @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) - public var maximumCancellationDuration: Duration? { - get { - if let maximumCancellationDuration = self._maximumCancellationDuration { - return .init( - secondsComponent: maximumCancellationDuration.secondsComponent, - attosecondsComponent: maximumCancellationDuration.attosecondsComponent - ) - } else { - return nil - } - } - set { - if let newValue = newValue { - self._maximumCancellationDuration = (newValue.components.seconds, newValue.components.attoseconds) - } else { - self._maximumCancellationDuration = nil - } - } - } - - private var _maximumGracefulShutdownDuration: (secondsComponent: Int64, attosecondsComponent: Int64)? - private var _maximumCancellationDuration: (secondsComponent: Int64, attosecondsComponent: Int64)? - - public init() {} - } - /// The groups's service configurations. public var services: [ServiceConfiguration] @@ -170,8 +111,60 @@ public struct ServiceGroupConfiguration: Sendable { /// The group's logging configuration. public var logging = LoggingConfiguration() - /// The group's escalation configuration. - public var escalation = EscalationBehaviour() + /// The maximum amount of time that graceful shutdown is allowed to take. + /// + /// After this time has elapsed graceful shutdown will be escalated to task cancellation. + @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) + public var maximumGracefulShutdownDuration: Duration? { + get { + if let maximumGracefulShutdownDuration = self._maximumGracefulShutdownDuration { + return .init( + secondsComponent: maximumGracefulShutdownDuration.secondsComponent, + attosecondsComponent: maximumGracefulShutdownDuration.attosecondsComponent + ) + } else { + return nil + } + } + set { + if let newValue = newValue { + self._maximumGracefulShutdownDuration = (newValue.components.seconds, newValue.components.attoseconds) + } else { + self._maximumGracefulShutdownDuration = nil + } + } + } + + internal var _maximumGracefulShutdownDuration: (secondsComponent: Int64, attosecondsComponent: Int64)? + + /// The maximum amount of time that task cancellation is allowed to take. + /// + /// After this time has elapsed task cancellation will be escalated to a `fatalError`. + /// + /// - Important: This setting is useful to guarantee that your application will exit at some point and + /// should be used to identify APIs that are not properly implementing task cancellation. + @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) + public var maximumCancellationDuration: Duration? { + get { + if let maximumCancellationDuration = self._maximumCancellationDuration { + return .init( + secondsComponent: maximumCancellationDuration.secondsComponent, + attosecondsComponent: maximumCancellationDuration.attosecondsComponent + ) + } else { + return nil + } + } + set { + if let newValue = newValue { + self._maximumCancellationDuration = (newValue.components.seconds, newValue.components.attoseconds) + } else { + self._maximumCancellationDuration = nil + } + } + } + + internal var _maximumCancellationDuration: (secondsComponent: Int64, attosecondsComponent: Int64)? /// Initializes a new ``ServiceGroupConfiguration``. ///