Add some NIO sugar to ServiceLifecycle.start()

[fabianfett]

Added some NIO friendly syntactic sugar, based on this comment: swift-server/swift-aws-lambda-runtime#141 (comment)

Is this something we want to follow through?

[tomerd]

thanks @fabianfett <3

[ktoso]

Oh boy, I missed to "Submit review" yesterday, sorry -_-''

Late, but I have concerns here so submitting now; please have a look @fabianfett

[ktoso]

I think we call them tasks? (LifecycleTask) so the items wording is wording consistent here?

-    /// Startup is performed in the order of items provided.
+    /// Startup is performed in the order of tasks provided.

[ktoso]

I kind of wonder if this has the expected semantics from looking at this signature...

The method reads as "start on eventloop" which to me implies that lifecycle tasks will execute on that event loop, but they won't! they'd run on some dispatch queue anyway and only the future is completed/created from the loop.

Should this rather perform a synchronous start() from inside of the passed in event loop (and also complete the promise as it does correctly)?

eventLoop.execute {
        do { 
            self._start() 
            // we'd need a new start impl, as the only existing one assumes it must run on a dispatch queue
        } catch { promise.fail(error) } 
        promise.succeed(())     
}

in order to be true to its name?

--

Say, someone writes an app and does not synchronize any of their initialization because they assume "it's all on the exact same EL so I don't need to" -- would the current signature lead them down to accidentally actually do have concurrency issues during init?

@fabianfett

[tomerd]

this is a good observation. in practice the lifecycle library always dispatches the start/stop calls onto a dispatch queue, as such naming the argument "on" could be confusing/misleading. do we have alternative proposals?

[ktoso]

I was thinking we could have some internal _start that somehow that gets passed a makeItHappen: () -> ... and that would be the _start(makeItHappen: { () in eventLoop.execute { ... } } or the DispatchQueue one by default. A bit weird but this way we're staying true to what the method signature promises.