0

In the below example, a call to HandleChangesAsync is made from within an asynchronous task, and via an event handler.

I'm concerned there's a potential race condition where one thread is executing the create_task block in HandleChangesAsync. While that's happening, the OnSomethingChanged handler fires, and a second thread ends up executing the same create_task block in HandleChangesAsync,

Question - Assuming this concern is legitimate, what's the right way to eliminate this race condition?

void MyClass::DoStuffAsync()
{    
    WeakReference weakThis(this);
    create_task(DoMoreStuffAsync())
        .then([weakThis]())
    {
        auto strongThis = weakThis.Resolve<HomePromotionTemplateViewModel>();
        if (strongThis)
        {
            strongThis->RegisterForChanges();
            strongThis->HandleChangesAsync();
        }
    });
}

void MyClass::RegisterForChanges()
{    
    // Attach event handler to &MyClass::OnSomethingChanged
}

void MyClass::OnSomethingChanged()
{
    HandleChangesAsync();
}

void MyClass::HandleChangesAsync()
{    
    WeakReference weakThis(this);
    create_task(DoMoreCoolStuffAsync())
        .then([weakThis]())
    {
        // do stuff 
    });
}
Craig
  • 1,890
  • 1
  • 26
  • 44
  • 1
    If you're going to downvote, please leave a comment explaining why. – Craig Apr 20 '18 at 23:51
  • Async ~ threads here? If so, don't the thread locking/signalling primitives apply? – user2864740 Apr 21 '18 at 00:02
  • @user2864740 - Then my question is where sync primitives belong in this example. The HandleChangesAsync function will return almost immediately since the create_task body will run later. So surrounding that with thread locking will be pointless. – Craig Apr 21 '18 at 00:03
  • @user2864740 - And I have not found a UWP example that covers this. – Craig Apr 21 '18 at 00:05
  • https://learn.microsoft.com/en-us/windows/uwp/threading-async/asynchronous-programming-in-cpp-universal-windows-platform-apps -- covers a bit of where the tasks run under "Managing the thread context". The important bit is about if they run on the UI thread or not. – user2864740 Apr 21 '18 at 00:06
  • @user2864740 - Don't see how that article helps at all. I understand that I can control the thread context; I'm not asking that. I'm asking if I have two background threads running the same async code block, how do I synchronize that execution within the create_task paradigm? Do I need to apply synchronization primitives in every continuation block? is that pattern safe/recommended? – Craig Apr 21 '18 at 00:11
  • 1
    Keep in mind that async code is *not* designed to encourage concurrency. It helps to deal with *latency*, code just taking a long time to get somewhere, usually due to networking delays. Or the api forcing async execution, as is done in WinRT and designed to improve battery life. Expectation is that only one thread ever executes any of this code, not necessarily the exact same thread. And that you take counter-measures in whatever code that activates this stuff to ensure only one async operation can be in flight. Usually by disabling a widget in the UI. – Hans Passant Apr 21 '18 at 08:22
  • It's not entirely clear what the problem is; assuming it is in `//do stuff` then what's the problem? If two calls come in "at the same time", should the code run only once (the second one is ignored) or should it run twice in series (second one is queued up behind the first). What if the second call comes after the first has finished? Or is the problem somewhere else? – Peter Torr - MSFT Apr 21 '18 at 20:23
  • @PeterTorr-MSFT - I've opened a new, more specific question so the ask is clearer. https://stackoverflow.com/questions/49960811/how-can-i-serialize-thread-execution-across-a-task-continuation-block – Craig Apr 21 '18 at 22:43

1 Answers1

0

One simple way to avoid the race condition is to chain the tasks you are creating in HandleChangesAsync so you will be sure that they will be executed one after the other.

To do so, you will need to add new members in your class like

task<void> _handleChangesAsyncTask;
std::mutex _lock;

Then your code in HandleChangesAsync becomes:

void MyClass::HandleChangesAsync()
{    
    std::lock_guard<std::mutex> lk(_lock);
    WeakReference weakThis(this);
    handleChangesAsyncTask = handleChangesAsyncTask.then(
        []()
    {
        return create_task(DoMoreCoolStuffAsync());
    }).then(
        [weakThis]()
    {
        // do stuff 
    });
}

Notice the mutex to prevent the concurrent accesses to _handleChangesAsyncTask.

Vincent
  • 3,656
  • 1
  • 23
  • 32