Skip to content

Commit

Permalink
George review
Browse files Browse the repository at this point in the history
  • Loading branch information
FranzBusch committed Oct 13, 2023
1 parent 8fc2e5d commit 9eff7ba
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 85 deletions.
60 changes: 38 additions & 22 deletions Sources/ServiceLifecycle/ServiceGroup.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 let maximumGracefulShutdownDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
/// The maximum amount of time that task cancellation is allowed to take.
private let maximumCancellationDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
/// The signals that lead to graceful shutdown.
private let gracefulShutdownSignals: [UnixSignal]
/// The signals that lead to cancellation.
Expand All @@ -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``.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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(())
}
}
Expand All @@ -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:
Expand Down Expand Up @@ -400,7 +404,7 @@ public actor ServiceGroup: Sendable {
"All services finished."
)

cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
return .success(())
}
}
Expand Down Expand Up @@ -433,7 +437,7 @@ public actor ServiceGroup: Sendable {
]
)

cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
}

case .gracefulShutdownCaught:
Expand All @@ -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
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -549,7 +556,7 @@ public actor ServiceGroup: Sendable {
]
)

cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
throw ServiceGroupError.serviceFinishedUnexpectedly()
}

Expand Down Expand Up @@ -601,7 +608,7 @@ public actor ServiceGroup: Sendable {
]
)

cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
}

case .gracefulShutdownTimedOut:
Expand All @@ -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.
Expand All @@ -645,19 +652,28 @@ public actor ServiceGroup: Sendable {
}

private func cancelGroupAndSpawnTimeoutIfNeeded(
group: inout ThrowingTaskGroup<ChildTaskResult, Error>
) -> Task<Void, Never>? {
group: inout ThrowingTaskGroup<ChildTaskResult, Error>,
cancellationTimeoutTask: inout Task<Void, Never>?
) {
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."
)
Expand All @@ -667,7 +683,7 @@ public actor ServiceGroup: Sendable {
}
}
} else {
return nil
cancellationTimeoutTask = nil
}
}
}
Expand Down
115 changes: 54 additions & 61 deletions Sources/ServiceLifecycle/ServiceGroupConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand All @@ -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``.
///
Expand Down
4 changes: 2 additions & 2 deletions Tests/ServiceLifecycleTests/ServiceGroupTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1190,8 +1190,8 @@ final class ServiceGroupTests: XCTestCase {
cancellationSignals: cancellationSignals,
logger: logger
)
configuration.escalation.maximumGracefulShutdownDuration = maximumGracefulShutdownDuration
configuration.escalation.maximumCancellationDuration = maximumCancellationDuration
configuration.maximumGracefulShutdownDuration = maximumGracefulShutdownDuration
configuration.maximumCancellationDuration = maximumCancellationDuration
return .init(
configuration: configuration
)
Expand Down

0 comments on commit 9eff7ba

Please sign in to comment.