Use after free when accessing captures in a dispatched fire_and_forget lambda

[sylveon]

When dispatching a fire_and_forget lambda through a DispatcherQueue, we get a use-after-free:

#include <DispatcherQueue.h>

#define __cpp_coroutines // temporary workaround to get coroutines working in C++20 mode
#include <winrt/base.h>
#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.System.h>

#include <chrono>
#include <iostream>
#include <memory>

winrt::Windows::System::DispatcherQueueController CreateDispatcherController()
{
	const DispatcherQueueOptions options = {
		.dwSize = sizeof(options),
		.threadType = DQTYPE_THREAD_CURRENT,
		.apartmentType = DQTAT_COM_STA
	};

	winrt::com_ptr<ABI::Windows::System::IDispatcherQueueController> controller;
	winrt::check_hresult(CreateDispatcherQueueController(options, controller.put()));
	return { controller.detach(), winrt::take_ownership_from_abi };
}

int main()
{
	auto controller = CreateDispatcherController();

	auto veryimportant = std::make_unique<int>(1337);
	controller.DispatcherQueue().TryEnqueue([stillveryimportant = std::move(veryimportant)]() -> winrt::fire_and_forget
	{
		using namespace std::chrono_literals;

		co_await 1s;

		// use-after-free here
		std::cout << *stillveryimportant;
	});

	MSG msg;
	while (GetMessage(&msg, nullptr, 0, 0))
	{
		TranslateMessage(&msg);
		DispatchMessage(&msg);
	}
}

The lambda internal this is freed on the first coroutine suspension point, meaning accessing captured variables after this point will lead to undefined behavior and crashes.

[DefaultRyan]

This is not a bug in C++/WinRT, but is a consequence of the way you are using the lambda in conjunction with coroutines. The temporary instance of the lambda (along with its captures) is destroyed after the call to TryEnqueue returns, which is going to be at the first suspension point.

coroutines create a frame that holds the lifetime of the function's parameters and local variables and keeps them alive across suspension points. A lambda is not a coroutine - it is a C++ object, with an overloaded operator() member function. So the lifetime extension afforded to a coroutine's parameters and locals is not afforded to the lambda object itself. And because a lambda's captures are simply member variables of the object, they also get destroyed.

See these links for more discussion:

• https://stackoverflow.com/questions/60592174/lambda-lifetime-explanation-for-c20-coroutines/60604095#60604095
• https://devblogs.microsoft.com/oldnewthing/20190117-00/?p=100725

You can either:

• Explicitly extend the lifetime of your captures.

controller.DispatcherQueue().TryEnqueue([&veryimportant]() -> winrt::fire_and_forget
  {
    auto stillveryimportant = std::move(veryimportant);
    co_await 1s;
    // Do stuff
  });

• Ensure the lambda stays alive by making it a local variable instead of a temporary (but this might not play nice with your move-only capture if the callee attempts to copy the lambda).

auto f = [stillveryimportant = std::move(veryimportant)]() -> fire_and_forget {
  // Do stuff
};
controller.DispatcherQueue().TryEnqueue(f);

The first option is cleaner and more robust. This, of course, relies on the fact that the C++/WinRT coroutine adapters will eagerly run the coroutine upto the first suspension point, giving the coroutine an opportunity to extend some lifetimes before it suspends (as opposed to immediately suspending like a lazily evaluated "generator").

[sylveon]

I guess this is what people mean when they talk about C++ and footguns... thanks