-1

I believe I have a race condition in this code sample but am unsure how to mitigate it.

My scenario is that XAsync() always runs on the UI thread. Within XAsync(), I set m_importantMemberVariable and then start a timer; there's a 1 second delay before the timer fires.

My concern is the timer's tick event calls a method on m_importantMemberVariable. However, in the 1 second interval between starting the timer and Tick firing, XAsync() could be called again and overwrite m_importantMemberVariable.

Code example:

task<void> BobViewModel::XAsync()
{
    return create_task(CreateSomethingAsync())
        .then([this](SomethingAsync^ aThing)
    {
        this->m_importantMemberVariable = aThing;
        OnPropertyChanged("ImportantMemberVariable");

        // Timer has 1 second delay.
        this->m_myDispatcherTimer->Start();
    }, task_continuation_context::use_current())
        .then([activity](task<void> result)
    {
        // more continuations...
    });
}

void BobViewModel::OnTimerTick(Object^, Object^)
{
    // Stopping the timer and detaching the event handler
    // so timer only fires once.
    m_myDispatcherTimer->Stop();
    m_myDispatcherTimer->Tick -= m_impressionTimerToken;
    m_myDispatcherTimer = { 0 };

    // * Possible race condition *
    m_importantMemberVariable->DoImportantThing();
}

Question: Assuming I'm correct about a race condition, is there a way to mitigate it?

My understanding is the tick event would fire on the UI thread so synchronization primitives won't help (as the UI thread would already have access).

Craig
  • 1,890
  • 1
  • 26
  • 44
  • There is no race. The only thing that can go wrong is that DoImportantThing() will be called less than 1 second after updating m_importantMemberVariable. The code forgets to Stop() the timer before calling Start(). – Hans Passant Apr 24 '18 at 00:24
  • @HansPassant - If a second caller to XAsync made m_importantMemberVariable null after the timer was started (but before the tick event fired), how would that not produce a null reference exception? Are you saying that can’t happen? If so, why? – Craig Apr 24 '18 at 01:10
  • You stopped the timer, if you followed by recommendation. Why would you call Start() again when the variable is null? – Hans Passant Apr 24 '18 at 02:11
  • This seems like a dupe of the question you already asked twice and I answered. – Peter Torr - MSFT Apr 24 '18 at 02:19
  • @PeterTorr-MSFT - The difference is this question throws the DispatcherTimer into the mix, so it's not limited to task continuations. – Craig Apr 25 '18 at 16:03
  • Then you don't even need the atomic bool since everything is on the same thread. Just set a flag in XAsync and clear it in the timer tick. XAsync bails out early if the flag is set. – Peter Torr - MSFT Apr 26 '18 at 01:24

2 Answers2

1

All your operations are on the UI thread, so they've already been serialized (synchronized) for you. A simple flag will suffice:

bool m_busy; // set to false in constructor

task<void> BobViewModel::XAsync()
{
    if (m_busy)
      return;

    m_busy = true;
    // the rest of your code...
}

void BobViewModel::OnTimerTick(Object^, Object^)
{
    m_busy = false;
    // the rest of your code...
}

Just make sure you handle any exceptions such that you set m_busy back to false if something goes horribly wrong.

Peter Torr - MSFT
  • 11,824
  • 3
  • 18
  • 51
0

The answer to this question suggests using compare_exchange_strong with a std::atomic to ensure only one thread executes a function at a time. The problems with that approach, for this question, are:
1. The DispatcherTimer Tick event fires outside of the task continuation block, and can fire after the continuation completes.
2. A constraint on this problem is for the timer to only fire once.

Some alternative solutions are:

  1. Use compare_exchange_strong but replace DispatcherTimer with create_delayed_task

Assuming the work doesn't have to happen on the UI thread, you can use create_delayed_task to delay work within a task continuation.

task<void>
BobViewModel::UseImportantVariableAsync(
    Object^ importantVariable
)
{
    return create_delayed_task(
        std::chrono::milliseconds(1000),
        [importantVariable]()
    {
        importantMemberVariable->DoImportantThing();
    });
}

Then, from the task continuation, simply:

return UseImportantVariableAsync(m_importantMemberVariable);
  1. Use a lambda for the DispatcherTimer's Tick event and capture 'aThing' from the question's example (instead of referencing the member variable in the handler). To only fire the timer once, assign the DispathcerTimer.Tick handler within a std::call_once block so only the first caller gets to do it.
Craig
  • 1,890
  • 1
  • 26
  • 44