Deadlock using manualResetEvent - c#

95 views Asked by At

In My Sample UI, there are 3 buttons. Start button, pause, resume. The start button is async/await and will call a method:

private async void btnStart_Click()
{
    await Task.Run(() => StartingMethod());
}

and we have a global ManualResetEvent named _pauseEvent. and here is the structure of StartingMethod():

public void StartingMethod()
{
   for (int i =0; i < 1000000; i++)
   {
      _pauseEvent.WaitOne();
      // Do Something
   }
}

and here is pause/resume buttons:

private async void btnPause_Click()
{
    await Task.Run(() => _pauseEvent.Reset());
}

private async void btnResume_Click()
{
    await Task.Run(() => _pauseEvent.Set());
}

but sometimes it will give The handle is invalid for the ManualResetEvent on pause/resume call. It seems it is a race condition problem. so I decided to create a global lock object.

Object _lockObject = new object();

and here is the new ones:

public void StartingMethod()
{
   for (int i =0; i < 1000000; i++)
   {
      lock (_lockObject)
         _pauseEvent.WaitOne();
      // Do Something
   }
}

private async void btnPause_Click()
{
    await Task.Run(() => lock (_lockObject) _pauseEvent.Reset());
}

private async void btnResume_Click()
{
    await Task.Run(() => lock (_lockObject) _pauseEvent.Set());
}

But it seems, here again, I will face a DeadLock problem with the lock object. How can I handle such a situation?

1

There are 1 answers

1
Andriy Shevchenko On

I'm pretty sure using async void, Task.Run and lock would result in a deadlock. What I suggest to do is use a CancellationToken together with some intermediate state to represent a progress of StartingMethod.

Pay attetion it will not handle concurrent execution of many StartingMethods and you would need to use something like CanExecute to prevent user from clicking start button many times in a row.

private int _state = 0;

public void StartingMethod(CancellationToken cancellation, bool resetState)
{
   if (resetState)
   {
       _state = 0;
      // reset your additional data  
   }
   for (int i = _state; i < 1000000; i++)
   {
      if (!cancellation.IsCancellationRequested)
      { 
          // Recover saved state
          // Do Something
      }
      else
      {
          // save intermediate state
          // save your additional data
          _state = i;
      }
   }
}

I would strongly suggest to add try-catch to an async method:

private CancellationTokenSource _cts;

private async Task HandleStart(bool resetState)
{ 
   _cts = new CancellationTokenSource();
    try
    {
        await Task.Run(() => StartingMethod(_cts.Token, resetState));
    }
    catch (Exception e)
    {
    // log or show some error on UI
    }
    finally
    {
        _cts.Dispose();
    }
}

private void btnStart_Click()
{
    // No need to await because it's a single statement in a method
    HandleStart(true);
}

private async void btnPause_Click()
{
    _cts.Cancel();
}

private void btnResume_Click()
{
    HandleStart(false);
}