Skip to content

add semantics for stopping tasks that have no start handler #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 26, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 47 additions & 18 deletions Sources/Lifecycle/Lifecycle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,36 @@ import Logging
/// Represents an item that can be started and shut down
public protocol LifecycleTask {
var label: String { get }
var shutdownIfNotStarted: Bool { get }
func start(_ callback: @escaping (Error?) -> Void)
func shutdown(_ callback: @escaping (Error?) -> Void)
}

extension LifecycleTask {
public var shutdownIfNotStarted: Bool {
return false
}
}

// MARK: - LifecycleHandler

/// Supported startup and shutdown method styles
public struct LifecycleHandler {
private let body: (@escaping (Error?) -> Void) -> Void
internal let noop: Bool

private init(_ callback: @escaping (@escaping (Error?) -> Void) -> Void, noop: Bool) {
self.body = callback
self.noop = noop
}

/// Initialize a `LifecycleHandler` based on a completion handler.
///
/// - parameters:
/// - callback: the underlying completion handler
/// - noop: the underlying completion handler is a no-op
public init(_ callback: @escaping (@escaping (Error?) -> Void) -> Void) {
self.body = callback
self.init(callback, noop: false)
}

/// Asynchronous `LifecycleHandler` based on a completion handler.
Expand Down Expand Up @@ -69,9 +83,9 @@ public struct LifecycleHandler {

/// Noop `LifecycleHandler`.
public static var none: LifecycleHandler {
return LifecycleHandler { callback in
return LifecycleHandler({ callback in
callback(nil)
}
}, noop: true)
}

internal func run(_ callback: @escaping (Error?) -> Void) {
Expand Down Expand Up @@ -242,12 +256,9 @@ public class ComponentLifecycle: LifecycleTask {
private let logger: Logger
internal let shutdownGroup = DispatchGroup()

private var state = State.idle
private var state = State.idle([])
private let stateLock = Lock()

private var tasks = [LifecycleTask]()
private let tasksLock = Lock()

/// Creates a `ComponentLifecycle` instance.
///
/// - parameters:
Expand Down Expand Up @@ -275,7 +286,9 @@ public class ComponentLifecycle: LifecycleTask {
/// - on: `DispatchQueue` to run the handlers callback on
/// - callback: The handler which is called after the start operation completes. The parameter will be `nil` on success and contain the `Error` otherwise.
public func start(on queue: DispatchQueue, _ callback: @escaping (Error?) -> Void) {
let tasks = self.tasksLock.withLock { self.tasks }
guard case .idle(let tasks) = (self.stateLock.withLock { self.state }) else {
preconditionFailure("invalid state, \(self.state)")
}
self._start(on: queue, tasks: tasks, callback: callback)
}

Expand Down Expand Up @@ -311,11 +324,17 @@ public class ComponentLifecycle: LifecycleTask {

self.stateLock.lock()
switch self.state {
case .idle:
case .idle(let tasks) where tasks.isEmpty:
self.state = .shutdown(nil)
self.stateLock.unlock()
defer { self.shutdownGroup.leave() }
callback(nil)
case .idle(let tasks):
self.stateLock.unlock()
let queue = DispatchQueue.global() // is this the best we can do?
setupShutdownListener(queue)
let stoppable = tasks.filter { $0.shutdownIfNotStarted }
self._shutdown(on: queue, tasks: stoppable, callback: self.shutdownGroup.leave)
case .shutdown:
self.stateLock.unlock()
self.log(level: .warning, "already shutdown")
Expand All @@ -328,7 +347,6 @@ public class ComponentLifecycle: LifecycleTask {
self.stateLock.unlock()
setupShutdownListener(queue)
case .started(let queue, let tasks):
self.state = .shuttingDown(queue)
self.stateLock.unlock()
setupShutdownListener(queue)
self._shutdown(on: queue, tasks: tasks, callback: self.shutdownGroup.leave)
Expand Down Expand Up @@ -362,7 +380,12 @@ public class ComponentLifecycle: LifecycleTask {
case .shuttingDown:
self.stateLock.unlock()
// shutdown was called while starting, or start failed, shutdown what we can
let stoppable = started < tasks.count ? Array(tasks.prefix(started + 1)) : tasks
let stoppable: [LifecycleTask]
if started < tasks.count {
stoppable = tasks.enumerated().filter { $0.offset <= started || $0.element.shutdownIfNotStarted }.map { $0.element }
} else {
stoppable = tasks
}
self._shutdown(on: queue, tasks: stoppable) {
callback(error)
self.shutdownGroup.leave()
Expand Down Expand Up @@ -443,7 +466,7 @@ public class ComponentLifecycle: LifecycleTask {
}

private enum State {
case idle
case idle([LifecycleTask])
case starting(DispatchQueue)
case started(DispatchQueue, [LifecycleTask])
case shuttingDown(DispatchQueue)
Expand All @@ -454,12 +477,10 @@ public class ComponentLifecycle: LifecycleTask {
extension ComponentLifecycle: LifecycleTasksContainer {
public func register(_ tasks: [LifecycleTask]) {
self.stateLock.withLock {
guard case .idle = self.state else {
guard case .idle(let existing) = self.state else {
preconditionFailure("invalid state, \(self.state)")
}
}
self.tasksLock.withLock {
self.tasks.append(contentsOf: tasks)
self.state = .idle(existing + tasks)
}
}
}
Expand Down Expand Up @@ -488,8 +509,8 @@ extension LifecycleTasksContainer {
/// - label: label of the item, useful for debugging.
/// - start: `Handler` to perform the startup.
/// - shutdown: `Handler` to perform the shutdown.
public func register(label: String, start: LifecycleHandler, shutdown: LifecycleHandler) {
self.register(_LifecycleTask(label: label, start: start, shutdown: shutdown))
public func register(label: String, shutdownIfNotStarted: Bool? = nil, start: LifecycleHandler, shutdown: LifecycleHandler) {
self.register(_LifecycleTask(label: label, shutdownIfNotStarted: shutdownIfNotStarted, start: start, shutdown: shutdown))
}

/// Adds a `LifecycleTask` to a `LifecycleTasks` collection.
Expand All @@ -504,9 +525,17 @@ extension LifecycleTasksContainer {

internal struct _LifecycleTask: LifecycleTask {
let label: String
let shutdownIfNotStarted: Bool
let start: LifecycleHandler
let shutdown: LifecycleHandler

init(label: String, shutdownIfNotStarted: Bool? = nil, start: LifecycleHandler, shutdown: LifecycleHandler) {
self.label = label
self.shutdownIfNotStarted = shutdownIfNotStarted ?? start.noop
self.start = start
self.shutdown = shutdown
}

func start(_ callback: @escaping (Error?) -> Void) {
self.start.run(callback)
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/LifecycleNIOCompat/Bridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ extension LifecycleHandler {
}
}

extension ServiceLifecycle {
extension ComponentLifecycle {
/// Starts the provided `LifecycleItem` array.
/// Startup is performed in the order of items provided.
///
/// - parameters:
/// - eventLoop: The `eventLoop` which is used to generate the `EventLoopFuture` that is returned. After the start the future is fulfilled:
func start(on eventLoop: EventLoop) -> EventLoopFuture<Void> {
public func start(on eventLoop: EventLoop) -> EventLoopFuture<Void> {
let promise = eventLoop.makePromise(of: Void.self)
self.start { error in
if let error = error {
Expand Down
4 changes: 4 additions & 0 deletions Tests/LifecycleTests/ComponentLifecycleTests+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ extension ComponentLifecycleTests {
("testNIOFailure", testNIOFailure),
("testInternalState", testInternalState),
("testExternalState", testExternalState),
("testNOOPHandlers", testNOOPHandlers),
("testShutdownOnlyStarted", testShutdownOnlyStarted),
("testShutdownWhenStartFailedIfAsked", testShutdownWhenStartFailedIfAsked),
("testShutdownWhenIdleIfAsked", testShutdownWhenIdleIfAsked),
]
}
}
Loading