Sharing the result of a single Windows Runtime IAsyncOperation among multiple coroutines, part 1 - The Old New Thing

Skip to main content

Search<br>Search

No results

Cancel

Raymond Chen

Suppose you have a coroutine method called GetThingAsync(), and you want to do the work of getting "something" only once and caching the result. Here’s the version that does no caching:

struct Widget : WidgetT<br>IAsyncOperation GetThingAsync()<br>co_return co_await GetThingWorkerAsync();

// The business logic goes here<br>IAsyncOperation GetThingWorkerAsync();<br>};

Now, if this code were written in C#, we could take advantage of the fact that C# projects the Windows Runtime IAsyncOperation as a Task, and Task objects support being awaited on multiple times.

async Task GetThingAsync()<br>lock (m_lock) {<br>// First person to call GetThingAsync starts the task.<br>if (m_task == null) {<br>m_task = GetThingWorker();<br>return await m_getThingTask;

But we don’t have that in C++/WinRT.

One customer tried to build a solution out of winrt::resume_on_signal, perhaps inspired by one of my earlier explorations of this topic.

// Don't use this code. See discussion.<br>struct Widget : WidgetT<br>winrt::Thing m_thing{ nullptr };<br>winrt::IAsyncOperation m_task{ nullptr };<br>wil::unique_event m_finished{ wil::EventOptions::ManualReset }; /* initially unsignaled */<br>bool m_busy{ false };<br>std::mutex m_mutex;

IAsyncOperation GetThingAsync()<br>bool shouldStart;<br>std::lock_guard guard(m_mutex);<br>if (m_thing != nullptr) {<br>// Operation has finished.<br>co_return m_thing;<br>} else if (m_busy) {<br>// Operation has started but not yet finished.<br>shouldStart = false;<br>} else {<br>// Operation hasn't even started.<br>m_busy = true;<br>shouldStart = true;

auto lifetime = get_strong();

if (shouldStart) {<br>auto task = GetThingWorker();<br>m_finished.ResetEvent();<br>task.Completed([weak = get_weak(), this](auto&&, auto&&) {<br>if (auto strong = weak.get()) {<br>m_busy = false;<br>m_finished.SetEvent();<br>});<br>m_task = std::move(task);

co_await winrt::resume_on_signal(m_finished.get());

std::lock_guard guard(m_mutex);<br>if (m_thing == nullptr && m_task) {<br>m_thing = m_task.GetResults();

co_return m_thing;

};

The idea is that the first time Get­Thing­Async() is called, we start the real task and then arrange to clear the busy flag and set the m_finished event when the task completes. Subsequent callers will see that the task has already started and will not start it again. And subsequent calls which occur after the task has completed will see that we already have a m_thing and return it immediately.

Everybody then waits for the m_finished event, and then whoever manages to enter the mutex first gets the result and saves it. Finally, everybody returns whatever is in m_thing, which should be the result of the task.

From the observation that they set m_busy back to false when the task completes, and they reset the m_finished event each time they start the task, I conclude that their intention was to allow multiple attempts to get the "something" if a previous attempt fails.

Okay, so let’s see what could go wrong.

For one thing, we see a data race because the completion lambda modifies m_busy outside the mutex. So we should at least protect that with a mutex.

Another problem is that this code is not exception-safe. If Get­Thing­Worker­Async throws an exception before returning an IAsync­Operation, then the m_busy flag is set and gets stuck there. This means that nobody else will try to start the task, and the m_task remains null, so all subsequent callers just fall through and return a null Thing, which may not be something that the callers are expecting. (I mean, this code certainly doesn’t handle the case where Get­Thing­Worker­Async produces a null result because it thinks that a null m_thing means that we should try again instead of "I successfully got nothing.")

There’s also a race condition if the task completes just as somebody calls Get­Thing­Async:

Thread 1<br>Thread 2

GetThingAsync called

m_busy = true

shouldStart = true

GetThingWorkerAsync()

m_finished.ResetEvent()

task.Completed(...)

m_task = std::move(task)

co_await resume_on_signal

(task completes)

m_busy = false

GetThing called

m_busy = true

shouldStart = true

GetThingWorkerAsync

m_finished.ResetEvent()

task.Completed(...)

m_task = std::move(task)

co_await resume_on_signal

m_finished.SetEvent()

(completion handler returns)

m_thing = m_task.GetResults()

Notice that the second call to Get­Thing­Async happens after m_busy has been reset, but before we have signaled the event. This creates a window inside which another thread calls Get­Thing­Async and tries to start the task again. The second calls assignment to m_task overwrites the one from the first call, and then when the first caller tries to get the results, it gets them from the wrong task.

But really, this code is trying too hard. We’ll look at a simpler version next time.

Category<br>Old New Thing

Topics<br>Code

Share

Author

Raymond Chen

Raymond has been involved in the...