0

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?

eglease
  • 2,445
  • 11
  • 18
  • 28
Inside Man
  • 4,194
  • 12
  • 59
  • 119
  • why do you need to wrap `_pauseEvent.Reset()` and `_pauseEvent.Set()` in a `Task.Run` ? – Andriy Shevchenko Oct 19 '21 at 11:02
  • It's just a simple view of my work. pausing may take some minutes to do, so I can not freeze the UI. so I should use `Task.Run` – Inside Man Oct 19 '21 at 11:03
  • Could you please post full `StartingMethod` ? – Andriy Shevchenko Oct 19 '21 at 11:08
  • you think in the Rest of `StatringMethod` I'm converting some videos... – Inside Man Oct 19 '21 at 11:12
  • If you get an invalid handle error for `_pauseEvent` it must be an issue with the lifetime of the handle. Do you create/destroy new event instances as you go? – 500 - Internal Server Error Oct 19 '21 at 11:30
  • BTW, deadlock is not the same as invalid handle. If you actually get a deadlock it would be helpful to know the calls stacks when this occurs. – 500 - Internal Server Error Oct 19 '21 at 11:36
  • You probably need a fully async version https://stackoverflow.com/questions/18756354/wrapping-manualresetevent-as-awaitable-task – Charlieface Oct 19 '21 at 11:45
  • @500-InternalServerError I just only create a new instance of `ManualResetEvent` once – Inside Man Oct 19 '21 at 11:49
  • That was the wrong way to deal with this issue, now you have two problems. It is *not* a race problem, "The handle is invalid" indicates a much more serious issue. One that the snippet gives no hints to. Start by observing the number of handles your program creates with Task Manager, add the "Handles" column. A steadily increasing count is not going to end well. If it doesn't then you'll have to suspect memory corruption issues. – Hans Passant Oct 19 '21 at 11:51

1 Answers1

0

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);
}
Andriy Shevchenko
  • 1,117
  • 6
  • 21
  • thanks, but you are trying to give me a solution for handling the loop inside the `Starting Method`. my question is about to handle the `ManualResetEvent` in a thread-safe manner – Inside Man Oct 19 '21 at 11:45