Using async WhenStarted & WhenStopped methods within TopShelf

Question

We use TopShelf like so to start our services. We are seeing slightly odd issues around Service Start and Stop though and wondered if it was due to our async start / stop methods. Having looked at the documentation using async is never mentioned. There is one mentionon their github pages which states that you should not use async in this manner.

But, having said that, it compiles and runs (mostly) ok. So is this correct or should I use a .Wait() instead?

var host = HostFactory.New(hostConfig =>
{
    hostConfig.Service<StreamClient>(serviceConfig =>
    {
        serviceConfig.ConstructUsing(name => new StreamClient());
        serviceConfig.WhenStarted(async tc => await tc.Start());
        serviceConfig.WhenStopped(async tc => await tc.Stop());
    });

    hostConfig.RunAsLocalSystem();

    hostConfig.SetDescription("Stream Client Service");
    hostConfig.SetDisplayName("Stream Client Service");
    hostConfig.SetServiceName("StreamClientService");
});

host.Run();

@Nkosi asked what the method signatures look like, they are async heavy and start internal clients and processes.

public async Task Start()
{
    // Dont start again if we are already running, or if we are already in the starting state
    if (this.Running || this.Starting)
    {
        return;
    }

    await this.slackService.SendSlackServiceEvent(ServiceEventType.Starting, serviceName, applicationVersion);

    this.Starting = true;
    this.Stopping = false;

    var configurationBuilder = new ClientConfigurationBuilder();

    ClientConfiguration clientConfiguration;
    if (Constants.UseLocalConnection)
    {
        await this.OnClientDebugMessage($"Using Local Connection");
        clientConfiguration = configurationBuilder.CreateLocalConfiguration();
    }
    else
    {
        await this.OnClientDebugMessage($"Using SQL Connection");
        clientConfiguration = configurationBuilder.CreateSqlConfiguration();
    }

    this.ClusterGrainClient = await this.StartClient(clientConfiguration);

    if (this.ClusterGrainClient == null)
    {
        using (ConsoleColours.TextColour(ConsoleColor.Red))
        {
            await this.OnClientDebugMessage($"Cluster client null, aborting!");
        }

        return;
    }

    this.Running = true;

    await this.OnClientStarted();
    await this.slackService.SendSlackServiceEvent(ServiceEventType.Started, serviceName, applicationVersion);
    this.Starting = false;

}

Answer 1

You are essentially doing an async void in those delegates which are fire and forget.

Reference Async/Await - Best Practices in Asynchronous Programming

Event handlers are the one exception to that rule where it is allowed

Convert your start and stop methods to synchronous methods that raise async events that can be awaited internally.

public void Start() {
    Started += OnStarted; //subscribe to event
    Started(this, EventArgs.Empty); //raise event
}

private event EventHandler Started = delegate { };

private async void OnStart(object sender, EventArgs args) {
    Started -= OnStarted;        
    await StartAsync();
}

public async Task StartAsync() {
    // ...
}

And then call the start as normal

serviceConfig.WhenStarted(_ => _.Start());

which would raise the event and flow as expected.