Skip to content

Commit 2513338

Browse files
sliemeobnsliemeobn
committed
renamed to addServiceUnlessShutdown and minor touch-ups
1 parent 84ccf18 commit 2513338

File tree

2 files changed

+43
-36
lines changed

2 files changed

+43
-36
lines changed

‎Sources/ServiceLifecycle/ServiceGroup.swift‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,13 @@ public actor ServiceGroup: Sendable, Service {
108108
self.maximumCancellationDuration = configuration._maximumCancellationDuration
109109
}
110110

111-
/// Adds a service to the group.
111+
/// Adds a new service to the group.
112112
///
113113
/// If the group is currently running, the added service will be started immediately.
114114
/// If the group is gracefully shutting down, cancelling, or already finished, the added service will not be started.
115115
/// - Parameters:
116116
/// - serviceConfiguration: The service configuration to add.
117-
public func addService(_ serviceConfiguration: ServiceGroupConfiguration.ServiceConfiguration) async {
117+
public func addServiceUnlessShutdown(_ serviceConfiguration: ServiceGroupConfiguration.ServiceConfiguration) async {
118118
switch self.state {
119119
case var .initial(services: services):
120120
self.state = .initial(services: [])
@@ -125,18 +125,19 @@ public actor ServiceGroup: Sendable, Service {
125125
await addedServiceChannel.send(serviceConfiguration)
126126

127127
case .finished:
128+
// Since this is a best effort operation we don't have to do anything here
128129
return
129130
}
130131
}
131132

132-
/// Adds a service to the group.
133+
/// Adds a new service to the group.
133134
///
134135
/// If the group is currently running, the added service will be started immediately.
135136
/// If the group is gracefully shutting down, cancelling, or already finished, the added service will not be started.
136137
/// - Parameters:
137138
/// - service: The service to add.
138-
public func addService(_ service: any Service) async {
139-
await self.addService(ServiceGroupConfiguration.ServiceConfiguration(service: service))
139+
public func addServiceUnlessShutdown(_ service: any Service) async {
140+
await self.addServiceUnlessShutdown(ServiceGroupConfiguration.ServiceConfiguration(service: service))
140141
}
141142

142143
/// Runs all the services by spinning up a child task per service.
@@ -225,7 +226,7 @@ public actor ServiceGroup: Sendable, Service {
225226
}
226227
}
227228

228-
fileprivate enum ChildTaskResult {
229+
private enum ChildTaskResult {
229230
case serviceFinished(service: ServiceGroupConfiguration.ServiceConfiguration, index: Int)
230231
case serviceThrew(service: ServiceGroupConfiguration.ServiceConfiguration, index: Int, error: any Error)
231232
case signalCaught(UnixSignal)
@@ -318,8 +319,9 @@ public actor ServiceGroup: Sendable, Service {
318319
let gracefulShutdownManager = GracefulShutdownManager()
319320
gracefulShutdownManagers.append(gracefulShutdownManager)
320321

321-
group.addServiceTask(
322-
serviceConfiguration,
322+
self.addServiceTask(
323+
group: &group,
324+
service: serviceConfiguration,
323325
gracefulShutdownManager: gracefulShutdownManager,
324326
index: index
325327
)
@@ -342,14 +344,14 @@ public actor ServiceGroup: Sendable, Service {
342344
}
343345

344346
// Adds a task that listens to added services and funnels them into the task group
345-
group.addAddedServiceListenerTask(addedServiceChannel)
347+
self.addAddedServiceListenerTask(group: &group, channel: addedServiceChannel)
346348

347349
// We are going to wait for any of the services to finish or
348350
// the signal sequence to throw an error.
349351
while !group.isEmpty {
350-
let nextEvent = try await group.next()
352+
let result: ChildTaskResult? = try await group.next()
351353

352-
switch nextEvent {
354+
switch result {
353355
case .newServiceAdded(let serviceConfiguration):
354356
self.logger.debug(
355357
"Starting added service",
@@ -367,14 +369,18 @@ public actor ServiceGroup: Sendable, Service {
367369
"Mismatch between services and graceful shutdown managers"
368370
)
369371

370-
group.addServiceTask(
371-
serviceConfiguration,
372+
self.addServiceTask(
373+
group: &group,
374+
service: serviceConfiguration,
372375
gracefulShutdownManager: gracefulShutdownManager,
373376
index: services.count - 1
374377
)
375378

376379
// Each listener task can only handle a single added service, so we must add a new listener
377-
group.addAddedServiceListenerTask(addedServiceChannel)
380+
self.addAddedServiceListenerTask(
381+
group: &group,
382+
channel: addedServiceChannel
383+
)
378384

379385
case .serviceFinished(let service, let index):
380386
if group.isCancelled {
@@ -782,8 +788,7 @@ public actor ServiceGroup: Sendable, Service {
782788
break
783789

784790
case .newServiceAdded:
785-
// TBD: How do we treat added services during graceful shutdown?
786-
// Currently, we ignore them - but we make sure that `run` is never called
791+
// Since adding services is best effort, we simply ignore this
787792
break
788793
}
789794
}
@@ -845,17 +850,16 @@ public actor ServiceGroup: Sendable, Service {
845850
cancellationTimeoutTask = nil
846851
}
847852
}
848-
}
849853

850-
extension ThrowingTaskGroup where Failure == Error, ChildTaskResult == ServiceGroup.ChildTaskResult {
851-
mutating func addServiceTask(
852-
_ serviceConfiguration: ServiceGroupConfiguration.ServiceConfiguration,
854+
private func addServiceTask(
855+
group: inout ThrowingTaskGroup<ChildTaskResult, Error>,
856+
service serviceConfiguration: ServiceGroupConfiguration.ServiceConfiguration,
853857
gracefulShutdownManager: GracefulShutdownManager,
854858
index: Int
855859
) {
856860
// This must be addTask and not addTaskUnlessCancelled
857861
// because we must run all the services for the shutdown logic to work.
858-
self.addTask {
862+
group.addTask {
859863
return await TaskLocals.$gracefulShutdownManager.withValue(gracefulShutdownManager) {
860864
do {
861865
try await serviceConfiguration.service.run()
@@ -867,8 +871,11 @@ extension ThrowingTaskGroup where Failure == Error, ChildTaskResult == ServiceGr
867871
}
868872
}
869873

870-
mutating func addAddedServiceListenerTask(_ channel: AsyncChannel<ServiceGroupConfiguration.ServiceConfiguration>) {
871-
self.addTask {
874+
private func addAddedServiceListenerTask(
875+
group: inout ThrowingTaskGroup<ChildTaskResult, Error>,
876+
channel: AsyncChannel<ServiceGroupConfiguration.ServiceConfiguration>
877+
) {
878+
group.addTask {
872879
return await withTaskCancellationHandler {
873880
var iterator = channel.makeAsyncIterator()
874881
if let addedService = await iterator.next() {

‎Tests/ServiceLifecycleTests/ServiceGroupAddServiceTests.swift‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ final class ServiceGroupAddServiceTests: XCTestCase {
2525
func testAddService_whenNotRunning() async {
2626
let mockService = MockService(description: "Service1")
2727
let serviceGroup = self.makeServiceGroup()
28-
await serviceGroup.addService(mockService)
28+
await serviceGroup.addServiceUnlessShutdown(mockService)
2929

3030
await withThrowingTaskGroup(of: Void.self) { group in
3131
group.addTask {
@@ -59,7 +59,7 @@ final class ServiceGroupAddServiceTests: XCTestCase {
5959
var eventIterator2 = mockService2.events.makeAsyncIterator()
6060
await XCTAsyncAssertEqual(await eventIterator1.next(), .run)
6161

62-
await serviceGroup.addService(mockService2)
62+
await serviceGroup.addServiceUnlessShutdown(mockService2)
6363
await XCTAsyncAssertEqual(await eventIterator2.next(), .run)
6464

6565
await mockService1.resumeRunContinuation(with: .success(()))
@@ -87,7 +87,7 @@ final class ServiceGroupAddServiceTests: XCTestCase {
8787
await serviceGroup.triggerGracefulShutdown()
8888
await XCTAsyncAssertEqual(await eventIterator1.next(), .shutdownGracefully)
8989

90-
await serviceGroup.addService(mockService2)
90+
await serviceGroup.addServiceUnlessShutdown(mockService2)
9191

9292
await mockService1.resumeRunContinuation(with: .success(()))
9393
}
@@ -113,7 +113,7 @@ final class ServiceGroupAddServiceTests: XCTestCase {
113113
group.cancelAll()
114114

115115
await XCTAsyncAssertEqual(await eventIterator1.next(), .runCancelled)
116-
await serviceGroup.addService(mockService2)
116+
await serviceGroup.addServiceUnlessShutdown(mockService2)
117117

118118
await mockService1.resumeRunContinuation(with: .success(()))
119119
}
@@ -129,7 +129,7 @@ final class ServiceGroupAddServiceTests: XCTestCase {
129129
gracefulShutdownSignals: [.sigalrm]
130130
)
131131

132-
await serviceGroup.addService(.init(service: service1, successTerminationBehavior: .ignore))
132+
await serviceGroup.addServiceUnlessShutdown(.init(service: service1, successTerminationBehavior: .ignore))
133133

134134
try await withThrowingTaskGroup(of: Void.self) { group in
135135
group.addTask {
@@ -140,7 +140,7 @@ final class ServiceGroupAddServiceTests: XCTestCase {
140140
var eventIterator2 = service2.events.makeAsyncIterator()
141141
await XCTAsyncAssertEqual(await eventIterator1.next(), .run)
142142

143-
await serviceGroup.addService(.init(service: service2, failureTerminationBehavior: .ignore))
143+
await serviceGroup.addServiceUnlessShutdown(.init(service: service2, failureTerminationBehavior: .ignore))
144144
await XCTAsyncAssertEqual(await eventIterator2.next(), .run)
145145

146146
await service1.resumeRunContinuation(with: .success(()))
@@ -172,10 +172,10 @@ final class ServiceGroupAddServiceTests: XCTestCase {
172172
var eventIterator1 = service1.events.makeAsyncIterator()
173173
await XCTAsyncAssertEqual(await eventIterator1.next(), .run)
174174

175-
await serviceGroup.addService(
175+
await serviceGroup.addServiceUnlessShutdown(
176176
.init(service: service2, successTerminationBehavior: .gracefullyShutdownGroup)
177177
)
178-
await serviceGroup.addService(.init(service: service3))
178+
await serviceGroup.addServiceUnlessShutdown(.init(service: service3))
179179

180180
var eventIterator2 = service2.events.makeAsyncIterator()
181181
await XCTAsyncAssertEqual(await eventIterator2.next(), .run)
@@ -223,7 +223,7 @@ final class ServiceGroupAddServiceTests: XCTestCase {
223223
var service1EventIterator = service1.events.makeAsyncIterator()
224224
var service2EventIterator = service2.events.makeAsyncIterator()
225225
await XCTAsyncAssertEqual(await service1EventIterator.next(), .run)
226-
await serviceGroup.addService(service2)
226+
await serviceGroup.addServiceUnlessShutdown(service2)
227227

228228
await XCTAsyncAssertEqual(await service2EventIterator.next(), .run)
229229

@@ -256,8 +256,8 @@ final class ServiceGroupAddServiceTests: XCTestCase {
256256
var eventIterator1 = service1.events.makeAsyncIterator()
257257
await XCTAsyncAssertEqual(await eventIterator1.next(), .run)
258258

259-
await serviceGroup.addService(service2)
260-
await serviceGroup.addService(service3)
259+
await serviceGroup.addServiceUnlessShutdown(service2)
260+
await serviceGroup.addServiceUnlessShutdown(service3)
261261

262262
var eventIterator2 = service2.events.makeAsyncIterator()
263263
await XCTAsyncAssertEqual(await eventIterator2.next(), .run)
@@ -324,8 +324,8 @@ final class ServiceGroupAddServiceTests: XCTestCase {
324324
var eventIterator1 = service1.events.makeAsyncIterator()
325325
await XCTAsyncAssertEqual(await eventIterator1.next(), .run)
326326

327-
await serviceGroup.addService(service2)
328-
await serviceGroup.addService(service3)
327+
await serviceGroup.addServiceUnlessShutdown(service2)
328+
await serviceGroup.addServiceUnlessShutdown(service3)
329329

330330
var eventIterator2 = service2.events.makeAsyncIterator()
331331
await XCTAsyncAssertEqual(await eventIterator2.next(), .run)

0 commit comments

Comments
 (0)