From 36cd0ef1195d27e3a0e73c82edfb39207af227c2 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 9 Apr 2026 21:25:34 -0700 Subject: [PATCH 01/19] start implementing promises --- .../include/launchdarkly/async/promise.hpp | 169 ++++++++++++++++++ libs/internal/src/CMakeLists.txt | 1 + libs/internal/tests/promise_test.cpp | 49 +++++ 3 files changed, 219 insertions(+) create mode 100644 libs/internal/include/launchdarkly/async/promise.hpp create mode 100644 libs/internal/tests/promise_test.cpp diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp new file mode 100644 index 000000000..c4bef7e14 --- /dev/null +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -0,0 +1,169 @@ +#pragma once + +#include +#include +#include +#include +#include + +namespace launchdarkly::async { + +// Continuation is a move-only type-erased callable, effectively a polyfill +// for C++23's std::move_only_function. It exists because C++17's std::function +// requires all captured variables to be copy-constructible, which prevents +// storing lambdas that capture move-only types. +template +class Continuation; + +template +class Continuation { + struct Base { + virtual R call(Args...) = 0; + virtual ~Base() = default; + }; + + template + struct Impl : Base { + F f; + Impl(F&& f) : f(std::move(f)) {} + R call(Args... args) override { return f(std::forward(args)...); } + }; + + std::unique_ptr impl_; + + public: + template + Continuation(F&& f) + : impl_(std::make_unique>(std::forward(f))) {} + Continuation(Continuation&&) = default; + Continuation& operator=(Continuation&&) = default; + + R operator()(Args... args) { + return impl_->call(std::forward(args)...); + } +}; + +template +class Promise; + +template +class Future; + +// TODO: Document why things are what they are. +// TODO: Is it okay that the shared_ptr lets copies mutate the result?? + +template +class PromiseInternal { + public: + PromiseInternal() = default; + ~PromiseInternal() = default; + PromiseInternal(PromiseInternal const&) = delete; + PromiseInternal(PromiseInternal&&) = delete; + + bool Resolve(T result) { + std::lock_guard lock(mutex_); + if (result_->has_value()) { + return false; + } + + *result_ = result; + + for (auto& continuation : continuations_) { + continuation(result_); + } + continuations_.clear(); + + return true; + } + + bool IsFinished() const { + std::lock_guard lock(mutex_); + return result_->has_value(); + } + + T const& GetResult() const { + std::lock_guard lock(mutex_); + return **result_; + } + + template > + Future Then(F&& continuation, + std::function)> executor) { + Promise newPromise; + std::lock_guard lock(mutex_); + + Future newFuture = newPromise.GetFuture(); + + if (result_->has_value()) { + executor(Continuation( + [newPromise = std::move(newPromise), + continuation = std::move(continuation), + result = result_]() mutable { + newPromise.Resolve(continuation(**result)); + })); + return newFuture; + } + + continuations_.push_back( + [newPromise = std::move(newPromise), + continuation = std::move(continuation), + executor](std::shared_ptr> result) mutable { + executor(Continuation( + [newPromise = std::move(newPromise), + continuation = std::move(continuation), result]() mutable { + newPromise.Resolve(continuation(**result)); + })); + }); + + return newFuture; + } + + private: + std::mutex mutex_; + std::shared_ptr> result_{ + new std::optional(std::nullopt)}; + std::vector>)>> + continuations_; +}; + +template +class Promise { + public: + Promise() : internal_(new PromiseInternal()) {} + ~Promise() = default; + Promise(Promise const&) = delete; + Promise(Promise&&) = default; + + bool Resolve(T result) { return internal_->Resolve(result); } + + Future GetFuture() { return Future(internal_); } + + private: + std::shared_ptr> internal_; +}; + +template +class Future { + public: + Future(std::shared_ptr> internal) + : internal_(internal) {} + ~Future() = default; + Future(Future const&) = default; + Future(Future&&) = default; + + bool IsFinished() const { return internal_->IsFinished(); } + + T const& GetResult() const { return internal_->GetResult(); } + + template > + Future Then(F&& continuation, + std::function)> executor) { + return internal_->Then(std::forward(continuation), + std::move(executor)); + } + + private: + std::shared_ptr> internal_; +}; + +} // namespace launchdarkly::async diff --git a/libs/internal/src/CMakeLists.txt b/libs/internal/src/CMakeLists.txt index 44600638b..2fffa42b3 100644 --- a/libs/internal/src/CMakeLists.txt +++ b/libs/internal/src/CMakeLists.txt @@ -1,6 +1,7 @@ file(GLOB HEADER_LIST CONFIGURE_DEPENDS "${LaunchDarklyInternalSdk_SOURCE_DIR}/include/launchdarkly/*.hpp" + "${LaunchDarklyInternalSdk_SOURCE_DIR}/include/launchdarkly/async/*.hpp" "${LaunchDarklyInternalSdk_SOURCE_DIR}/include/launchdarkly/events/*.hpp" "${LaunchDarklyInternalSdk_SOURCE_DIR}/include/launchdarkly/network/*.hpp" "${LaunchDarklyInternalSdk_SOURCE_DIR}/include/launchdarkly/serialization/*.hpp" diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp new file mode 100644 index 000000000..248a65470 --- /dev/null +++ b/libs/internal/tests/promise_test.cpp @@ -0,0 +1,49 @@ +#include +#include +#include + +#include + +#include "launchdarkly/async/promise.hpp" + +using namespace launchdarkly::async; + +// Cases to cover: +// Resolved after continuation. +// Resolved before continuation. +// Continue with value. +// Continue with promise. +// A result that can be copied but not moved. +// A result that can be moved but not copied. +// A callback that can be copied but not moved. +// A callback that can be moved but not copied. +// Test that results are moved if possible, even if they could be copied. +// Test that callbacks are moved if possible, even if they could be copied. + +TEST(Promise, SimplePromise) { + boost::asio::io_context ioc; + auto work = boost::asio::make_work_guard(ioc); + std::thread ioc_thread([&]() { ioc.run(); }); + + int result = 0; + + Promise promise; + Future future = promise.GetFuture(); + + Future future2 = future.Then( + [&](int const& inner) { + result = 42; + return static_cast(result); + }, + [](Continuation f) { f(); }); + + promise.Resolve(43); + + bool work_ran = false; + boost::asio::post(ioc, [&]() { work_ran = true; }); + + work.reset(); + ioc_thread.join(); + + ASSERT_TRUE(work_ran); +} From 82a9d84faf08da7d29cd33c3964a0dcf702ec0e2 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 9 Apr 2026 21:48:54 -0700 Subject: [PATCH 02/19] add a flattening version of Then --- .../include/launchdarkly/async/promise.hpp | 116 ++++++++++++++++-- 1 file changed, 109 insertions(+), 7 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index c4bef7e14..8cfe269af 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -12,11 +12,18 @@ namespace launchdarkly::async { // for C++23's std::move_only_function. It exists because C++17's std::function // requires all captured variables to be copy-constructible, which prevents // storing lambdas that capture move-only types. +// +// The primary template is declared but not defined; only the partial +// specialization below (which splits Sig into R and Args...) is usable. +// This lets callers write Continuation instead of Continuation. template class Continuation; template class Continuation { + // Base and Impl form a classic type-erasure pair. Base is a non-template + // abstract interface stored via unique_ptr, giving a stable type regardless + // of F. Impl is the concrete template subclass that holds and calls F. struct Base { virtual R call(Args...) = 0; virtual ~Base() = default; @@ -32,6 +39,8 @@ class Continuation { std::unique_ptr impl_; public: + // F&& is a forwarding reference: accepts any callable by move or copy, + // then moves it into Impl so Continuation itself owns the callable. template Continuation(F&& f) : impl_(std::make_unique>(std::forward(f))) {} @@ -49,6 +58,24 @@ class Promise; template class Future; +// Type trait to detect whether a type is a Future. The primary template +// defaults to false; the partial specialization matches Future specifically. +template +struct is_future : std::false_type {}; + +template +struct is_future> : std::true_type {}; + +// Type trait to extract T from Future. Only the partial specialization is +// defined, so using future_value on a non-Future type is a compile error. +template +struct future_value; + +template +struct future_value> { + using type = T; +}; + // TODO: Document why things are what they are. // TODO: Is it okay that the shared_ptr lets copies mutate the result?? @@ -86,13 +113,18 @@ class PromiseInternal { return **result_; } - template > - Future Then(F&& continuation, - std::function)> executor) { - Promise newPromise; + // Then where the continuation returns R directly, yielding Future. + template , + // Disable when R is a Future so the flattening overload wins instead. + typename = std::enable_if_t::value>> + Future Then(F&& continuation, + std::function)> executor) { + Promise newPromise; std::lock_guard lock(mutex_); - Future newFuture = newPromise.GetFuture(); + Future newFuture = newPromise.GetFuture(); if (result_->has_value()) { executor(Continuation( @@ -110,7 +142,8 @@ class PromiseInternal { executor](std::shared_ptr> result) mutable { executor(Continuation( [newPromise = std::move(newPromise), - continuation = std::move(continuation), result]() mutable { + continuation = std::move(continuation), + result]() mutable { newPromise.Resolve(continuation(**result)); })); }); @@ -118,6 +151,55 @@ class PromiseInternal { return newFuture; } + // Then where the continuation returns Future (i.e. R = Future), + // yielding a flattened Future that resolves when the inner future does. + template , + // Unwrap Future -> T2 so the return type is Future, not Future>. + typename T2 = typename future_value::type, + // Only enabled when R is a Future; otherwise the direct overload wins. + typename = std::enable_if_t::value>> + Future Then(F&& continuation, + std::function)> executor) { + Promise outerPromise; + std::lock_guard lock(mutex_); + + Future outerFuture = outerPromise.GetFuture(); + + auto do_work = [outerPromise = std::move(outerPromise), + continuation = std::move(continuation), + executor](T const& val) mutable { + Future innerFuture = continuation(val); + innerFuture.Then( + [outerPromise = std::move(outerPromise)]( + T2 const& inner_val) mutable -> T2 { + outerPromise.Resolve(inner_val); + return inner_val; + }, + executor); + }; + + if (result_->has_value()) { + executor(Continuation( + [do_work = std::move(do_work), result = result_]() mutable { + do_work(**result); + })); + return outerFuture; + } + + continuations_.push_back( + [do_work = std::move(do_work), + executor](std::shared_ptr> result) mutable { + executor(Continuation( + [do_work = std::move(do_work), result]() mutable { + do_work(**result); + })); + }); + + return outerFuture; + } + private: std::mutex mutex_; std::shared_ptr> result_{ @@ -155,7 +237,27 @@ class Future { T const& GetResult() const { return internal_->GetResult(); } - template > + // Then where the continuation returns R directly, yielding Future. + template , + // Disable when R is a Future so the flattening overload wins instead. + typename = std::enable_if_t::value>> + Future Then(F&& continuation, + std::function)> executor) { + return internal_->Then(std::forward(continuation), + std::move(executor)); + } + + // Then where the continuation returns Future (i.e. R = Future), + // yielding a flattened Future that resolves when the inner future does. + template , + // Unwrap Future -> T2 so the return type is Future, not Future>. + typename T2 = typename future_value::type, + // Only enabled when R is a Future; otherwise the direct overload wins. + typename = std::enable_if_t::value>> Future Then(F&& continuation, std::function)> executor) { return internal_->Then(std::forward(continuation), From 0a07a9a15a66f7f7c45e5ec58054e736ff4129c7 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 9 Apr 2026 22:11:40 -0700 Subject: [PATCH 03/19] add comments --- .../include/launchdarkly/async/promise.hpp | 111 +++++++++++++++++- 1 file changed, 106 insertions(+), 5 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index 8cfe269af..328d62ff7 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -13,6 +13,14 @@ namespace launchdarkly::async { // requires all captured variables to be copy-constructible, which prevents // storing lambdas that capture move-only types. // +// Continuation is used to represent units of work passed to executors. An +// executor is a callable with signature void(Continuation) that +// schedules the work to run somewhere — for example, on an ASIO io_context: +// +// auto executor = [&ioc](Continuation work) { +// boost::asio::post(ioc, std::move(work)); +// }; +// // The primary template is declared but not defined; only the partial // specialization below (which splits Sig into R and Args...) is usable. // This lets callers write Continuation instead of Continuation. @@ -39,6 +47,8 @@ class Continuation { std::unique_ptr impl_; public: + // Constructs a Continuation from any callable F. F may be a lambda, + // function pointer, or other callable; it need not be copy-constructible. // F&& is a forwarding reference: accepts any callable by move or copy, // then moves it into Impl so Continuation itself owns the callable. template @@ -47,6 +57,8 @@ class Continuation { Continuation(Continuation&&) = default; Continuation& operator=(Continuation&&) = default; + // Invokes the stored callable with the given arguments. + // Returns whatever the callable returns. R operator()(Args... args) { return impl_->call(std::forward(args)...); } @@ -76,9 +88,17 @@ struct future_value> { using type = T; }; -// TODO: Document why things are what they are. +// PromiseInternal holds the shared state between a Promise and its associated +// Futures: the result value, the mutex protecting it, and the list of +// continuations waiting to run when the result is set. +// +// This is an internal class and not intended to be used directly. Promise and +// Future each hold a shared_ptr, which lets multiple Future +// copies all refer to the same underlying state, and lets Promise and Future +// have independent lifetimes while the shared state remains alive as long as +// either end holds it. +// // TODO: Is it okay that the shared_ptr lets copies mutate the result?? - template class PromiseInternal { public: @@ -87,6 +107,9 @@ class PromiseInternal { PromiseInternal(PromiseInternal const&) = delete; PromiseInternal(PromiseInternal&&) = delete; + // Sets the result and schedules all registered continuations via their + // executors. Returns true if the result was set, or false if it was already + // set by a previous call to Resolve. bool Resolve(T result) { std::lock_guard lock(mutex_); if (result_->has_value()) { @@ -103,11 +126,14 @@ class PromiseInternal { return true; } + // Returns true if Resolve has been called. bool IsFinished() const { std::lock_guard lock(mutex_); return result_->has_value(); } + // Returns a reference to the result. The caller must ensure IsFinished() + // is true before calling this. T const& GetResult() const { std::lock_guard lock(mutex_); return **result_; @@ -208,6 +234,21 @@ class PromiseInternal { continuations_; }; +// Promise is the write end of a one-shot async value, similar to std::promise. +// Create a Promise, hand its Future to a consumer via GetFuture(), then +// call Resolve() exactly once to deliver the value. +// +// Promise is move-only: it cannot be copied, but it can be moved. This +// prevents accidentally resolving the same promise from two places. +// +// Using std::expected as T is the recommended way to represent +// operations that may fail: +// +// Promise> promise; +// Future> future = promise.GetFuture(); +// // ... hand future to a consumer, then later: +// promise.Resolve(42); // success +// promise.Resolve(std::unexpected("timed out")); // failure template class Promise { public: @@ -216,14 +257,44 @@ class Promise { Promise(Promise const&) = delete; Promise(Promise&&) = default; + // Sets the result to the given value and schedules any continuations that + // were registered via Future::Then. Returns true if the result was set, or + // false if Resolve was already called. bool Resolve(T result) { return internal_->Resolve(result); } + // Returns a Future that will resolve when this Promise is resolved. + // May be called multiple times; each call returns a Future referring to + // the same underlying state. Future GetFuture() { return Future(internal_); } private: std::shared_ptr> internal_; }; +// Future is the read end of a one-shot async value, similar to std::future, +// but with support for chaining via Then. +// +// A Future is obtained from Promise::GetFuture(). Multiple copies of a Future +// may exist and all refer to the same underlying result. When the associated +// Promise is resolved, all continuations registered via Then are scheduled. +// +// Unlike std::future, Future does not support blocking on the result directly. +// Instead, use Then to attach work that runs once the value is available. +// +// Example using std::expected to represent a fallible async operation: +// +// boost::asio::io_context ioc; +// auto executor = [&ioc](Continuation work) { +// boost::asio::post(ioc, std::move(work)); +// }; +// +// Future> result = future.Then( +// [](std::expected const& val) +// -> std::expected { +// if (!val) return std::unexpected(val.error()); +// return *val * 1.5f; +// }, +// executor); template class Future { public: @@ -233,11 +304,24 @@ class Future { Future(Future const&) = default; Future(Future&&) = default; + // Returns true if the associated Promise has been resolved. bool IsFinished() const { return internal_->IsFinished(); } + // Returns a reference to the result. The caller must ensure IsFinished() + // is true before calling this. The reference is valid for the lifetime of + // the Future (or any copy of it). T const& GetResult() const { return internal_->GetResult(); } - // Then where the continuation returns R directly, yielding Future. + // Registers a continuation to run when this Future resolves, returning a + // new Future that resolves to the continuation's return value. + // + // Parameters: + // continuation - Called with the resolved T const& when this Future + // resolves. Must return a value of type R (not a Future). + // executor - Called with the work to schedule when this Future + // resolves. Controls where and when the continuation runs. + // + // Returns a Future that resolves to whatever the continuation returns. template , @@ -249,8 +333,25 @@ class Future { std::move(executor)); } - // Then where the continuation returns Future (i.e. R = Future), - // yielding a flattened Future that resolves when the inner future does. + // Registers a continuation to run when this Future resolves, where the + // continuation itself returns a Future. Returns a flattened Future + // that resolves when the inner future does, avoiding Future>. + // Use this overload to chain async operations that themselves return a + // Future, avoiding a nested Future>: + // + // Future> result = future.Then( + // [](std::expected const& key) { + // return fetch(key); // fetch returns Future> + // }, + // executor); + // + // Parameters: + // continuation - Called with the resolved T const& when this Future + // resolves. Must return a Future. + // executor - Called with the work to schedule when this Future + // resolves, and again when the inner Future resolves. + // + // Returns a Future that resolves when the inner Future resolves. template , From 66a9181a723c190f309c8c00722c83307c8e4d18 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 10 Apr 2026 12:35:09 -0700 Subject: [PATCH 04/19] add tests --- .../include/launchdarkly/async/promise.hpp | 93 ++++-- libs/internal/tests/promise_test.cpp | 296 ++++++++++++++++-- 2 files changed, 344 insertions(+), 45 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index 328d62ff7..66222ecd1 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -1,5 +1,7 @@ #pragma once +#include +#include #include #include #include @@ -116,7 +118,12 @@ class PromiseInternal { return false; } - *result_ = result; + // Move result into storage if possible; otherwise copy. + if constexpr (std::is_move_assignable_v) { + *result_ = std::move(result); + } else { + *result_ = result; + } for (auto& continuation : continuations_) { continuation(result_); @@ -132,11 +139,12 @@ class PromiseInternal { return result_->has_value(); } - // Returns a reference to the result. The caller must ensure IsFinished() - // is true before calling this. - T const& GetResult() const { + // Returns a const reference to the optional result. The optional contains + // a value if resolved, or is empty if not yet resolved. The reference is + // valid for the lifetime of any Future referring to this PromiseInternal. + std::optional const& GetResult() const { std::lock_guard lock(mutex_); - return **result_; + return *result_; } // Then where the continuation returns R directly, yielding Future. @@ -227,7 +235,7 @@ class PromiseInternal { } private: - std::mutex mutex_; + mutable std::mutex mutex_; std::shared_ptr> result_{ new std::optional(std::nullopt)}; std::vector>)>> @@ -241,14 +249,14 @@ class PromiseInternal { // Promise is move-only: it cannot be copied, but it can be moved. This // prevents accidentally resolving the same promise from two places. // -// Using std::expected as T is the recommended way to represent +// Using tl::expected as T is the recommended way to represent // operations that may fail: // -// Promise> promise; -// Future> future = promise.GetFuture(); +// Promise> promise; +// Future> future = promise.GetFuture(); // // ... hand future to a consumer, then later: // promise.Resolve(42); // success -// promise.Resolve(std::unexpected("timed out")); // failure +// promise.Resolve(tl::unexpected("timed out")); // failure template class Promise { public: @@ -260,7 +268,13 @@ class Promise { // Sets the result to the given value and schedules any continuations that // were registered via Future::Then. Returns true if the result was set, or // false if Resolve was already called. - bool Resolve(T result) { return internal_->Resolve(result); } + bool Resolve(T result) { + if constexpr (std::is_move_constructible_v) { + return internal_->Resolve(std::move(result)); + } else { + return internal_->Resolve(result); + } + } // Returns a Future that will resolve when this Promise is resolved. // May be called multiple times; each call returns a Future referring to @@ -281,17 +295,17 @@ class Promise { // Unlike std::future, Future does not support blocking on the result directly. // Instead, use Then to attach work that runs once the value is available. // -// Example using std::expected to represent a fallible async operation: +// Example using tl::expected to represent a fallible async operation: // // boost::asio::io_context ioc; // auto executor = [&ioc](Continuation work) { // boost::asio::post(ioc, std::move(work)); // }; // -// Future> result = future.Then( -// [](std::expected const& val) -// -> std::expected { -// if (!val) return std::unexpected(val.error()); +// Future> result = future.Then( +// [](tl::expected const& val) +// -> tl::expected { +// if (!val) return tl::unexpected(val.error()); // return *val * 1.5f; // }, // executor); @@ -307,10 +321,45 @@ class Future { // Returns true if the associated Promise has been resolved. bool IsFinished() const { return internal_->IsFinished(); } - // Returns a reference to the result. The caller must ensure IsFinished() - // is true before calling this. The reference is valid for the lifetime of - // the Future (or any copy of it). - T const& GetResult() const { return internal_->GetResult(); } + // Returns a const reference to the optional result. The optional contains + // a value if resolved, or is empty if not yet resolved. The reference is + // valid for the lifetime of this Future (or any copy of it). + std::optional const& GetResult() const { return internal_->GetResult(); } + + // Blocks the calling thread until the future resolves or the timeout + // expires. Returns a const reference to the optional result: the optional + // contains a value if resolved within the timeout, or is empty if the + // timeout expired first. The reference is valid for the lifetime of this + // Future (or any copy of it). + template + std::optional const& WaitForResult( + std::chrono::duration timeout) { + struct State { + std::mutex mutex; + std::condition_variable cv; + bool ready = false; + }; + auto state = std::make_shared(); + + // The continuation ignores T entirely (returning a dummy int) so that + // T is not required to be copyable. The executor signals the cv when + // called, since being called means the original future has resolved. + Then( + [](T const&) { return 0; }, + [state](Continuation work) { + { + std::lock_guard lock(state->mutex); + state->ready = true; + } + state->cv.notify_one(); + work(); + }); + + std::unique_lock lock(state->mutex); + state->cv.wait_for(lock, timeout, [&state] { return state->ready; }); + + return GetResult(); + } // Registers a continuation to run when this Future resolves, returning a // new Future that resolves to the continuation's return value. @@ -339,8 +388,8 @@ class Future { // Use this overload to chain async operations that themselves return a // Future, avoiding a nested Future>: // - // Future> result = future.Then( - // [](std::expected const& key) { + // Future> result = future.Then( + // [](tl::expected const& key) { // return fetch(key); // fetch returns Future> // }, // executor); diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index 248a65470..5dbced30e 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include @@ -8,42 +9,291 @@ using namespace launchdarkly::async; -// Cases to cover: -// Resolved after continuation. -// Resolved before continuation. -// Continue with value. -// Continue with promise. -// A result that can be copied but not moved. -// A result that can be moved but not copied. -// A callback that can be copied but not moved. -// A callback that can be moved but not copied. -// Test that results are moved if possible, even if they could be copied. -// Test that callbacks are moved if possible, even if they could be copied. +TEST(Promise, GetResultNotFinished) { + Promise promise; + Future future = promise.GetFuture(); + + auto& result = future.GetResult(); + EXPECT_FALSE(result.has_value()); +} + +TEST(Promise, GetResultFinished) { + Promise promise; + Future future = promise.GetFuture(); + + promise.Resolve(42); + + auto& result = future.GetResult(); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(*result, 42); +} + +TEST(Promise, CopyOnlyResult) { + struct CopyOnly { + int value; + explicit CopyOnly(int v) : value(v) {} + CopyOnly(CopyOnly const&) = default; + CopyOnly& operator=(CopyOnly const&) = default; + CopyOnly(CopyOnly&&) = delete; + CopyOnly& operator=(CopyOnly&&) = delete; + }; + + Promise promise; + Future future = promise.GetFuture(); + + promise.Resolve(CopyOnly{42}); + + auto& result = future.GetResult(); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->value, 42); +} + +TEST(Promise, ContinueByReturningFuture) { + Promise promise1; + Promise promise2; + Future future2 = promise2.GetFuture(); + + Future chained = promise1.GetFuture().Then( + [future2](int const&) { return future2; }, + [](Continuation f) { f(); }); + + promise1.Resolve(0); + promise2.Resolve(42); + + auto& result = chained.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(*result, 42); +} + +TEST(Promise, ResolvedBeforeContinuation) { + Promise promise; + Future future = promise.GetFuture(); + + promise.Resolve(21); + + Future future2 = future.Then( + [](int const& val) { return val * 2; }, + [](Continuation f) { f(); }); + + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(*result, 42); +} + +TEST(Promise, ResolvedAfterContinuation) { + Promise promise; + Future future = promise.GetFuture(); + + Future future2 = future.Then( + [](int const& val) { return val * 2; }, + [](Continuation f) { f(); }); + + promise.Resolve(21); + + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(*result, 42); +} + +TEST(Promise, MoveOnlyResult) { + Promise> promise; + Future> future = promise.GetFuture(); + + promise.Resolve(std::make_unique(42)); + + auto& result = future.GetResult(); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(**result, 42); +} + +// Verifies that a continuation which captures a copy-only type (deleted move +// constructor) can still be registered and invoked correctly. +TEST(Promise, CopyOnlyCallback) { + struct CopyOnlyInt { + int value; + explicit CopyOnlyInt(int v) : value(v) {} + CopyOnlyInt(CopyOnlyInt const&) = default; + CopyOnlyInt& operator=(CopyOnlyInt const&) = default; + CopyOnlyInt(CopyOnlyInt&&) = delete; + CopyOnlyInt& operator=(CopyOnlyInt&&) = delete; + }; + + Promise promise; + Future future = promise.GetFuture(); + + CopyOnlyInt multiplier{2}; + Future future2 = future.Then( + [multiplier](int const& val) { return val * multiplier.value; }, + [](Continuation f) { f(); }); + + promise.Resolve(21); + + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(*result, 42); +} + +// Verifies that a continuation which captures a move-only type (deleted copy +// constructor) can still be registered and invoked correctly. +TEST(Promise, MoveOnlyCallback) { + Promise promise; + Future future = promise.GetFuture(); + + auto captured = std::make_unique(2); + Future future2 = future.Then( + [captured = std::move(captured)](int const& val) { + return val * *captured; + }, + [](Continuation f) { f(); }); + + promise.Resolve(21); + + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(*result, 42); +} + +// Verifies that when T is both moveable and copyable, resolving a promise +// moves the value into storage rather than copying it. +TEST(Promise, ResultMovedWhenPossible) { + int copies = 0; + struct Counted { + int value; + int* copy_count; + explicit Counted(int v, int* c) : value(v), copy_count(c) {} + Counted(Counted const& o) : value(o.value), copy_count(o.copy_count) { ++(*copy_count); } + Counted& operator=(Counted const& o) { value = o.value; copy_count = o.copy_count; ++(*copy_count); return *this; } + Counted(Counted&&) noexcept = default; + Counted& operator=(Counted&&) noexcept = default; + }; + + Promise promise; + Future future = promise.GetFuture(); + + promise.Resolve(Counted{42, &copies}); + + auto& result = future.GetResult(); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->value, 42); + EXPECT_EQ(copies, 0); +} + +// Verifies that when a continuation is both moveable and copyable, Then stores +// it by moving rather than copying. +TEST(Promise, CallbackMovedWhenPossible) { + int copies = 0; + struct Counted { + int value; + int* copy_count; + explicit Counted(int v, int* c) : value(v), copy_count(c) {} + Counted(Counted const& o) : value(o.value), copy_count(o.copy_count) { ++(*copy_count); } + Counted& operator=(Counted const& o) { value = o.value; copy_count = o.copy_count; ++(*copy_count); return *this; } + Counted(Counted&&) noexcept = default; + Counted& operator=(Counted&&) noexcept = default; + }; + + Promise promise; + Future future = promise.GetFuture(); + + Counted multiplier{2, &copies}; + copies = 0; // reset after construction + + future.Then( + [multiplier = std::move(multiplier)](int const& val) mutable { + return val * multiplier.value; + }, + [](Continuation f) { f(); }); + + EXPECT_EQ(copies, 0); + + promise.Resolve(21); +} + +// Demonstrates using std::monostate as a void-like result type for fire-and-forget +// async operations where the completion matters but no value is produced. +TEST(Promise, MonostateVoidLike) { + Promise promise; + Future future = promise.GetFuture(); + + bool ran = false; + future.Then( + [&ran](std::monostate const&) { + ran = true; + return std::monostate{}; + }, + [](Continuation f) { f(); }); + + promise.Resolve(std::monostate{}); + + auto& result = future.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_TRUE(ran); +} + +// Demonstrates using tl::expected as T to represent a fallible async operation +// that resolves successfully. +TEST(Promise, ExpectedSuccess) { + Promise> promise; + Future> future = promise.GetFuture(); + + promise.Resolve(42); + + auto& result = future.GetResult(); + ASSERT_TRUE(result.has_value()); + ASSERT_TRUE(result->has_value()); + EXPECT_EQ(**result, 42); +} + +// Demonstrates using tl::expected as T to represent a fallible async operation +// that resolves with an error. +TEST(Promise, ExpectedFailure) { + Promise> promise; + Future> future = promise.GetFuture(); + + promise.Resolve(tl::unexpected(std::string("timed out"))); + + auto& result = future.GetResult(); + ASSERT_TRUE(result.has_value()); + ASSERT_FALSE(result->has_value()); + EXPECT_EQ(result->error(), "timed out"); +} + TEST(Promise, SimplePromise) { + Promise promise; + Future future = promise.GetFuture(); + + Future future2 = future.Then( + [](int const& inner) { return static_cast(inner * 2.0); }, + [](Continuation f) { f(); }); + + promise.Resolve(43); + + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_FLOAT_EQ(*result, 86.0f); +} + +TEST(Promise, ASIOTest) { boost::asio::io_context ioc; auto work = boost::asio::make_work_guard(ioc); std::thread ioc_thread([&]() { ioc.run(); }); - int result = 0; - Promise promise; Future future = promise.GetFuture(); Future future2 = future.Then( - [&](int const& inner) { - result = 42; - return static_cast(result); - }, - [](Continuation f) { f(); }); + [](int const& inner) { return static_cast(inner * 2.0); }, + [&ioc](Continuation f) { + boost::asio::post(ioc, [f = std::move(f)]() mutable { f(); }); + }); - promise.Resolve(43); + promise.Resolve(42); - bool work_ran = false; - boost::asio::post(ioc, [&]() { work_ran = true; }); + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_FLOAT_EQ(*result, 84.0f); work.reset(); ioc_thread.join(); - - ASSERT_TRUE(work_ran); } From 982bce3aaedffe472f2b5c93f090be70ca234643 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 10 Apr 2026 14:17:40 -0700 Subject: [PATCH 05/19] add WhenAny and WhenAll helpers --- .../include/launchdarkly/async/promise.hpp | 92 ++++++++ libs/internal/tests/promise_test.cpp | 211 +++++++++++++----- 2 files changed, 251 insertions(+), 52 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index 66222ecd1..db8e426b0 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -418,4 +419,95 @@ class Future { std::shared_ptr> internal_; }; +// WhenAll takes a variadic list of Futures (each with potentially different +// value types) and returns a Future that resolves once all +// of the input futures have resolved. The result carries no value; callers +// who need the individual results can read them from their original futures +// after WhenAll resolves. +// +// If called with no arguments, the returned future is already resolved. +// +// Example: +// +// Future f1 = ...; +// Future f2 = ...; +// WhenAll(f1, f2).Then( +// [&](std::monostate const&) { +// // f1 and f2 are both finished here. +// use(*f1.GetResult(), *f2.GetResult()); +// return std::monostate{}; +// }, +// executor); +template +Future WhenAll(Future... futures) { + Promise promise; + Future result = promise.GetFuture(); + + if constexpr (sizeof...(Ts) == 0) { + promise.Resolve(std::monostate{}); + return result; + } + + auto shared_promise = + std::make_shared>(std::move(promise)); + auto count = std::make_shared>(sizeof...(Ts)); + + auto attach = [&](auto future) { + future.Then( + [shared_promise, count](auto const&) -> std::monostate { + if (count->fetch_sub(1) == 1) { + shared_promise->Resolve(std::monostate{}); + } + return std::monostate{}; + }, + [](Continuation f) { f(); }); + }; + + (attach(futures), ...); + + return result; +} + +// WhenAny takes a variadic list of Futures (each with potentially different +// value types) and returns a Future that resolves with the +// 0-based index of whichever input future resolves first. The caller can use +// the index to identify the winning future and read its result directly. +// +// If called with no arguments, the returned future never resolves. +// +// Example: +// +// Future f0 = ...; +// Future f1 = ...; +// WhenAny(f0, f1).Then( +// [&](std::size_t const& index) { +// if (index == 0) use(*f0.GetResult()); +// else use(*f1.GetResult()); +// return std::monostate{}; +// }, +// executor); +template +Future WhenAny(Future... futures) { + Promise promise; + Future result = promise.GetFuture(); + + auto shared_promise = + std::make_shared>(std::move(promise)); + + std::size_t index = 0; + auto attach = [&](auto future) { + std::size_t i = index++; + future.Then( + [shared_promise, i](auto const&) -> std::monostate { + shared_promise->Resolve(i); + return std::monostate{}; + }, + [](Continuation f) { f(); }); + }; + + (attach(futures), ...); + + return result; +} + } // namespace launchdarkly::async diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index 5dbced30e..382e33e63 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -9,6 +9,45 @@ using namespace launchdarkly::async; +TEST(Promise, SimplePromise) { + Promise promise; + Future future = promise.GetFuture(); + + Future future2 = future.Then( + [](int const& inner) { return static_cast(inner * 2.0); }, + [](Continuation f) { f(); }); + + promise.Resolve(43); + + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_FLOAT_EQ(*result, 86.0f); +} + +TEST(Promise, ASIOTest) { + boost::asio::io_context ioc; + auto work = boost::asio::make_work_guard(ioc); + std::thread ioc_thread([&]() { ioc.run(); }); + + Promise promise; + Future future = promise.GetFuture(); + + Future future2 = future.Then( + [](int const& inner) { return static_cast(inner * 2.0); }, + [&ioc](Continuation f) { + boost::asio::post(ioc, [f = std::move(f)]() mutable { f(); }); + }); + + promise.Resolve(42); + + auto& result = future2.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(result.has_value()); + EXPECT_FLOAT_EQ(*result, 84.0f); + + work.reset(); + ioc_thread.join(); +} + TEST(Promise, GetResultNotFinished) { Promise promise; Future future = promise.GetFuture(); @@ -53,9 +92,9 @@ TEST(Promise, ContinueByReturningFuture) { Promise promise2; Future future2 = promise2.GetFuture(); - Future chained = promise1.GetFuture().Then( - [future2](int const&) { return future2; }, - [](Continuation f) { f(); }); + Future chained = + promise1.GetFuture().Then([future2](int const&) { return future2; }, + [](Continuation f) { f(); }); promise1.Resolve(0); promise2.Resolve(42); @@ -71,9 +110,8 @@ TEST(Promise, ResolvedBeforeContinuation) { promise.Resolve(21); - Future future2 = future.Then( - [](int const& val) { return val * 2; }, - [](Continuation f) { f(); }); + Future future2 = future.Then([](int const& val) { return val * 2; }, + [](Continuation f) { f(); }); auto& result = future2.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); @@ -84,9 +122,8 @@ TEST(Promise, ResolvedAfterContinuation) { Promise promise; Future future = promise.GetFuture(); - Future future2 = future.Then( - [](int const& val) { return val * 2; }, - [](Continuation f) { f(); }); + Future future2 = future.Then([](int const& val) { return val * 2; }, + [](Continuation f) { f(); }); promise.Resolve(21); @@ -140,11 +177,10 @@ TEST(Promise, MoveOnlyCallback) { Future future = promise.GetFuture(); auto captured = std::make_unique(2); - Future future2 = future.Then( - [captured = std::move(captured)](int const& val) { - return val * *captured; - }, - [](Continuation f) { f(); }); + Future future2 = + future.Then([captured = std::move(captured)]( + int const& val) { return val * *captured; }, + [](Continuation f) { f(); }); promise.Resolve(21); @@ -161,8 +197,15 @@ TEST(Promise, ResultMovedWhenPossible) { int value; int* copy_count; explicit Counted(int v, int* c) : value(v), copy_count(c) {} - Counted(Counted const& o) : value(o.value), copy_count(o.copy_count) { ++(*copy_count); } - Counted& operator=(Counted const& o) { value = o.value; copy_count = o.copy_count; ++(*copy_count); return *this; } + Counted(Counted const& o) : value(o.value), copy_count(o.copy_count) { + ++(*copy_count); + } + Counted& operator=(Counted const& o) { + value = o.value; + copy_count = o.copy_count; + ++(*copy_count); + return *this; + } Counted(Counted&&) noexcept = default; Counted& operator=(Counted&&) noexcept = default; }; @@ -186,8 +229,15 @@ TEST(Promise, CallbackMovedWhenPossible) { int value; int* copy_count; explicit Counted(int v, int* c) : value(v), copy_count(c) {} - Counted(Counted const& o) : value(o.value), copy_count(o.copy_count) { ++(*copy_count); } - Counted& operator=(Counted const& o) { value = o.value; copy_count = o.copy_count; ++(*copy_count); return *this; } + Counted(Counted const& o) : value(o.value), copy_count(o.copy_count) { + ++(*copy_count); + } + Counted& operator=(Counted const& o) { + value = o.value; + copy_count = o.copy_count; + ++(*copy_count); + return *this; + } Counted(Counted&&) noexcept = default; Counted& operator=(Counted&&) noexcept = default; }; @@ -198,19 +248,18 @@ TEST(Promise, CallbackMovedWhenPossible) { Counted multiplier{2, &copies}; copies = 0; // reset after construction - future.Then( - [multiplier = std::move(multiplier)](int const& val) mutable { - return val * multiplier.value; - }, - [](Continuation f) { f(); }); + future.Then([multiplier = std::move(multiplier)]( + int const& val) mutable { return val * multiplier.value; }, + [](Continuation f) { f(); }); EXPECT_EQ(copies, 0); promise.Resolve(21); } -// Demonstrates using std::monostate as a void-like result type for fire-and-forget -// async operations where the completion matters but no value is produced. +// Demonstrates using std::monostate as a void-like result type for +// fire-and-forget async operations where the completion matters but no value is +// produced. TEST(Promise, MonostateVoidLike) { Promise promise; Future future = promise.GetFuture(); @@ -258,42 +307,100 @@ TEST(Promise, ExpectedFailure) { EXPECT_EQ(result->error(), "timed out"); } +TEST(WhenAll, NoFutures) { + Future result = WhenAll(); + EXPECT_TRUE(result.IsFinished()); +} -TEST(Promise, SimplePromise) { - Promise promise; - Future future = promise.GetFuture(); +// Verifies WhenAll resolves when all futures are already resolved. +TEST(WhenAll, AllAlreadyResolved) { + Promise p1; + Promise p2; - Future future2 = future.Then( - [](int const& inner) { return static_cast(inner * 2.0); }, - [](Continuation f) { f(); }); + p1.Resolve(1); + p2.Resolve("hello"); - promise.Resolve(43); + Future result = WhenAll(p1.GetFuture(), p2.GetFuture()); + auto& r = result.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(r.has_value()); +} - auto& result = future2.WaitForResult(std::chrono::seconds(5)); - ASSERT_TRUE(result.has_value()); - EXPECT_FLOAT_EQ(*result, 86.0f); +// Verifies WhenAll resolves only after all futures resolve, using futures of +// mixed value types. The original futures still hold their results afterward. +TEST(WhenAll, ResolvesAfterAll) { + Promise p1; + Promise p2; + + Future f1 = p1.GetFuture(); + Future f2 = p2.GetFuture(); + + Future result = WhenAll(f1, f2); + + EXPECT_FALSE(result.IsFinished()); + p1.Resolve(42); + EXPECT_FALSE(result.IsFinished()); + p2.Resolve("done"); + + auto& r = result.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(r.has_value()); + EXPECT_EQ(*f1.GetResult(), 42); + EXPECT_EQ(*f2.GetResult(), "done"); } -TEST(Promise, ASIOTest) { - boost::asio::io_context ioc; - auto work = boost::asio::make_work_guard(ioc); - std::thread ioc_thread([&]() { ioc.run(); }); +// Verifies that WhenAny resolves with the index of the first future to resolve. +TEST(WhenAny, FirstResolved) { + Promise p0; + Promise p1; + Promise p2; - Promise promise; - Future future = promise.GetFuture(); + Future result = + WhenAny(p0.GetFuture(), p1.GetFuture(), p2.GetFuture()); - Future future2 = future.Then( - [](int const& inner) { return static_cast(inner * 2.0); }, - [&ioc](Continuation f) { - boost::asio::post(ioc, [f = std::move(f)]() mutable { f(); }); - }); + EXPECT_FALSE(result.IsFinished()); + p1.Resolve(42); - promise.Resolve(42); + auto& r = result.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(r.has_value()); + EXPECT_EQ(*r, 1u); +} - auto& result = future2.WaitForResult(std::chrono::seconds(5)); - ASSERT_TRUE(result.has_value()); - EXPECT_FLOAT_EQ(*result, 84.0f); +// Verifies that WhenAny works with futures of mixed value types, and that +// resolving a later future after the winner does not change the result. +TEST(WhenAny, MixedTypesFirstWins) { + Promise p0; + Promise p1; + + Future f0 = p0.GetFuture(); + Future f1 = p1.GetFuture(); + + Future> result = WhenAny(f0, f1).Then( + [f0, f1](size_t const& index) -> std::variant { + if (index == 0) { + return f0.GetResult().value(); + } else { + return f1.GetResult().value(); + } + }, + [](Continuation f) { f(); }); - work.reset(); - ioc_thread.join(); + p1.Resolve("hello"); + p0.Resolve(99); + + auto& r = result.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(r.has_value()); + EXPECT_EQ(std::get(*result.GetResult()), "hello"); +} + +// Verifies that WhenAny resolves immediately if a future is already resolved. +TEST(WhenAny, AlreadyResolved) { + Promise p0; + Promise p1; + + p0.Resolve(42); + + Future result = WhenAny(p0.GetFuture(), p1.GetFuture()); + + auto& r = result.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(r.has_value()); + EXPECT_EQ(*r, 0u); } From 9e414532ea43eccab0b426da101b4dffa264c151 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 14:39:39 -0700 Subject: [PATCH 06/19] fix possible race condition by running the executor outside the lock --- .../include/launchdarkly/async/promise.hpp | 114 ++++++++++-------- 1 file changed, 66 insertions(+), 48 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index db8e426b0..047e1e02c 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -114,22 +114,29 @@ class PromiseInternal { // executors. Returns true if the result was set, or false if it was already // set by a previous call to Resolve. bool Resolve(T result) { - std::lock_guard lock(mutex_); - if (result_->has_value()) { - return false; + std::vector>)>> + to_call; + { + std::lock_guard lock(mutex_); + if (result_->has_value()) { + return false; + } + + // Move result into storage if possible; otherwise copy. + if constexpr (std::is_move_assignable_v) { + *result_ = std::move(result); + } else { + *result_ = result; + } + + to_call = std::move(continuations_); } - // Move result into storage if possible; otherwise copy. - if constexpr (std::is_move_assignable_v) { - *result_ = std::move(result); - } else { - *result_ = result; - } - - for (auto& continuation : continuations_) { + // Call continuations outside the lock so that continuations which + // re-enter this future (e.g. via GetResult or Then) don't deadlock. + for (auto& continuation : to_call) { continuation(result_); } - continuations_.clear(); return true; } @@ -157,32 +164,39 @@ class PromiseInternal { Future Then(F&& continuation, std::function)> executor) { Promise newPromise; - std::lock_guard lock(mutex_); - Future newFuture = newPromise.GetFuture(); - if (result_->has_value()) { - executor(Continuation( - [newPromise = std::move(newPromise), - continuation = std::move(continuation), - result = result_]() mutable { - newPromise.Resolve(continuation(**result)); - })); - return newFuture; - } + std::shared_ptr> already_resolved; + { + std::lock_guard lock(mutex_); - continuations_.push_back( - [newPromise = std::move(newPromise), - continuation = std::move(continuation), - executor](std::shared_ptr> result) mutable { - executor(Continuation( + if (result_->has_value()) { + already_resolved = result_; + } else { + continuations_.push_back( [newPromise = std::move(newPromise), continuation = std::move(continuation), - result]() mutable { - newPromise.Resolve(continuation(**result)); - })); - }); + executor]( + std::shared_ptr> result) mutable { + executor(Continuation( + [newPromise = std::move(newPromise), + continuation = std::move(continuation), + result]() mutable { + newPromise.Resolve(continuation(**result)); + })); + }); + return newFuture; + } + } + // Already resolved: call executor outside the lock so that + // continuations which re-enter this future don't deadlock. + executor(Continuation( + [newPromise = std::move(newPromise), + continuation = std::move(continuation), + result = already_resolved]() mutable { + newPromise.Resolve(continuation(**result)); + })); return newFuture; } @@ -198,8 +212,6 @@ class PromiseInternal { Future Then(F&& continuation, std::function)> executor) { Promise outerPromise; - std::lock_guard lock(mutex_); - Future outerFuture = outerPromise.GetFuture(); auto do_work = [outerPromise = std::move(outerPromise), @@ -215,23 +227,29 @@ class PromiseInternal { executor); }; - if (result_->has_value()) { - executor(Continuation( - [do_work = std::move(do_work), result = result_]() mutable { - do_work(**result); - })); - return outerFuture; + std::shared_ptr> already_resolved; + { + std::lock_guard lock(mutex_); + if (result_->has_value()) { + already_resolved = result_; + } else { + continuations_.push_back( + [do_work = std::move(do_work), + executor](std::shared_ptr> result) mutable { + executor(Continuation( + [do_work = std::move(do_work), result]() mutable { + do_work(**result); + })); + }); + return outerFuture; + } } - continuations_.push_back( + // Already resolved: call executor outside the lock so that + // continuations which re-enter this future don't deadlock. + executor(Continuation( [do_work = std::move(do_work), - executor](std::shared_ptr> result) mutable { - executor(Continuation( - [do_work = std::move(do_work), result]() mutable { - do_work(**result); - })); - }); - + result = already_resolved]() mutable { do_work(**result); })); return outerFuture; } From a78b638a7857fdfe8cfa9d2a689b7235ca742be5 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 14:42:35 -0700 Subject: [PATCH 07/19] autoformat --- .../include/launchdarkly/async/promise.hpp | 58 ++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index 047e1e02c..411e92f48 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -159,7 +159,8 @@ class PromiseInternal { template , - // Disable when R is a Future so the flattening overload wins instead. + // Disable when R is a Future so the flattening overload wins + // instead. typename = std::enable_if_t::value>> Future Then(F&& continuation, std::function)> executor) { @@ -175,8 +176,7 @@ class PromiseInternal { } else { continuations_.push_back( [newPromise = std::move(newPromise), - continuation = std::move(continuation), - executor]( + continuation = std::move(continuation), executor]( std::shared_ptr> result) mutable { executor(Continuation( [newPromise = std::move(newPromise), @@ -191,12 +191,11 @@ class PromiseInternal { // Already resolved: call executor outside the lock so that // continuations which re-enter this future don't deadlock. - executor(Continuation( - [newPromise = std::move(newPromise), - continuation = std::move(continuation), - result = already_resolved]() mutable { - newPromise.Resolve(continuation(**result)); - })); + executor(Continuation([newPromise = std::move(newPromise), + continuation = std::move(continuation), + result = already_resolved]() mutable { + newPromise.Resolve(continuation(**result)); + })); return newFuture; } @@ -205,9 +204,11 @@ class PromiseInternal { template , - // Unwrap Future -> T2 so the return type is Future, not Future>. + // Unwrap Future -> T2 so the return type is Future, not + // Future>. typename T2 = typename future_value::type, - // Only enabled when R is a Future; otherwise the direct overload wins. + // Only enabled when R is a Future; otherwise the direct overload + // wins. typename = std::enable_if_t::value>> Future Then(F&& continuation, std::function)> executor) { @@ -234,8 +235,8 @@ class PromiseInternal { already_resolved = result_; } else { continuations_.push_back( - [do_work = std::move(do_work), - executor](std::shared_ptr> result) mutable { + [do_work = std::move(do_work), executor]( + std::shared_ptr> result) mutable { executor(Continuation( [do_work = std::move(do_work), result]() mutable { do_work(**result); @@ -363,16 +364,15 @@ class Future { // The continuation ignores T entirely (returning a dummy int) so that // T is not required to be copyable. The executor signals the cv when // called, since being called means the original future has resolved. - Then( - [](T const&) { return 0; }, - [state](Continuation work) { - { - std::lock_guard lock(state->mutex); - state->ready = true; - } - state->cv.notify_one(); - work(); - }); + Then([](T const&) { return 0; }, + [state](Continuation work) { + { + std::lock_guard lock(state->mutex); + state->ready = true; + } + state->cv.notify_one(); + work(); + }); std::unique_lock lock(state->mutex); state->cv.wait_for(lock, timeout, [&state] { return state->ready; }); @@ -393,7 +393,8 @@ class Future { template , - // Disable when R is a Future so the flattening overload wins instead. + // Disable when R is a Future so the flattening overload wins + // instead. typename = std::enable_if_t::value>> Future Then(F&& continuation, std::function)> executor) { @@ -409,7 +410,8 @@ class Future { // // Future> result = future.Then( // [](tl::expected const& key) { - // return fetch(key); // fetch returns Future> + // return fetch(key); // fetch returns Future> // }, // executor); // @@ -423,9 +425,11 @@ class Future { template , - // Unwrap Future -> T2 so the return type is Future, not Future>. + // Unwrap Future -> T2 so the return type is Future, not + // Future>. typename T2 = typename future_value::type, - // Only enabled when R is a Future; otherwise the direct overload wins. + // Only enabled when R is a Future; otherwise the direct overload + // wins. typename = std::enable_if_t::value>> Future Then(F&& continuation, std::function)> executor) { From c3db48af77bd2a537a838234d3ef40a2dd7d49bf Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 15:09:18 -0700 Subject: [PATCH 08/19] fix continuations with lvalue callables --- libs/internal/include/launchdarkly/async/promise.hpp | 4 ++-- libs/internal/tests/promise_test.cpp | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index 411e92f48..cd441081c 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -43,7 +43,7 @@ class Continuation { template struct Impl : Base { F f; - Impl(F&& f) : f(std::move(f)) {} + Impl(F f) : f(std::move(f)) {} R call(Args... args) override { return f(std::forward(args)...); } }; @@ -56,7 +56,7 @@ class Continuation { // then moves it into Impl so Continuation itself owns the callable. template Continuation(F&& f) - : impl_(std::make_unique>(std::forward(f))) {} + : impl_(std::make_unique>>(std::forward(f))) {} Continuation(Continuation&&) = default; Continuation& operator=(Continuation&&) = default; diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index 382e33e63..9d4ec1c21 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -307,6 +307,14 @@ TEST(Promise, ExpectedFailure) { EXPECT_EQ(result->error(), "timed out"); } +// Verifies that a Continuation can be constructed from an lvalue callable +// (named lambda), not just from a temporary. +TEST(Promise, LvalueLambdaContinuation) { + auto fn = [](int x) { return x * 2; }; + Continuation c(fn); + EXPECT_EQ(c(21), 42); +} + TEST(WhenAll, NoFutures) { Future result = WhenAll(); EXPECT_TRUE(result.IsFinished()); From 190d114da00e66e0aa3a0df1d20162444a1d5731 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 15:31:27 -0700 Subject: [PATCH 09/19] fix assignment operators on Promise/Future --- .../include/launchdarkly/async/promise.hpp | 4 ++ libs/internal/tests/promise_test.cpp | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index cd441081c..fade3123f 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -283,7 +283,9 @@ class Promise { Promise() : internal_(new PromiseInternal()) {} ~Promise() = default; Promise(Promise const&) = delete; + Promise& operator=(Promise const&) = delete; Promise(Promise&&) = default; + Promise& operator=(Promise&&) = default; // Sets the result to the given value and schedules any continuations that // were registered via Future::Then. Returns true if the result was set, or @@ -336,7 +338,9 @@ class Future { : internal_(internal) {} ~Future() = default; Future(Future const&) = default; + Future& operator=(Future const&) = default; Future(Future&&) = default; + Future& operator=(Future&&) = default; // Returns true if the associated Promise has been resolved. bool IsFinished() const { return internal_->IsFinished(); } diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index 9d4ec1c21..bade3ac05 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -307,6 +307,43 @@ TEST(Promise, ExpectedFailure) { EXPECT_EQ(result->error(), "timed out"); } +// Verifies that Promise supports move assignment, consistent with it being +// move-only. +TEST(Promise, MoveAssignment) { + Promise p1; + Future future = p1.GetFuture(); + + Promise p2; + p2 = std::move(p1); + p2.Resolve(42); + + EXPECT_EQ(*future.GetResult(), 42); +} + +// Verifies that Future supports move assignment. +TEST(Promise, FutureMoveAssignment) { + Promise promise; + Future f1 = promise.GetFuture(); + Future f2 = promise.GetFuture(); + + f2 = std::move(f1); + promise.Resolve(42); + + EXPECT_EQ(*f2.GetResult(), 42); +} + +// Verifies that Future supports copy assignment. +TEST(Promise, FutureCopyAssignment) { + Promise promise; + Future f1 = promise.GetFuture(); + Future f2 = promise.GetFuture(); + + f2 = f1; + promise.Resolve(42); + + EXPECT_EQ(*f2.GetResult(), 42); +} + // Verifies that a Continuation can be constructed from an lvalue callable // (named lambda), not just from a temporary. TEST(Promise, LvalueLambdaContinuation) { From 7fa60a3468eaefab756cbda44aecbd63623c0b37 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 15:41:01 -0700 Subject: [PATCH 10/19] handle the race where the result is modified after its reference is returned --- libs/internal/include/launchdarkly/async/promise.hpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index fade3123f..eb4662ac0 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -150,9 +150,17 @@ class PromiseInternal { // Returns a const reference to the optional result. The optional contains // a value if resolved, or is empty if not yet resolved. The reference is // valid for the lifetime of any Future referring to this PromiseInternal. + // + // Thread-safety: if the future is already resolved, the reference points + // to write-once storage and is safe to read without holding the lock. If + // the future is not yet resolved, the reference points to empty_, a const + // member that is never written, so it is equally safe. std::optional const& GetResult() const { std::lock_guard lock(mutex_); - return *result_; + if (result_->has_value()) { + return *result_; + } + return empty_; } // Then where the continuation returns R directly, yielding Future. @@ -260,6 +268,7 @@ class PromiseInternal { new std::optional(std::nullopt)}; std::vector>)>> continuations_; + std::optional const empty_{}; }; // Promise is the write end of a one-shot async value, similar to std::promise. From 0e817d428d4146929c1e13fe1cddeeb68d0be684 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 23:09:33 -0700 Subject: [PATCH 11/19] require the result of a promise to be copyable --- .../include/launchdarkly/async/promise.hpp | 115 +++++++----------- libs/internal/tests/promise_test.cpp | 79 +++--------- 2 files changed, 62 insertions(+), 132 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index eb4662ac0..b892f687e 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -100,8 +100,6 @@ struct future_value> { // copies all refer to the same underlying state, and lets Promise and Future // have independent lifetimes while the shared state remains alive as long as // either end holds it. -// -// TODO: Is it okay that the shared_ptr lets copies mutate the result?? template class PromiseInternal { public: @@ -114,28 +112,26 @@ class PromiseInternal { // executors. Returns true if the result was set, or false if it was already // set by a previous call to Resolve. bool Resolve(T result) { - std::vector>)>> - to_call; + std::vector> to_call; { std::lock_guard lock(mutex_); - if (result_->has_value()) { + if (result_.has_value()) { return false; } // Move result into storage if possible; otherwise copy. if constexpr (std::is_move_assignable_v) { - *result_ = std::move(result); + result_ = std::move(result); } else { - *result_ = result; + result_ = result; } - to_call = std::move(continuations_); } // Call continuations outside the lock so that continuations which // re-enter this future (e.g. via GetResult or Then) don't deadlock. for (auto& continuation : to_call) { - continuation(result_); + continuation(*result_); } return true; @@ -144,23 +140,13 @@ class PromiseInternal { // Returns true if Resolve has been called. bool IsFinished() const { std::lock_guard lock(mutex_); - return result_->has_value(); + return result_.has_value(); } - // Returns a const reference to the optional result. The optional contains - // a value if resolved, or is empty if not yet resolved. The reference is - // valid for the lifetime of any Future referring to this PromiseInternal. - // - // Thread-safety: if the future is already resolved, the reference points - // to write-once storage and is safe to read without holding the lock. If - // the future is not yet resolved, the reference points to empty_, a const - // member that is never written, so it is equally safe. - std::optional const& GetResult() const { + // Returns a copy of the result, if resolved. + std::optional GetResult() const { std::lock_guard lock(mutex_); - if (result_->has_value()) { - return *result_; - } - return empty_; + return result_; } // Then where the continuation returns R directly, yielding Future. @@ -175,22 +161,22 @@ class PromiseInternal { Promise newPromise; Future newFuture = newPromise.GetFuture(); - std::shared_ptr> already_resolved; + T already_resolved; { std::lock_guard lock(mutex_); - if (result_->has_value()) { - already_resolved = result_; + if (result_.has_value()) { + already_resolved = *result_; } else { continuations_.push_back( [newPromise = std::move(newPromise), - continuation = std::move(continuation), executor]( - std::shared_ptr> result) mutable { + continuation = std::move(continuation), + executor](T const& result) mutable { executor(Continuation( [newPromise = std::move(newPromise), continuation = std::move(continuation), result]() mutable { - newPromise.Resolve(continuation(**result)); + newPromise.Resolve(continuation(result)); })); }); return newFuture; @@ -199,11 +185,12 @@ class PromiseInternal { // Already resolved: call executor outside the lock so that // continuations which re-enter this future don't deadlock. - executor(Continuation([newPromise = std::move(newPromise), - continuation = std::move(continuation), - result = already_resolved]() mutable { - newPromise.Resolve(continuation(**result)); - })); + executor(Continuation( + [newPromise = std::move(newPromise), + continuation = std::move(continuation), + result = std::move(already_resolved)]() mutable { + newPromise.Resolve(continuation(result)); + })); return newFuture; } @@ -236,20 +223,19 @@ class PromiseInternal { executor); }; - std::shared_ptr> already_resolved; + T already_resolved; { std::lock_guard lock(mutex_); - if (result_->has_value()) { - already_resolved = result_; + if (result_.has_value()) { + already_resolved = *result_; } else { - continuations_.push_back( - [do_work = std::move(do_work), executor]( - std::shared_ptr> result) mutable { - executor(Continuation( - [do_work = std::move(do_work), result]() mutable { - do_work(**result); - })); - }); + continuations_.push_back([do_work = std::move(do_work), + executor](T const& result) mutable { + executor(Continuation( + [do_work = std::move(do_work), result]() mutable { + do_work(result); + })); + }); return outerFuture; } } @@ -258,17 +244,16 @@ class PromiseInternal { // continuations which re-enter this future don't deadlock. executor(Continuation( [do_work = std::move(do_work), - result = already_resolved]() mutable { do_work(**result); })); + result = std::move(already_resolved)]() mutable { + do_work(result); + })); return outerFuture; } private: mutable std::mutex mutex_; - std::shared_ptr> result_{ - new std::optional(std::nullopt)}; - std::vector>)>> - continuations_; - std::optional const empty_{}; + std::optional result_{std::nullopt}; + std::vector> continuations_; }; // Promise is the write end of a one-shot async value, similar to std::promise. @@ -299,13 +284,7 @@ class Promise { // Sets the result to the given value and schedules any continuations that // were registered via Future::Then. Returns true if the result was set, or // false if Resolve was already called. - bool Resolve(T result) { - if constexpr (std::is_move_constructible_v) { - return internal_->Resolve(std::move(result)); - } else { - return internal_->Resolve(result); - } - } + bool Resolve(T result) { return internal_->Resolve(result); } // Returns a Future that will resolve when this Promise is resolved. // May be called multiple times; each call returns a Future referring to @@ -354,19 +333,13 @@ class Future { // Returns true if the associated Promise has been resolved. bool IsFinished() const { return internal_->IsFinished(); } - // Returns a const reference to the optional result. The optional contains - // a value if resolved, or is empty if not yet resolved. The reference is - // valid for the lifetime of this Future (or any copy of it). - std::optional const& GetResult() const { return internal_->GetResult(); } + // Returns a copy of the result, if resolved. + std::optional GetResult() const { return internal_->GetResult(); } // Blocks the calling thread until the future resolves or the timeout - // expires. Returns a const reference to the optional result: the optional - // contains a value if resolved within the timeout, or is empty if the - // timeout expired first. The reference is valid for the lifetime of this - // Future (or any copy of it). + // expires. Returns a copy of the result, if resolved within the timeout. template - std::optional const& WaitForResult( - std::chrono::duration timeout) { + std::optional WaitForResult(std::chrono::duration timeout) { struct State { std::mutex mutex; std::condition_variable cv; @@ -374,9 +347,9 @@ class Future { }; auto state = std::make_shared(); - // The continuation ignores T entirely (returning a dummy int) so that - // T is not required to be copyable. The executor signals the cv when - // called, since being called means the original future has resolved. + // The continuation ignores T entirely (returning a dummy int). The + // executor signals the cv when called, since being called means the + // original future has resolved. Then([](T const&) { return 0; }, [state](Continuation work) { { diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index bade3ac05..5efe00df3 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -19,7 +19,7 @@ TEST(Promise, SimplePromise) { promise.Resolve(43); - auto& result = future2.WaitForResult(std::chrono::seconds(5)); + auto result = future2.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_FLOAT_EQ(*result, 86.0f); } @@ -40,7 +40,7 @@ TEST(Promise, ASIOTest) { promise.Resolve(42); - auto& result = future2.WaitForResult(std::chrono::seconds(5)); + auto result = future2.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_FLOAT_EQ(*result, 84.0f); @@ -52,7 +52,7 @@ TEST(Promise, GetResultNotFinished) { Promise promise; Future future = promise.GetFuture(); - auto& result = future.GetResult(); + auto result = future.GetResult(); EXPECT_FALSE(result.has_value()); } @@ -62,7 +62,7 @@ TEST(Promise, GetResultFinished) { promise.Resolve(42); - auto& result = future.GetResult(); + auto result = future.GetResult(); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, 42); } @@ -82,7 +82,7 @@ TEST(Promise, CopyOnlyResult) { promise.Resolve(CopyOnly{42}); - auto& result = future.GetResult(); + auto result = future.GetResult(); ASSERT_TRUE(result.has_value()); EXPECT_EQ(result->value, 42); } @@ -99,7 +99,7 @@ TEST(Promise, ContinueByReturningFuture) { promise1.Resolve(0); promise2.Resolve(42); - auto& result = chained.WaitForResult(std::chrono::seconds(5)); + auto result = chained.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, 42); } @@ -113,7 +113,7 @@ TEST(Promise, ResolvedBeforeContinuation) { Future future2 = future.Then([](int const& val) { return val * 2; }, [](Continuation f) { f(); }); - auto& result = future2.WaitForResult(std::chrono::seconds(5)); + auto result = future2.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, 42); } @@ -127,22 +127,11 @@ TEST(Promise, ResolvedAfterContinuation) { promise.Resolve(21); - auto& result = future2.WaitForResult(std::chrono::seconds(5)); + auto result = future2.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, 42); } -TEST(Promise, MoveOnlyResult) { - Promise> promise; - Future> future = promise.GetFuture(); - - promise.Resolve(std::make_unique(42)); - - auto& result = future.GetResult(); - ASSERT_TRUE(result.has_value()); - EXPECT_EQ(**result, 42); -} - // Verifies that a continuation which captures a copy-only type (deleted move // constructor) can still be registered and invoked correctly. TEST(Promise, CopyOnlyCallback) { @@ -165,7 +154,7 @@ TEST(Promise, CopyOnlyCallback) { promise.Resolve(21); - auto& result = future2.WaitForResult(std::chrono::seconds(5)); + auto result = future2.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, 42); } @@ -184,43 +173,11 @@ TEST(Promise, MoveOnlyCallback) { promise.Resolve(21); - auto& result = future2.WaitForResult(std::chrono::seconds(5)); + auto result = future2.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, 42); } -// Verifies that when T is both moveable and copyable, resolving a promise -// moves the value into storage rather than copying it. -TEST(Promise, ResultMovedWhenPossible) { - int copies = 0; - struct Counted { - int value; - int* copy_count; - explicit Counted(int v, int* c) : value(v), copy_count(c) {} - Counted(Counted const& o) : value(o.value), copy_count(o.copy_count) { - ++(*copy_count); - } - Counted& operator=(Counted const& o) { - value = o.value; - copy_count = o.copy_count; - ++(*copy_count); - return *this; - } - Counted(Counted&&) noexcept = default; - Counted& operator=(Counted&&) noexcept = default; - }; - - Promise promise; - Future future = promise.GetFuture(); - - promise.Resolve(Counted{42, &copies}); - - auto& result = future.GetResult(); - ASSERT_TRUE(result.has_value()); - EXPECT_EQ(result->value, 42); - EXPECT_EQ(copies, 0); -} - // Verifies that when a continuation is both moveable and copyable, Then stores // it by moving rather than copying. TEST(Promise, CallbackMovedWhenPossible) { @@ -274,7 +231,7 @@ TEST(Promise, MonostateVoidLike) { promise.Resolve(std::monostate{}); - auto& result = future.WaitForResult(std::chrono::seconds(5)); + auto result = future.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(result.has_value()); EXPECT_TRUE(ran); } @@ -287,7 +244,7 @@ TEST(Promise, ExpectedSuccess) { promise.Resolve(42); - auto& result = future.GetResult(); + auto result = future.GetResult(); ASSERT_TRUE(result.has_value()); ASSERT_TRUE(result->has_value()); EXPECT_EQ(**result, 42); @@ -301,7 +258,7 @@ TEST(Promise, ExpectedFailure) { promise.Resolve(tl::unexpected(std::string("timed out"))); - auto& result = future.GetResult(); + auto result = future.GetResult(); ASSERT_TRUE(result.has_value()); ASSERT_FALSE(result->has_value()); EXPECT_EQ(result->error(), "timed out"); @@ -366,7 +323,7 @@ TEST(WhenAll, AllAlreadyResolved) { p2.Resolve("hello"); Future result = WhenAll(p1.GetFuture(), p2.GetFuture()); - auto& r = result.WaitForResult(std::chrono::seconds(5)); + auto r = result.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(r.has_value()); } @@ -386,7 +343,7 @@ TEST(WhenAll, ResolvesAfterAll) { EXPECT_FALSE(result.IsFinished()); p2.Resolve("done"); - auto& r = result.WaitForResult(std::chrono::seconds(5)); + auto r = result.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(r.has_value()); EXPECT_EQ(*f1.GetResult(), 42); EXPECT_EQ(*f2.GetResult(), "done"); @@ -404,7 +361,7 @@ TEST(WhenAny, FirstResolved) { EXPECT_FALSE(result.IsFinished()); p1.Resolve(42); - auto& r = result.WaitForResult(std::chrono::seconds(5)); + auto r = result.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(r.has_value()); EXPECT_EQ(*r, 1u); } @@ -431,7 +388,7 @@ TEST(WhenAny, MixedTypesFirstWins) { p1.Resolve("hello"); p0.Resolve(99); - auto& r = result.WaitForResult(std::chrono::seconds(5)); + auto r = result.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(r.has_value()); EXPECT_EQ(std::get(*result.GetResult()), "hello"); } @@ -445,7 +402,7 @@ TEST(WhenAny, AlreadyResolved) { Future result = WhenAny(p0.GetFuture(), p1.GetFuture()); - auto& r = result.WaitForResult(std::chrono::seconds(5)); + auto r = result.WaitForResult(std::chrono::seconds(5)); ASSERT_TRUE(r.has_value()); EXPECT_EQ(*r, 0u); } From dd3563101207e18454a61bb9e4ab90674db4ec2d Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 23:19:38 -0700 Subject: [PATCH 12/19] minor cleanup --- .../include/launchdarkly/async/promise.hpp | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index b892f687e..eadd59cc6 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -161,12 +161,12 @@ class PromiseInternal { Promise newPromise; Future newFuture = newPromise.GetFuture(); - T already_resolved; + std::optional already_resolved; { std::lock_guard lock(mutex_); if (result_.has_value()) { - already_resolved = *result_; + already_resolved = result_; } else { continuations_.push_back( [newPromise = std::move(newPromise), @@ -189,7 +189,7 @@ class PromiseInternal { [newPromise = std::move(newPromise), continuation = std::move(continuation), result = std::move(already_resolved)]() mutable { - newPromise.Resolve(continuation(result)); + newPromise.Resolve(continuation(*result)); })); return newFuture; } @@ -216,18 +216,18 @@ class PromiseInternal { Future innerFuture = continuation(val); innerFuture.Then( [outerPromise = std::move(outerPromise)]( - T2 const& inner_val) mutable -> T2 { + T2 const& inner_val) mutable -> std::monostate { outerPromise.Resolve(inner_val); - return inner_val; + return {}; }, executor); }; - T already_resolved; + std::optional already_resolved; { std::lock_guard lock(mutex_); if (result_.has_value()) { - already_resolved = *result_; + already_resolved = result_; } else { continuations_.push_back([do_work = std::move(do_work), executor](T const& result) mutable { @@ -245,14 +245,14 @@ class PromiseInternal { executor(Continuation( [do_work = std::move(do_work), result = std::move(already_resolved)]() mutable { - do_work(result); + do_work(*result); })); return outerFuture; } private: mutable std::mutex mutex_; - std::optional result_{std::nullopt}; + std::optional result_{}; std::vector> continuations_; }; @@ -284,7 +284,13 @@ class Promise { // Sets the result to the given value and schedules any continuations that // were registered via Future::Then. Returns true if the result was set, or // false if Resolve was already called. - bool Resolve(T result) { return internal_->Resolve(result); } + bool Resolve(T result) { + if constexpr (std::is_move_constructible_v) { + return internal_->Resolve(std::move(result)); + } else { + return internal_->Resolve(result); + } + } // Returns a Future that will resolve when this Promise is resolved. // May be called multiple times; each call returns a Future referring to From 2b2f37127b14341b4a123f5ee4ccd2afb065f24d Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 23:30:09 -0700 Subject: [PATCH 13/19] minor improvements --- libs/internal/include/launchdarkly/async/promise.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index eadd59cc6..ce65e53f7 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -62,7 +62,7 @@ class Continuation { // Invokes the stored callable with the given arguments. // Returns whatever the callable returns. - R operator()(Args... args) { + R operator()(Args... args) const { return impl_->call(std::forward(args)...); } }; @@ -106,7 +106,9 @@ class PromiseInternal { PromiseInternal() = default; ~PromiseInternal() = default; PromiseInternal(PromiseInternal const&) = delete; + PromiseInternal& operator=(PromiseInternal const&) = delete; PromiseInternal(PromiseInternal&&) = delete; + PromiseInternal& operator=(PromiseInternal&&) = delete; // Sets the result and schedules all registered continuations via their // executors. Returns true if the result was set, or false if it was already @@ -329,7 +331,7 @@ template class Future { public: Future(std::shared_ptr> internal) - : internal_(internal) {} + : internal_(std::move(internal)) {} ~Future() = default; Future(Future const&) = default; Future& operator=(Future const&) = default; @@ -448,7 +450,7 @@ class Future { // WhenAll(f1, f2).Then( // [&](std::monostate const&) { // // f1 and f2 are both finished here. -// use(*f1.GetResult(), *f2.GetResult()); +// use(f1.GetResult().value(), f2.GetResult().value()); // return std::monostate{}; // }, // executor); From dec560f553a18e8b61bbaf3474f8e3b5382a4a7f Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 13 Apr 2026 23:36:35 -0700 Subject: [PATCH 14/19] minor improvements --- libs/internal/include/launchdarkly/async/promise.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index ce65e53f7..dd5498a2c 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -276,7 +276,7 @@ class PromiseInternal { template class Promise { public: - Promise() : internal_(new PromiseInternal()) {} + Promise() : internal_(std::make_shared>()) {} ~Promise() = default; Promise(Promise const&) = delete; Promise& operator=(Promise const&) = delete; @@ -497,8 +497,8 @@ Future WhenAll(Future... futures) { // Future f1 = ...; // WhenAny(f0, f1).Then( // [&](std::size_t const& index) { -// if (index == 0) use(*f0.GetResult()); -// else use(*f1.GetResult()); +// if (index == 0) use(f0.GetResult().value()); +// else use(f1.GetResult().value()); // return std::monostate{}; // }, // executor); From 50adc32251945e15b418f2f7ca55f2f1e2fa5e9c Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 15 Apr 2026 09:54:32 -0700 Subject: [PATCH 15/19] switch Then temp Future to use monostate --- libs/internal/include/launchdarkly/async/promise.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index dd5498a2c..ace1be67d 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -133,6 +133,8 @@ class PromiseInternal { // Call continuations outside the lock so that continuations which // re-enter this future (e.g. via GetResult or Then) don't deadlock. for (auto& continuation : to_call) { + // It's safe to access result_ outside the lock here, because it + // can't be changed again. continuation(*result_); } @@ -355,10 +357,10 @@ class Future { }; auto state = std::make_shared(); - // The continuation ignores T entirely (returning a dummy int). The - // executor signals the cv when called, since being called means the - // original future has resolved. - Then([](T const&) { return 0; }, + // The continuation ignores T entirely. The executor signals the cv + // when called, since being called means the original future has + // resolved. + Then([](T const&) { return std::monostate{}; }, [state](Continuation work) { { std::lock_guard lock(state->mutex); From 9a4c0d19f000cb04420cbeea76d39e8e0ed49953 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 15 Apr 2026 10:28:09 -0700 Subject: [PATCH 16/19] add another test with more threads --- libs/internal/tests/promise_test.cpp | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index 5efe00df3..96d6fe486 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -406,3 +406,38 @@ TEST(WhenAny, AlreadyResolved) { ASSERT_TRUE(r.has_value()); EXPECT_EQ(*r, 0u); } + +TEST(WhenAll, ConcurrentResolution) { + auto spawn = [](int val) { + Promise p; + Future f = p.GetFuture(); + return std::make_pair( + f, std::thread([p = std::move(p), val]() mutable { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + p.Resolve(val); + })); + }; + + auto [f1, t1] = spawn(1); + auto [f2, t2] = spawn(2); + auto [f3, t3] = spawn(3); + auto [f4, t4] = spawn(4); + auto [f5, t5] = spawn(5); + + auto result = WhenAll(f1, f2, f3, f4, f5); + + auto r = result.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(r.has_value()); + + EXPECT_EQ(f1.GetResult().value(), 1); + EXPECT_EQ(f2.GetResult().value(), 2); + EXPECT_EQ(f3.GetResult().value(), 3); + EXPECT_EQ(f4.GetResult().value(), 4); + EXPECT_EQ(f5.GetResult().value(), 5); + + t1.join(); + t2.join(); + t3.join(); + t4.join(); + t5.join(); +} From e3151f5e5a6ad7ed964885019f59be43419fbe77 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 15 Apr 2026 12:03:52 -0700 Subject: [PATCH 17/19] add tests --- libs/internal/tests/promise_test.cpp | 95 ++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index 96d6fe486..e0a1a6774 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -441,3 +441,98 @@ TEST(WhenAll, ConcurrentResolution) { t4.join(); t5.join(); } + +// Verifies that WaitForResult returns nullopt when the promise is never +// resolved within the timeout. +TEST(Promise, WaitForResultTimeout) { + Promise promise; + Future future = promise.GetFuture(); + + auto result = future.WaitForResult(std::chrono::milliseconds(50)); + EXPECT_FALSE(result.has_value()); +} + +// Verifies that multiple threads can concurrently register continuations on +// the same future before it resolves, and all continuations run after +// resolution. +TEST(Promise, ConcurrentThenRegistration) { + Promise promise; + Future future = promise.GetFuture(); + + std::atomic count{0}; + std::vector threads; + std::vector> results; + + for (int i = 0; i < 10; i++) { + results.push_back(future.Then( + [&count](int const&) { + count++; + return std::monostate{}; + }, + [](Continuation f) { f(); })); + } + + promise.Resolve(42); + + for (auto& r : results) { + ASSERT_TRUE(r.WaitForResult(std::chrono::seconds(5)).has_value()); + } + EXPECT_EQ(count, 10); +} + +// Verifies that when multiple threads race to resolve the same promise, +// exactly one succeeds and the rest return false. +TEST(Promise, ConcurrentResolveSingleWinner) { + Promise promise; + Future future = promise.GetFuture(); + + std::atomic winners{0}; + std::vector threads; + for (int i = 0; i < 10; i++) { + threads.emplace_back([&promise, &winners, i] { + if (promise.Resolve(i)) { + winners++; + } + }); + } + + for (auto& t : threads) { + t.join(); + } + + EXPECT_EQ(winners, 1); + EXPECT_TRUE(future.GetResult().has_value()); +} + +// Verifies that a chain of Then calls executes correctly when continuations +// are dispatched via a multi-threaded ASIO executor. +TEST(Promise, MultiThreadedASIOExecutor) { + boost::asio::io_context ioc; + auto work = boost::asio::make_work_guard(ioc); + + std::vector ioc_threads; + for (int i = 0; i < 4; i++) { + ioc_threads.emplace_back([&ioc] { ioc.run(); }); + } + + auto executor = [&ioc](Continuation f) { + boost::asio::post(ioc, std::move(f)); + }; + + Promise promise; + Future result = + promise.GetFuture() + .Then([](int const& v) { return v * 2; }, executor) + .Then([](int const& v) { return v + 1; }, executor); + + promise.Resolve(10); + + auto r = result.WaitForResult(std::chrono::seconds(5)); + ASSERT_TRUE(r.has_value()); + EXPECT_EQ(*r, 21); + + work.reset(); + for (auto& t : ioc_threads) { + t.join(); + } +} From c33f9f3defd45f610823fbf786d1676756b19222 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 15 Apr 2026 12:10:46 -0700 Subject: [PATCH 18/19] check for move constructible, not move assignable --- .../include/launchdarkly/async/promise.hpp | 2 +- libs/internal/tests/promise_test.cpp | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/libs/internal/include/launchdarkly/async/promise.hpp b/libs/internal/include/launchdarkly/async/promise.hpp index ace1be67d..ffdc66b87 100644 --- a/libs/internal/include/launchdarkly/async/promise.hpp +++ b/libs/internal/include/launchdarkly/async/promise.hpp @@ -122,7 +122,7 @@ class PromiseInternal { } // Move result into storage if possible; otherwise copy. - if constexpr (std::is_move_assignable_v) { + if constexpr (std::is_move_constructible_v) { result_ = std::move(result); } else { result_ = result; diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index e0a1a6774..23874df91 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -67,6 +67,30 @@ TEST(Promise, GetResultFinished) { EXPECT_EQ(*result, 42); } +// Verifies that a type which is move-assignable but not move-constructible +// can still be used as T. The optional assignment for an empty optional needs +// move-constructibility, not move-assignability, so the if constexpr branch +// must check the correct trait. +TEST(Promise, MoveAssignableNotMoveConstructible) { + struct MoveAssignableOnly { + int value; + explicit MoveAssignableOnly(int v) : value(v) {} + MoveAssignableOnly(MoveAssignableOnly const&) = default; + MoveAssignableOnly& operator=(MoveAssignableOnly const&) = default; + MoveAssignableOnly(MoveAssignableOnly&&) = delete; + MoveAssignableOnly& operator=(MoveAssignableOnly&&) = default; + }; + + Promise promise; + Future future = promise.GetFuture(); + + promise.Resolve(MoveAssignableOnly{42}); + + auto result = future.GetResult(); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->value, 42); +} + TEST(Promise, CopyOnlyResult) { struct CopyOnly { int value; From 5d5548861d94bde317e5502b7b73c491d76d008b Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 15 Apr 2026 12:30:08 -0700 Subject: [PATCH 19/19] remove an extraneous test --- libs/internal/tests/promise_test.cpp | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/libs/internal/tests/promise_test.cpp b/libs/internal/tests/promise_test.cpp index 23874df91..d338bcabd 100644 --- a/libs/internal/tests/promise_test.cpp +++ b/libs/internal/tests/promise_test.cpp @@ -476,34 +476,6 @@ TEST(Promise, WaitForResultTimeout) { EXPECT_FALSE(result.has_value()); } -// Verifies that multiple threads can concurrently register continuations on -// the same future before it resolves, and all continuations run after -// resolution. -TEST(Promise, ConcurrentThenRegistration) { - Promise promise; - Future future = promise.GetFuture(); - - std::atomic count{0}; - std::vector threads; - std::vector> results; - - for (int i = 0; i < 10; i++) { - results.push_back(future.Then( - [&count](int const&) { - count++; - return std::monostate{}; - }, - [](Continuation f) { f(); })); - } - - promise.Resolve(42); - - for (auto& r : results) { - ASSERT_TRUE(r.WaitForResult(std::chrono::seconds(5)).has_value()); - } - EXPECT_EQ(count, 10); -} - // Verifies that when multiple threads race to resolve the same promise, // exactly one succeeds and the rest return false. TEST(Promise, ConcurrentResolveSingleWinner) {