think about associated data when started

[weissi]

Manual state tracking has the advantage that you're able to associate data with a state. For example

enum LifecycleState {
    case stopped
    case started(Channel)
    case stopping(Channel)
}

migrating this to Lifecycle currently would require to add an optional

var serverChannel: Channel?

and then the code would start to contain self.serverChannel!. The other option is to duplicate the state tracking in Lifecycle as well as the application but that's not nice either because the states can get out of sync.

[tomerd]

I think this can be potentially achieved with the current API. check out the following test

 func testExternalState() {
        enum State: Equatable {
            case idle
            case started(String)
            case shutdown
        }
        
        class Item {
            let eventLoopGroup: EventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)

            let data: String
            init(_ data: String) {
                self.data = data
            }
            
            func start() -> EventLoopFuture<String> {
                self.eventLoopGroup.next().makeSucceededFuture(self.data)
            }

            func shutdown() -> EventLoopFuture<Void> {
                self.eventLoopGroup.next().makeSucceededFuture(())
            }
        }
        
        var state = State.idle
        
        let expectedData = UUID().uuidString
        let item = Item(expectedData)
        let lifecycle = Lifecycle()
        lifecycle.register(name: "test",
                            start: .async{
                                item.start().map{ data -> Void in
                                    state = .started(data)
                                }
                            },
                            shutdown: .async{
                                item.shutdown().map{ _ -> Void in
                                    state = .shutdown
                                }
                            })

        lifecycle.start(configuration: .init(shutdownSignal: nil)) { error in
            XCTAssertNil(error, "not expecting error")
            XCTAssert(state == .started(expectedData), "expected item to be shutdown, but \(state)")
            lifecycle.shutdown()
        }
        lifecycle.wait()
        XCTAssert(state == .shutdown, "expected item to be shutdown, but \(state)")
    }

if you are trying to point out to a different issue, please elaborate

[tomerd]

cc @yim-lee @ktoso @ddossot

[weissi]

@tomerd sure, you can add manual state tracking again but then you also need to add locking etc. The test case as it stands today doesn't work because self.state gets read/written from different threads without synchronisation.

I was just hoping that we can come up with something better because otherwise we need to track the state in two places, think about locking, ...

Another thing to consider is what happens if we get a signal whilst holding a lock, then shutdown won't complete I think? It's just that lifecycle management is hard and I don't know if the library helps as much as it could? One option would be to give lifecycle some state? Maybe Lifecycle<StartedState, StoppedState>? Not sure if that's a good idea but i thought we should think about it.

[ddossot]

Considering the synchronization concerns @weissi mentions it seems that Lifecycle should (optionally?) provide a way to safely assemble started/stopped states as it progresses through the up/down steps. Lifecycle<StartedState, StoppedState> sounds like an interesting idea, I imagine StartedState would be a non optional return of lifecycle.start?

[tomerd]

@weissi @ddossot the original design thinking here was that state would be contained within each LifecycleItem. so, to @weissi's example if some unit of logic uses swift-nio and needs to track a Channel it would not be done externally to the Lifecycle but rather encapsulated in a LifecycleItem that gets started/stopped by Lifecycle.

If I understand your point correctly, the idea of Lifecycle<StartedState, StoppedState> could interesting if we feel different LifecycleItems need to co-operate towards some external, bigger, state that is composed from the state provided by different LifecycleItem start/stop return values. Is this what you have in mind?

[ktoso]

Long story short: the var state = State.idle is terrifying (for concurrency reasons) and something I really really would want to not teach people that "it's fine". I outlined an API shape that could work below. If you'd like me to help out and PoC this I'm happy to.

So getting the type erasure dance will "the usual pain" but what we'd want is something among the lines of:

public class LifecycleItem<State> {
    var label: String { get }
    func start(callback: @escaping (Error?) -> State)
    func shutdown(callback: @escaping (State?, Error?) -> Void)
}

// we'd need
let stages: [AnyLifecycleItem]

for stage in stages {
    // ~~~ pseudo-code ~~~
    let state = stage.invokeStart()
    stage.store(state) // or just invokeStart stores it 
}

for stage in shuttingDownStatges {
    // ~~~ pseudo-code ~~~
    stage.invokeShutdown() // shutdown(stage.storedState, maybeError)
}

we'd need type erasure dance around the storage of LifecycleItems

[tomerd]

thanks @ktoso for the additional input.

Long story short: the var state = State.idle is terrifying (for concurrency reasons) and something I really really would want to not teach people that "it's fine"

of course, this is for illustration purposes

I outlined an API shape that could work below.

its sill not clear to me what is the advantage of the library offering an API around an item's state, unless items needs to expose their state to other items. iow, there is nothing in the current design preventing an item to manage it state in a contained way without the lifecycle library knowing / caring about it. what is the advantage of proposed API? do you see a need in items exposing their state to other items?

this is what I mean in #11 (comment)

[yim-lee]

Based on previous experiences I think it's sufficient to keep any associated state/data within the individual LifecycleItems themselves--by this I am not suggesting we move the proposed feature from LifecycleState to LifecycleItem, but rather, if for example a HTTPServer "server" is a LifecycleItem and it has Channel, then Channel is an attribute of HTTPServer and not LifecycleState's.

[tomerd]

Based on previous experiences I think it's sufficient to keep any associated state/data within the individual LifecycleItems themselves--by this I am not suggesting we move the proposed feature from LifecycleState to LifecycleItem, but rather, if for example a HTTPServer "server" is a LifecycleItem and it has Channel, then Channel is an attribute of HTTPServer and not LifecycleState's

yes. this was the original design and how I am using it too

[rauhul]

As a new user to Lifecycle, I found the need to erase my EventLoopFuture<Channel> to EventLoopFuture<Void> a bit cumbersome/unintuitive. I had to store the result of start in a sideband promise so I could properly shutdown the Channel.

ex:

final class MetricsHTTPServer {
    let lifecycle = ComponentLifecycle(label: "foo")

    private(set) var channelPromise: EventLoopPromise<Channel>
    let eventLoopGroup: EventLoopGroup

    init(eventLoopGroup: EventLoopGroup) {
        self.eventLoopGroup = eventLoopGroup
        self.channelPromise = eventLoopGroup.next().makePromise(of: Channel.self)
        lifecycle.register(
            label: "Channel",
            start: .eventLoopFuture(self.start),
            shutdown: .eventLoopFuture(self.shutdown))
    }

    func start() -> EventLoopFuture<Void> {
        let bootstrap = ServerBootstrap(group: eventLoopGroup)
        // ...
        let future = bootstrap.bind(host: ..., port: ...)
        future.cascade(to: channelPromise)
        return future.map { _ in }
    }

    func shutdown() -> EventLoopFuture<Void> {
        return channelPromise.futureResult.flatMap { $0.close() }
    }
}

As a user, I would've preferred to defer the storage of the Channel to the ComponentLifecycle and have it passed back to me as part of the shutdown callback.