From 0576251eee13e34aa5c438948f5cab7b3869874e Mon Sep 17 00:00:00 2001 From: Raymond Chen Date: Tue, 14 Jul 2020 08:08:25 -0700 Subject: [PATCH] Don't free the apartment_context while we are still using it The apartment-switching function `resume_apartment_sync` takes a context and a coroutine handle and resumes the coroutine in that context. The context is taken by reference, which is a problem if the coroutine resumption is going to destruct the context: We destruct the context while it is call to `ContextCallback` is still active. The awaiter for `IAsync*` captures the context into the completion handler, which is a separate object from the coroutine, so resuming the coroutine will not destruct the context. However, the awaiter for `apartment_context` uses the context stored directly inside the `apartment_context` itself, which could be destructed as part of resumption: ```cpp fire_and_forget Something() { // Delay 1 second and return to original context. { apartment_context save; co_await resume_after(1s); co_await save; } // do more stuff } ``` The first thing the coroutine does on resumption from `co_await save` is destruct the `apartment_context`, which releases the `context` while `ContextCallback` is still active. Fix by copying the context and resuming the copy. That way, the context isn't destructed until after `resume_context_sync` has definitely returned from `ContextCallback`. We can't `std::move` the context out of the `apartment_context` because that would prevent an apartment context from being `co_await`ed more than once. --- strings/base_coroutine_threadpool.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/strings/base_coroutine_threadpool.h b/strings/base_coroutine_threadpool.h index acb03624f..2409aecf6 100644 --- a/strings/base_coroutine_threadpool.h +++ b/strings/base_coroutine_threadpool.h @@ -336,7 +336,8 @@ WINRT_EXPORT namespace winrt void await_suspend(std::experimental::coroutine_handle<> handle) const { - impl::resume_apartment(context, handle); + auto copy = context; // resuming may destruct *this, so use a copy + impl::resume_apartment(copy, handle); } impl::resume_apartment_context context;