Async/await in Parallel.ForEach

Question

I have a Timer:

var QueryReportTimer = new Timer(QueryReportTimerCallback, null, TimeSpan.Zero, TimeSpan.FromSeconds(15));

The important part here is the method QueryReportTimerCallback.

I have a few doubts here:

• Is it ok to have an async void method? In general it is not, but how can I avoid this when the timer callback is delegate public delegate void TimerCallback(object state);?
• Is it ok to have await inside Parallel.ForEach?

Method QueryReportTimerCallback:

private async void QueryReportTimerCallback(object state)
{
    if (await semaphoreQueryReportSlim.WaitAsync(10))
    {
        try
        {
            if (machineConfigurations != null)
            {
                await Task.Factory.StartNew(() =>
                    Parallel.ForEach(machineConfigurations.Where(x => x.QueryReport), async (configuration) =>
                    {
                        if (configuration.IsConnectionValid)
                        {
                            var queryReport = new QueryReport(configuration, ReportConfigurations, fileContainer, applicationConfiguration, logger);
                            await QueryAReport(configuration, queryReport);
                        }
                    })
                );
            }
        }
        catch (Exception e)
        {
            logger.LogError(e, e.Message);
        }
        finally
        {
            semaphoreQueryReportSlim.Release();
        }
    }
}

Answer 1

Is it ok to have an async void method?

Referencing Async/Await - Best Practices in Asynchronous Programming

As already stated in the OP async void should be avoided as much as possible.

The one exception to that rule being for event handlers, which can be the loophole to achieving the desired behavior while still having the ability to catch and handle any thrown exceptions.

Create an event

event EventHandler QueryReportCallbackEvent = delegate { };

to be raised by the timer callback

private void QueryReportTimerCallback(object state) {
    QueryReportCallbackEvent(this, EventArgs.Empty);
}

The event handler allows async void, so now you can safely do

private async void QueryReportCallbackEventHandler(object sender, EventArgs e) {
    if (await semaphoreQueryReportSlim.WaitAsync(10))
        await queryReportsCore();
}

Is it ok to have await inside Parallel.ForEach?

NO!!!!

The async lambda

async (configuration) => ...

will be converted to async void which takes us right back to what was said in the beginning. async void BAD!!!

Instead refactor that lambda out into its own method that returns a Task

private async Task HandleReport(MachineConfiguration configuration) {
    if (configuration.IsConnectionValid) {
        var queryReport = new QueryReport(configuration, ReportConfigurations, fileContainer, applicationConfiguration, logger);
        await QueryAReport(configuration, queryReport);
    }
}

and this will now allow for the use of Task.WhenAll with all the machine configurations retrieved from the query.

var tasks = machineConfigurations
                .Where(x => x.QueryReport)
                .Select(configuration => HandleReport(configuration));

await Task.WhenAll(tasks);

This actually removes the need for the Paralell.ForEach.

Here is the complete code for what was described above.

//CTOR
public MyClass() {
    QueryReportCallbackEvent += QueryReportCallbackEventHandler;
    var QueryReportTimer = new Timer(QueryReportTimerCallback, null, TimeSpan.Zero, TimeSpan.FromSeconds(15));
}

event EventHandler QueryReportCallbackEvent = delegate { };

private void QueryReportTimerCallback(object state) {
    QueryReportCallbackEvent(this, EventArgs.Empty);
}

private async void QueryReportCallbackEventHandler(object sender, EventArgs e) {
    if (await semaphoreQueryReportSlim.WaitAsync(10))
        await queryReportsCore();
}

private async Task queryReportsCore() {
    try {
        if (machineConfigurations != null) {
            var tasks = machineConfigurations
                            .Where(x => x.QueryReport)
                            .Select(configuration => HandleReport(configuration));

            await Task.WhenAll(tasks);
        }
    } catch (Exception e) {
        logger.LogError(e, e.Message);
    } finally {
        semaphoreQueryReportSlim.Release();
    }
}

private async Task HandleReport(MachineConfiguration configuration) {
    if (configuration.IsConnectionValid) {
        var queryReport = new QueryReport(configuration, ReportConfigurations, fileContainer, applicationConfiguration, logger);
        await QueryAReport(configuration, queryReport);
    }
}

Lastly take note of how the functions were broken down into smaller chunks that allowed for cleaner, more easy to read code.

Answer 2

.net6 now supports Parallel.ForEachAsync

https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.parallel.foreachasync?view=net-6.0