Skip to content

Commit

Permalink
Fix crash when an already-done TFuture is co_awaited
Browse files Browse the repository at this point in the history
  • Loading branch information
landelare committed Mar 12, 2023
1 parent d4eb86e commit e19b394
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 11 deletions.
2 changes: 0 additions & 2 deletions Source/UE5Coro/Private/Promise.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ FPromise::~FPromise()
void FPromise::Resume()
{
#if UE5CORO_DEBUG
checkf(ResumeStack.Num() == 0 || ResumeStack.Last() != this,
TEXT("Internal error"));
checkf(!Extras->IsComplete(),
TEXT("Attempting to resume completed coroutine"));
ResumeStack.Push(this);
Expand Down
33 changes: 25 additions & 8 deletions Source/UE5Coro/Public/UE5Coro/AsyncAwaiters.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,30 +104,33 @@ class [[nodiscard]] TFutureAwaiter final : public TAwaiter<TFutureAwaiter<T>>

bool await_ready()
{
checkf(this->Future.IsValid(),
checkf(!Result, TEXT("Attempting to reuse spent TFutureAwaiter"));
checkf(Future.IsValid(),
TEXT("Awaiting invalid/spent future will never resume"));
return Future.IsReady();
}

void Suspend(FPromise& Promise)
{
checkf(!Result, TEXT("Attempting to reuse spent TFutureAwaiter"));
// Extremely rarely, Then will run synchronously because Future
// finished after IsReady but before Suspend.
// This is OK and will result in the caller coroutine resuming itself.

Future.Then([this, &Promise](auto InFuture)
{
checkf(!Future.IsValid(), TEXT("Internal error"));

// TFuture<T&> will pass T* for Value, TFuture<void> an int
if constexpr (std::is_lvalue_reference_v<T>)
{
static_assert(std::is_pointer_v<decltype(InFuture.Get())>);
checkf(!Future.IsValid(), TEXT("Internal error"));
Result = InFuture.Get();
Promise.Resume();
}
else
{
// It's normally dangerous to expose a pointer to a local, but
auto Value = InFuture.Get(); // This will be alive while...
checkf(!Future.IsValid(), TEXT("Internal error"));
Result = &Value;
Promise.Resume(); // ...await_resume moves from it here
}
Expand All @@ -136,10 +139,24 @@ class [[nodiscard]] TFutureAwaiter final : public TAwaiter<TFutureAwaiter<T>>

T await_resume()
{
if constexpr (std::is_lvalue_reference_v<T>)
return *Result;
else if constexpr (!std::is_void_v<T>)
return std::move(*Result);
if (!Result)
{
// Result being nullptr indicates that await_ready returned true,
// Then has not and will not run, and Future is still valid
checkf(Future.IsValid(), TEXT("Internal error"));
static_assert(std::is_same_v<T, decltype(Future.Get())>);
Result = reinterpret_cast<decltype(Result)>(-1); // Mark as spent
return Future.Get();
}
else
{
// Otherwise, we're being called from Then, and Future is spent
checkf(!Future.IsValid(), TEXT("Internal error"));
if constexpr (std::is_lvalue_reference_v<T>)
return *Result;
else if constexpr (!std::is_void_v<T>)
return std::move(*Result); // This will move from Then's local
}
}
};

Expand Down
17 changes: 17 additions & 0 deletions Source/UE5CoroTests/Private/FutureTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,23 @@ void DoTest(FAutomationTestBase& Test)
{
FTestWorld World;

{
TPromise<int> Promise;
Promise.SetValue(1);
auto Coro = World.Run(CORO_R(int)
{
co_return co_await Promise.GetFuture();
});

IF_CORO_LATENT
{
Test.TestFalse(TEXT("Not polled yet"), Coro.IsDone());
World.Tick();
}
Test.TestTrue(TEXT("Already done"), Coro.IsDone());
Test.TestEqual(TEXT("Value"), Coro.GetResult(), 1);
}

{
int State = 0;
TPromise<void> Promise;
Expand Down
2 changes: 1 addition & 1 deletion UE5Coro.uplugin
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"FileVersion": 3,
"Version": 1,
"VersionName": "1.6.2",
"VersionName": "1.7",
"FriendlyName": "UE5Coro",
"Description": "C++17/20 coroutine implementation for Unreal Engine",
"Category": "Programming",
Expand Down

0 comments on commit e19b394

Please sign in to comment.