Skip to content

Commit 8772634

Browse files
FranzBuschFranzBusch
committed
George review
1 parent 8fc2e5d commit 8772634

File tree

2 files changed

+93
-83
lines changed

2 files changed

+93
-83
lines changed

‎Sources/ServiceLifecycle/ServiceGroup.swift

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@ public actor ServiceGroup: Sendable {
3333
private let logger: Logger
3434
/// The logging configuration.
3535
private let loggingConfiguration: ServiceGroupConfiguration.LoggingConfiguration
36-
/// The escalation configuration.
37-
private let escalationConfiguration: ServiceGroupConfiguration.EscalationBehaviour
36+
/// The maximum amount of time that graceful shutdown is allowed to take.
37+
private var maximumGracefulShutdownDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
38+
/// The maximum amount of time that task cancellation is allowed to take.
39+
private var maximumCancellationDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
3840
/// The signals that lead to graceful shutdown.
3941
private let gracefulShutdownSignals: [UnixSignal]
4042
/// The signals that lead to cancellation.
@@ -59,7 +61,8 @@ public actor ServiceGroup: Sendable {
5961
self.cancellationSignals = configuration.cancellationSignals
6062
self.logger = configuration.logger
6163
self.loggingConfiguration = configuration.logging
62-
self.escalationConfiguration = configuration.escalation
64+
self.maximumGracefulShutdownDuration = configuration._maximumCancellationDuration
65+
self.maximumCancellationDuration = configuration._maximumCancellationDuration
6366
}
6467

6568
/// Initializes a new ``ServiceGroup``.
@@ -97,7 +100,8 @@ public actor ServiceGroup: Sendable {
97100
self.cancellationSignals = configuration.cancellationSignals
98101
self.logger = logger
99102
self.loggingConfiguration = configuration.logging
100-
self.escalationConfiguration = configuration.escalation
103+
self.maximumGracefulShutdownDuration = configuration._maximumCancellationDuration
104+
self.maximumCancellationDuration = configuration._maximumCancellationDuration
101105
}
102106

103107
/// Runs all the services by spinning up a child task per service.
@@ -310,7 +314,7 @@ public actor ServiceGroup: Sendable {
310314
self.loggingConfiguration.keys.serviceKey: "\(service.service)",
311315
]
312316
)
313-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
317+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
314318
return .failure(ServiceGroupError.serviceFinishedUnexpectedly())
315319

316320
case .gracefullyShutdownGroup:
@@ -345,7 +349,7 @@ public actor ServiceGroup: Sendable {
345349
self.logger.debug(
346350
"All services finished."
347351
)
348-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
352+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
349353
return .success(())
350354
}
351355
}
@@ -360,7 +364,7 @@ public actor ServiceGroup: Sendable {
360364
self.loggingConfiguration.keys.errorKey: "\(serviceError)",
361365
]
362366
)
363-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
367+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
364368
return .failure(serviceError)
365369

366370
case .gracefullyShutdownGroup:
@@ -400,7 +404,7 @@ public actor ServiceGroup: Sendable {
400404
"All services finished."
401405
)
402406

403-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
407+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
404408
return .success(())
405409
}
406410
}
@@ -433,7 +437,7 @@ public actor ServiceGroup: Sendable {
433437
]
434438
)
435439

436-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
440+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
437441
}
438442

439443
case .gracefulShutdownCaught:
@@ -455,7 +459,7 @@ public actor ServiceGroup: Sendable {
455459
// We caught cancellation in our child task so we have to spawn
456460
// our cancellation timeout task if needed
457461
self.logger.debug("Caught cancellation.")
458-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
462+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
459463

460464
case .signalSequenceFinished, .gracefulShutdownFinished:
461465
// This can happen when we are either cancelling everything or
@@ -491,9 +495,12 @@ public actor ServiceGroup: Sendable {
491495
fatalError("Unexpected state")
492496
}
493497

494-
if #available(macOS 13.0, *), let maximumGracefulShutdownDuration = self.escalationConfiguration.maximumGracefulShutdownDuration {
498+
if #available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *), let maximumGracefulShutdownDuration = self.maximumGracefulShutdownDuration {
495499
group.addTask {
496-
try await Task.sleep(for: maximumGracefulShutdownDuration)
500+
try await Task.sleep(for: Duration(
501+
secondsComponent: maximumGracefulShutdownDuration.secondsComponent,
502+
attosecondsComponent: maximumGracefulShutdownDuration.attosecondsComponent
503+
))
497504
return .gracefulShutdownTimedOut
498505
}
499506
}
@@ -549,7 +556,7 @@ public actor ServiceGroup: Sendable {
549556
]
550557
)
551558

