0

Using .Net Framework:4.7.1 Expectation: - The simulation is started in a new task - A message box will be displayed when the simulation has completed. - The simulation will continue delaying 250ms for a duration of 30 seconds.

Problem: Task.WhenAll waits until the simulation delays. Hovering the mouse over the simulation task instance reports it as completed even though it has not. The simulation will continue to run until 30 seconds has completed then the task exists.

Is this a bug?

When Task.Delay is commented out, it works as expected.

 public partial class Form1 : Form
{
    private List<Task> m_simulations = new List<Task>();

    public Form1()
    {
        InitializeComponent();
    }

    private async void button1_Click(object sender, EventArgs e)
    {
        m_simulations.Add(
            Task.Run(() =>
              {
                  Simulation simulation = new Simulation();
                  simulation.StartSimulation(new TimeSpan(0, 0, 30));
              })
            );

        await Task.WhenAll(m_simulations);

        MessageBox.Show("Tasks Complete Apparently!!!");
    }
}

public class Simulation
{
    public async void StartSimulation(TimeSpan waitDuration)
    {
        Stopwatch stopwatch = Stopwatch.StartNew();

        while (stopwatch.Elapsed <= waitDuration)
        {
            await Task.Delay(250);
        }
    }
}
Fabien
  • 21
  • 3
  • 1
    You are not awaiting the `StartSimulation` task (and its not returning a task because you used `async void`). Why even use the Task.Run, just have StartSimulation return a task and await on that. – John Koerner Feb 22 '18 at 16:31

1 Answers1

1

By using Task.Run you're creating a task to invoke the StartSimulation method, but you're never awaiting the simulation to complete, so the task you're awaiting ends after StartSimulation has begun, but before it is finished.

First, change StartSimulation to return a Task:

public async Task StartSimulation(TimeSpan waitDuration)

Then, rather than using Task.Run, just start the simulation and use the returned Task in your list of things to wait for.

    Simulation simulation = new Simulation();
    m_simulations.Add(simulation.StartSimulation(new TimeSpan(0, 0, 30));
StriplingWarrior
  • 151,543
  • 27
  • 246
  • 315
  • Your solution works, I have changed the click method so there are multiple instances of the simulation running concurrently, would you agree this a good approach?? `await Task.Run(() => { Parallel.For(0, 3, (i) => { m_simulations.Add(new Simulation().StartSimulation(new TimeSpan(0, 0, 5), i)); }); }); await Task.WhenAll(m_simulations);` – Fabien Feb 22 '18 at 17:42
  • @Fabien: No, I don't think that's a good approach. Assuming your simulation is truly asynchronous then it can be *concurrent* without running in *parallel*. This should give you roughly the same performance boost without exposing you to the risks of multithreading. For example, `List<>` is not thread-safe, but the code in your comment adds items to it from multiple threads concurrently. Why not just do `m_Simulations.AddRange(Enumerable.Range(0, 3, i => new Simulation().StartSimulation(new TimeSpan(0, 0, 5), i))); await Task.WhenAll(m_simulations);`? – StriplingWarrior Feb 22 '18 at 20:18
  • See [this SO post](https://stackoverflow.com/questions/1050222/what-is-the-difference-between-concurrency-and-parallelism) for a discussion on concurrent vs parallel code – StriplingWarrior Feb 22 '18 at 20:19
  • There are exceptions with the code due to Enumerable.Range only taking 2 parameters ints and it returning an int when m_Simulations.AddRange expects a collection. However I have understood the points you put across, you have been a great help StriplingWarrior. – Fabien Feb 23 '18 at 10:21
  • @Fabien: Sorry, I meant: `m_Simulations.AddRange(Enumerable.Range(0, 3).Select(i => new Simulation().StartSimulation(new TimeSpan(0, 0, 5), i))); await Task.WhenAll(m_simulations);` It's hard to write meaningful code in these little comments boxes. – StriplingWarrior Feb 23 '18 at 16:25