Skip to content

when shutdownIfNotStarted is set to false, dont shutdown items that failed to start #107

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.

Sign up for GitHub

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 2 commits into from
Sep 4, 2021
Merged

when shutdownIfNotStarted is set to false, dont shutdown items that failed to start #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
594f2f9
when shutdownIfNotStarted is set to false, dont shutdown items that f…
tomerd Sep 2, 2021
2b66515
fixup
tomerd Sep 2, 2021
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
22 changes: 13 additions & 9 deletions Sources/Lifecycle/Lifecycle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,13 @@ public class ComponentLifecycle: LifecycleTask {
case .shuttingDown:
self.stateLock.unlock()
// shutdown was called while starting, or start failed, shutdown what we can
let stoppable: [LifecycleTask]
if started < tasks.count {
stoppable = tasks.enumerated().filter { $0.offset <= started || $0.element.shutdownIfNotStarted }.map { $0.element }
} else {
stoppable = tasks
var stoppable = started
if started.count < tasks.count {
let shutdownIfNotStarted = tasks.enumerated()
.filter { $0.offset >= started.count }
.map { $0.element }
.filter { $0.shutdownIfNotStarted }
stoppable.append(contentsOf: shutdownIfNotStarted)
}
self._shutdown(on: queue, tasks: stoppable) {
callback(error)
Expand All @@ -596,25 +598,27 @@ public class ComponentLifecycle: LifecycleTask {
}
}

private func startTask(on queue: DispatchQueue, tasks: [LifecycleTask], index: Int, callback: @escaping (Int, Error?) -> Void) {
private func startTask(on queue: DispatchQueue, tasks: [LifecycleTask], index: Int, callback: @escaping ([LifecycleTask], Error?) -> Void) {
// async barrier
let start = { (callback) -> Void in queue.async { tasks[index].start(callback) } }
let callback = { (index, error) -> Void in queue.async { callback(index, error) } }

if index >= tasks.count {
return callback(index, nil)
return callback(tasks, nil)
}
self.logger.info("starting tasks [\(tasks[index].label)]")
let startTime = DispatchTime.now()
start { error in
Timer(label: "\(self.label).\(tasks[index].label).lifecycle.start").recordNanoseconds(DispatchTime.now().uptimeNanoseconds - startTime.uptimeNanoseconds)
if let error = error {
self.logger.error("failed to start [\(tasks[index].label)]: \(error)")
return callback(index, error)
let started = Array(tasks.prefix(index))
return callback(started, error)
}
// shutdown called while starting
if case .shuttingDown = self.stateLock.withLock({ self.state }) {
return callback(index, nil)
let started = index < tasks.count ? Array(tasks.prefix(index + 1)) : tasks
Copy link
Collaborator

@yim-lee yim-lee Sep 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would the task at index be considered started?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a non-error case, meaning the task was started successfully. do you see something else?

return callback(started, nil)
}
self.startTask(on: queue, tasks: tasks, index: index + 1, callback: callback)
}
Expand Down
2 changes: 2 additions & 0 deletions Tests/LifecycleTests/ComponentLifecycleTests+XCTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ extension ComponentLifecycleTests {
("testNOOPHandlers", testNOOPHandlers),
("testShutdownOnlyStarted", testShutdownOnlyStarted),
("testShutdownWhenStartFailedIfAsked", testShutdownWhenStartFailedIfAsked),
("testShutdownWhenStartFailsAndAsked", testShutdownWhenStartFailsAndAsked),
("testStatefulSync", testStatefulSync),
("testStatefulSyncStartError", testStatefulSyncStartError),
("testStatefulSyncShutdownError", testStatefulSyncShutdownError),
Expand All @@ -71,6 +72,7 @@ extension ComponentLifecycleTests {
("testAsyncAwait", testAsyncAwait),
("testAsyncAwaitStateful", testAsyncAwaitStateful),
("testAsyncAwaitErrorOnStart", testAsyncAwaitErrorOnStart),
("testAsyncAwaitErrorOnStartShutdownRequested", testAsyncAwaitErrorOnStartShutdownRequested),
("testAsyncAwaitErrorOnShutdown", testAsyncAwaitErrorOnShutdown),
("testMetrics", testMetrics),
]
Expand Down
181 changes: 157 additions & 24 deletions Tests/LifecycleTests/ComponentLifecycleTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,21 +911,21 @@ final class ComponentLifecycleTests: XCTestCase {
func testShutdownOnlyStarted() {
class Item {
let label: String
let sempahore: DispatchSemaphore
let semaphore: DispatchSemaphore
let failStart: Bool
let exptectedState: State
let expectedState: State
var state = State.idle

deinit {
XCTAssertEqual(self.state, self.exptectedState, "\"\(self.label)\" should be \(self.exptectedState)")
self.sempahore.signal()
XCTAssertEqual(self.state, self.expectedState, "\"\(self.label)\" should be \(self.expectedState)")
self.semaphore.signal()
}

init(label: String, failStart: Bool, exptectedState: State, sempahore: DispatchSemaphore) {
init(label: String, failStart: Bool, expectedState: State, semaphore: DispatchSemaphore) {
self.label = label
self.failStart = failStart
self.exptectedState = exptectedState
self.sempahore = sempahore
self.expectedState = expectedState
self.semaphore = semaphore
}

func start() throws {
Expand All @@ -951,11 +951,12 @@ final class ComponentLifecycleTests: XCTestCase {
}

let count = Int.random(in: 10 ..< 20)
let sempahore = DispatchSemaphore(value: count)
let semaphore = DispatchSemaphore(value: count)
let lifecycle = ServiceLifecycle(configuration: .init(shutdownSignal: nil))

for index in 0 ..< count {
let item = Item(label: "\(index)", failStart: index == count / 2, exptectedState: index <= count / 2 ? .shutdown : .idle, sempahore: sempahore)
let failStart = index == count / 2
let item = Item(label: "\(index)", failStart: failStart, expectedState: failStart ? .error : index <= count / 2 ? .shutdown : .idle, semaphore: semaphore)
lifecycle.register(label: item.label, start: .sync(item.start), shutdown: .sync(item.shutdown))
}

Expand All @@ -965,25 +966,29 @@ final class ComponentLifecycleTests: XCTestCase {
}
lifecycle.wait()

XCTAssertEqual(.success, sempahore.wait(timeout: .now() + 1))
XCTAssertEqual(.success, semaphore.wait(timeout: .now() + 1))
}

func testShutdownWhenStartFailedIfAsked() {
class DestructionSensitive {
let label: String
let failStart: Bool
let sempahore: DispatchSemaphore
let semaphore: DispatchSemaphore
var state = State.idle

deinit {
XCTAssertEqual(self.state, .shutdown, "\"\(self.label)\" should be shutdown")
self.sempahore.signal()
if failStart {
XCTAssertEqual(self.state, .error, "\"\(self.label)\" should be error")
} else {
XCTAssertEqual(self.state, .shutdown, "\"\(self.label)\" should be shutdown")
}
self.semaphore.signal()
}

init(label: String, failStart: Bool = false, sempahore: DispatchSemaphore) {
init(label: String, failStart: Bool = false, semaphore: DispatchSemaphore) {
self.label = label
self.failStart = failStart
self.sempahore = sempahore
self.semaphore = semaphore
}

func start() throws {
Expand All @@ -1008,25 +1013,25 @@ final class ComponentLifecycleTests: XCTestCase {
struct InitError: Error {}
}

let sempahore = DispatchSemaphore(value: 6)
let semaphore = DispatchSemaphore(value: 6)
let lifecycle = ServiceLifecycle(configuration: .init(shutdownSignal: nil))

let item1 = DestructionSensitive(label: "1", sempahore: sempahore)
let item1 = DestructionSensitive(label: "1", semaphore: semaphore)
lifecycle.register(label: item1.label, start: .sync(item1.start), shutdown: .sync(item1.shutdown))

let item2 = DestructionSensitive(label: "2", sempahore: sempahore)
let item2 = DestructionSensitive(label: "2", semaphore: semaphore)
lifecycle.registerShutdown(label: item2.label, .sync(item2.shutdown))

let item3 = DestructionSensitive(label: "3", failStart: true, sempahore: sempahore)
let item3 = DestructionSensitive(label: "3", failStart: true, semaphore: semaphore)
lifecycle.register(label: item3.label, start: .sync(item3.start), shutdown: .sync(item3.shutdown))

let item4 = DestructionSensitive(label: "4", sempahore: sempahore)
let item4 = DestructionSensitive(label: "4", semaphore: semaphore)
lifecycle.registerShutdown(label: item4.label, .sync(item4.shutdown))

let item5 = DestructionSensitive(label: "5", sempahore: sempahore)
let item5 = DestructionSensitive(label: "5", semaphore: semaphore)
lifecycle.register(label: item5.label, start: .none, shutdown: .sync(item5.shutdown))

let item6 = DestructionSensitive(label: "6", sempahore: sempahore)
let item6 = DestructionSensitive(label: "6", semaphore: semaphore)
lifecycle.register(_LifecycleTask(label: item6.label, shutdownIfNotStarted: true, start: .sync(item6.start), shutdown: .sync(item6.shutdown)))

lifecycle.start { error in
Expand All @@ -1035,7 +1040,99 @@ final class ComponentLifecycleTests: XCTestCase {
}
lifecycle.wait()

XCTAssertEqual(.success, sempahore.wait(timeout: .now() + 1))
XCTAssertEqual(.success, semaphore.wait(timeout: .now() + 1))
}

func testShutdownWhenStartFailsAndAsked() {
class BadItem: LifecycleTask {
let label: String = UUID().uuidString
var shutdown: Bool = false

func start(_ callback: (Error?) -> Void) {
callback(TestError())
}

func shutdown(_ callback: (Error?) -> Void) {
self.shutdown = true
callback(nil)
}
}

do {
let lifecycle = ComponentLifecycle(label: "test")

let item = BadItem()
lifecycle.register(label: "test", start: .async(item.start), shutdown: .async(item.shutdown), shutdownIfNotStarted: true)

XCTAssertThrowsError(try lifecycle.startAndWait()) { error in
XCTAssert(error is TestError, "expected error to match")
}

XCTAssertTrue(item.shutdown, "expected item to be shutdown")
}

do {
let lifecycle = ComponentLifecycle(label: "test")

let item = BadItem()
lifecycle.register(label: "test", start: .async(item.start), shutdown: .async(item.shutdown), shutdownIfNotStarted: false)

XCTAssertThrowsError(try lifecycle.startAndWait()) { error in
XCTAssert(error is TestError, "expected error to match")
}

XCTAssertFalse(item.shutdown, "expected item to be not shutdown")
}

do {
let lifecycle = ComponentLifecycle(label: "test")

let item1 = GoodItem()
lifecycle.register(item1)

let item2 = BadItem()
lifecycle.register(label: "test", start: .async(item2.start), shutdown: .async(item2.shutdown), shutdownIfNotStarted: true)

let item3 = GoodItem()
lifecycle.register(item3)

let item4 = GoodItem()
lifecycle.registerShutdown(label: "test", .async(item4.shutdown))

XCTAssertThrowsError(try lifecycle.startAndWait()) { error in
XCTAssert(error is TestError, "expected error to match")
}

XCTAssertEqual(item1.state, .shutdown, "expected item to be shutdown")
XCTAssertTrue(item2.shutdown, "expected item to be shutdown")
XCTAssertEqual(item3.state, .idle, "expected item to be idle")
XCTAssertEqual(item4.state, .shutdown, "expected item to be shutdown")
}

do {
let lifecycle = ComponentLifecycle(label: "test")

let item1 = GoodItem()
lifecycle.register(item1)

let item2 = BadItem()
lifecycle.register(label: "test", start: .async(item2.start), shutdown: .async(item2.shutdown), shutdownIfNotStarted: false)

let item3 = GoodItem()
lifecycle.register(item3)

let item4 = GoodItem()
lifecycle.registerShutdown(label: "test", .async(item4.shutdown))

XCTAssertThrowsError(try lifecycle.startAndWait()) { error in
XCTAssert(error is TestError, "expected error to match")
}

XCTAssertEqual(item1.state, .shutdown, "expected item to be shutdown")
XCTAssertFalse(item2.shutdown, "expected item to be not shutdown")
XCTAssertEqual(item3.state, .idle, "expected item to be idle")
XCTAssertEqual(item4.state, .shutdown, "expected item to be shutdown")
}
}

func testStatefulSync() {
Expand Down Expand Up @@ -1398,7 +1495,43 @@ final class ComponentLifecycleTests: XCTestCase {
let lifecycle = ComponentLifecycle(label: "test")

let item = Item()
lifecycle.register(label: "test", start: .async(item.start), shutdown: .async(item.shutdown))
lifecycle.register(label: "test", start: .async(item.start), shutdown: .async(item.shutdown), shutdownIfNotStarted: false)

lifecycle.start { error in
XCTAssert(error is TestError, "expected error to match")
lifecycle.shutdown()
}
lifecycle.wait()
XCTAssertFalse(item.isShutdown, "expected item to be shutdown")
#endif
}

func testAsyncAwaitErrorOnStartShutdownRequested() throws {
#if compiler(<5.2)
return
#elseif compiler(<5.5)
throw XCTSkip()
#else
guard #available(macOS 12.0, *) else {
throw XCTSkip()
}

class Item {
var isShutdown: Bool = false

func start() async throws {
throw TestError()
}

func shutdown() async throws {
self.isShutdown = true // not thread safe but okay for this purpose
}
}

let lifecycle = ComponentLifecycle(label: "test")

let item = Item()
lifecycle.register(label: "test", start: .async(item.start), shutdown: .async(item.shutdown), shutdownIfNotStarted: true)

lifecycle.start { error in
XCTAssert(error is TestError, "expected error to match")
Expand Down