552-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
559+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
553560
throw ServiceGroupError.serviceFinishedUnexpectedly()
554561
}
555562

@@ -601,7 +608,7 @@ public actor ServiceGroup: Sendable {
601608
]
602609
)
603610

604-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
611+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
605612
}
606613

607614
case .gracefulShutdownTimedOut:
@@ -613,13 +620,13 @@ public actor ServiceGroup: Sendable {
613620
self.loggingConfiguration.keys.serviceKey: "\(service.service)",
614621
]
615622
)
616-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
623+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
617624

618625
case .cancellationCaught:
619626
// We caught cancellation in our child task so we have to spawn
620627
// our cancellation timeout task if needed
621628
self.logger.debug("Caught cancellation.")
622-
cancellationTimeoutTask = self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group)
629+
self.cancelGroupAndSpawnTimeoutIfNeeded(group: &group, cancellationTimeoutTask: &cancellationTimeoutTask)
623630

624631
case .signalSequenceFinished, .gracefulShutdownCaught, .gracefulShutdownFinished:
625632
// We just have to tolerate this since signals and parent graceful shutdowns downs can race.
@@ -645,19 +652,29 @@ public actor ServiceGroup: Sendable {
645652
}
646653

647654
private func cancelGroupAndSpawnTimeoutIfNeeded(
648-
group: inout ThrowingTaskGroup<ChildTaskResult, Error>
649-
) -> Task<Void, Never>? {
655+
group: inout ThrowingTaskGroup<ChildTaskResult, Error>,
656+
cancellationTimeoutTask: inout Task<Void, Never>?
657+
) {
658+
guard cancellationTimeoutTask == nil else {
659+
// We already have a cancellation timeout task running.
660+
return
661+
}
650662
group.cancelAll()
651-
if #available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *), let maximumCancellationDuration = self.escalationConfiguration.maximumCancellationDuration {
663+
664+
665+
if #available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *), let maximumCancellationDuration = self.maximumCancellationDuration {
652666
// We have to spawn an unstructured task here because the call to our `run`
653667
// method might have already been cancelled and we need to protect the sleep
654668
// from being cancelled.
655-
return Task {
669+
cancellationTimeoutTask = Task {
656670
do {
657671
self.logger.debug(
658672
"Task cancellation timeout task started."
659673
)
660-
try await Task.sleep(for: maximumCancellationDuration)
674+
try await Task.sleep(for: Duration(
675+
secondsComponent: maximumCancellationDuration.secondsComponent,
676+
attosecondsComponent: maximumCancellationDuration.attosecondsComponent
677+
))
661678
self.logger.debug(
662679
"Cancellation took longer than allowed by the configuration."
663680
)
@@ -667,7 +684,7 @@ public actor ServiceGroup: Sendable {
667684
}
668685
}
669686
} else {
670-
return nil
687+
cancellationTimeoutTask = nil
671688
}
672689
}
673690
}

‎Sources/ServiceLifecycle/ServiceGroupConfiguration.swift

Lines changed: 54 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -96,65 +96,6 @@ public struct ServiceGroupConfiguration: Sendable {
9696
}
9797
}
9898

