Race in moduleService wrapper

[pstibrany]

moduleService wrapper is responsible for waiting until all dependencies start, and then start the wrapped service. It does so by calling

dskit/modules/module_service.go

Line 65 in de9a686

err := w.service.StartAsync(context.Background())

and then waiting for service to enter Running state:

dskit/modules/module_service.go

Line 70 in de9a686

return w.service.AwaitRunning(serviceContext)

However if the wrapped service is so fast that it ends before AwaitRunning call, then AwaitRunning returns error invalid service state: Terminated, expected: Running.

We can solve this by starting AwaitRunning before StartAsync (in separate goroutine), or adding a listener to the service instead and wait for any transition away from Starting state.

[pracucci]

or adding a listener to the service instead and wait for any transition away from Starting state.

This looks cleaner. However, doesn't it break the start() contract? Shouldn't the start() successfully return only if the state is running?

[pstibrany]

or adding a listener to the service instead and wait for any transition away from Starting state.

This looks cleaner.

I weakly prefer goroutine approach tbh. That listener will stay there registered forever, receiving updates.

However, doesn't it break the start() contract? Shouldn't the start() successfully return only if the state is running?

Do you mean StartAsync()? No, its contract is:

	// StartAsync starts Service asynchronously. Service must be in New State, otherwise error is returned.
	// Context is used as a parent context for service own context.

It returns quickly, and doesn't wait for any state transition. (Well, implementation in BasicService executes New to Starting transition, but that's not specified in the comment.)

[pstibrany]

However, doesn't it break the start() contract? Shouldn't the start() successfully return only if the state is running?

Ah, I see now what you mean. Yes, you're right – we need to wait until services' Starting function finishes (succesfully or not). So listener would either react on running or failed transitions.

[pstibrany]

Yes, you're right – we need to wait until services' Starting function finishes (succesfully or not). So listener would either react on running or failed transitions.

(Btw, these are the only available transitions from starting state)

[pstibrany]

services.StartAndAwaitRunning has the same problem :-(

We cannot use goroutine-approach that I preferred, it just doesn't work: We need to run AwaitRunning in different goroutine than StartAsync, but we don't know when it is safe to call StartAsync. It needs to happen only when AwaitRunning is already waiting, but the coordination is tricky.

I'm going to use listener-based approach, and fix services.StartAndAwaitRunning too.