Code execution blocked with Factory.StartNew

Question

Factory.StartNew leads to code execution blockage when I don't used Thread.Sleep(20);

I tried the following:

Thread.Sleep() - This works with Factory.StartNew & produces desirable result
Task.Delay(20) - This doesn't work with Factory.StartNew
Task<bool>.Run - Using this instead of Factory.StartNew doesn't make any difference

The code:

private async Task<bool> GetStatus()
{                        
  var MyTask = Task<bool>.Factory.StartNew( () => 
  //var MyTask = Task<bool>.Run( () => // Using Task.Run doesn't make any 
  //difference
  {
    while (true)
    {              
      if (EventStatus.ToString() == "Rejected")
        break;
      if (EventStatus.ToString() == "Error")
        break;
      Thread.Sleep(20);// This gives the desirable result.Removing it 
                       //causes application to hang
      //Task.Delay(20);// This line doesn't make any difference
    }                
    return true;
   });            
  MyTask.Wait();            
  return await MyTask;
}

If I use Task.Factory.StartNew without using Thread.Sleep(20) code gets stuck in endless loop.

How can I improve code and get it to work without using Thread.Sleep(20) ?

I tried Task.Factory.StartNew specifying TaskScheduler but that caused code to hang too.

Could it be because `Task.Delay` is awaitable, and you're not awaiting it? — stuartd, Jul 24 '19 at 16:06
What is `EventStatus`? Does it use proper lock? Otherwise C# may be allowed to read it only once, but not on every loop iteration. — user4003407, Jul 24 '19 at 16:10
EventStatus is updated everytime a callaback event fires. No, I'm not using any lock, I'll explore this option. Thank you. — Fred Smith, Jul 24 '19 at 16:18

score 4 · Answer 1 · answered Jul 24 '19 at 18:10

First, don't use StartNew. It's dangerous. Use Task.Run instead.

Second, the reason you're seeing and endless loop is that Task.Delay just starts a timer and returns a task that completes when that timer fires. If you want the code to actually wait for the timer, then you need to await that task. Also, EventStatus is being accessed from multiple threads without protection, which is not good. You'll need to add a lock to fix this permanently:

private async Task<bool> GetStatus()
{
  var task = Task.Run(async () =>
  {
    while (true)
    {              
      // TODO: add lock for EventStatus
      if (EventStatus.ToString() == "Rejected")
        break;
      if (EventStatus.ToString() == "Error")
        break;
      await Task.Delay(20);
    }                

    return true;
  });

  return await task;
}

As a final note, I would get rid of this entirely. Instead of polling for EventStatus to change, make it an actual "signal". E.g., assuming EventStatus is set once, then a TaskCompletionSource<bool> would work fine.

Code execution blocked with Factory.StartNew

1 Answers1