99-
/// The group's escalation configuration.
100-
public struct EscalationBehaviour: Sendable {
101-
/// The maximum amount of time that graceful shutdown is allowed to take.
102-
///
103-
/// After this time has elapsed graceful shutdown will be escalated to task cancellation.
104-
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
105-
public var maximumGracefulShutdownDuration: Duration? {
106-
get {
107-
if let maximumGracefulShutdownDuration = self._maximumGracefulShutdownDuration {
108-
return .init(
109-
secondsComponent: maximumGracefulShutdownDuration.secondsComponent,
110-
attosecondsComponent: maximumGracefulShutdownDuration.attosecondsComponent
111-
)
112-
} else {
113-
return nil
114-
}
115-
}
116-
set {
117-
if let newValue = newValue {
118-
self._maximumGracefulShutdownDuration = (newValue.components.seconds, newValue.components.attoseconds)
119-
} else {
120-
self._maximumCancellationDuration = nil
121-
}
122-
}
123-
}
124-
125-
/// The maximum amount of time that task cancellation is allowed to take.
126-
///
127-
/// After this time has elapsed task cancellation will be escalated to a `fatalError`.
128-
///
129-
/// - Important: This setting is useful to guarantee that your application will exit at some point and
130-
/// should be used to identify APIs that are not properly implementing task cancellation.
131-
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
132-
public var maximumCancellationDuration: Duration? {
133-
get {
134-
if let maximumCancellationDuration = self._maximumCancellationDuration {
135-
return .init(
136-
secondsComponent: maximumCancellationDuration.secondsComponent,
137-
attosecondsComponent: maximumCancellationDuration.attosecondsComponent
138-
)
139-
} else {
140-
return nil
141-
}
142-
}
143-
set {
144-
if let newValue = newValue {
145-
self._maximumCancellationDuration = (newValue.components.seconds, newValue.components.attoseconds)
146-
} else {
147-
self._maximumCancellationDuration = nil
148-
}
149-
}
150-
}
151-
152-
private var _maximumGracefulShutdownDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
153-
private var _maximumCancellationDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
154-
155-
public init() {}
156-
}
157-
15899
/// The groups's service configurations.
159100
public var services: [ServiceConfiguration]
160101

@@ -170,8 +111,60 @@ public struct ServiceGroupConfiguration: Sendable {
170111
/// The group's logging configuration.
171112
public var logging = LoggingConfiguration()
172113

173-
/// The group's escalation configuration.
174-
public var escalation = EscalationBehaviour()
114+
/// The maximum amount of time that graceful shutdown is allowed to take.
115+
///
116+
/// After this time has elapsed graceful shutdown will be escalated to task cancellation.
117+
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
118+
public var maximumGracefulShutdownDuration: Duration? {
119+
get {
120+
if let maximumGracefulShutdownDuration = self._maximumGracefulShutdownDuration {
121+
return .init(
122+
secondsComponent: maximumGracefulShutdownDuration.secondsComponent,
123+
attosecondsComponent: maximumGracefulShutdownDuration.attosecondsComponent
124+
)
125+
} else {
126+
return nil
127+
}
128+
}
129+
set {
130+
if let newValue = newValue {
131+
self._maximumGracefulShutdownDuration = (newValue.components.seconds, newValue.components.attoseconds)
132+
} else {
133+
self._maximumGracefulShutdownDuration = nil
134+
}
135+
}
136+
}
137+
138+
internal var _maximumGracefulShutdownDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
139+
140+
/// The maximum amount of time that task cancellation is allowed to take.
141+
///
142+
/// After this time has elapsed task cancellation will be escalated to a `fatalError`.
143+
///
144+
/// - Important: This setting is useful to guarantee that your application will exit at some point and
145+
/// should be used to identify APIs that are not properly implementing task cancellation.
146+
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
147+
public var maximumCancellationDuration: Duration? {
148+
get {
149+
if let maximumCancellationDuration = self._maximumCancellationDuration {
150+
return .init(
151+
secondsComponent: maximumCancellationDuration.secondsComponent,
152+
attosecondsComponent: maximumCancellationDuration.attosecondsComponent
153+
)
154+
} else {
155+
return nil
156+
}
157+
}
158+
set {
159+
if let newValue = newValue {
160+
self._maximumCancellationDuration = (newValue.components.seconds, newValue.components.attoseconds)
161+
} else {
162+
self._maximumCancellationDuration = nil
163+
}
164+
}
165+
}
166+
167+
internal var _maximumCancellationDuration: (secondsComponent: Int64, attosecondsComponent: Int64)?
175168

176169
/// Initializes a new ``ServiceGroupConfiguration``.
177170
///

0 commit comments

Comments
 (0)