Commit 4b40bb4
authored
refactor: add internal Promise/Future classes for future use (#521)
This PR adds internal-only `Promise` and `Future` classes. These have
some advantages over using callbacks everywhere:
* They are more easily composable. That means utility functions like
`WhenAll` and `WhenAny` only have to be written once, instead of having
to re-invent the wheel every time we need those concepts.
* This allows C++ code to be structured more like the code in other
languages with proper promise types.
* Unlike `std::future`, this supports a `Then` method, which enables
things like `WhenAny` without occupying an extra thread per future.
* Unlike `std::function`, the `Continuation` class used in this
implementation supports move-only functions.
Some points to consider:
* Would other people like using these, or am I the only one?
* **Naming**: I chose to mimic the C++ names of `std::promise`,
`std::future`, and `then()` (experimental).
* **Executors**: I chose to _require_ passing an executor into `Then`,
because there is no natural default:
* C++ doesn't have a "main" thread exactly, or at least, it's not used
the same way as on other platforms.
* Immediate executors are confusing to reason about, and result in
calling work on unpredictable threads.
* We shouldn't assume `asio::post`, as this class may be used for other
things.
* I didn't want to add the complexity of storing execution contexts in a
thread-local.
* **Error handling**: I chose not to put error handling directly in the
`Future`, and encourage users to use `Future<std::expected<>>` if they
care about it, to keep things simpler.
* **Cancellation**: I chose not to implement cancellation. We can add it
in the future. But cancellation can be treated as mostly orthogonal to
futures, and we can't cancel http requests currently anyway, so 🤷♀️ .
* By default, `GetResult` is non-blocking, as this prevents unintended
blocking. There is a blocking version called `WaitForResult` with a
timeout if needed.
* The continuations in `Then` are passed the _result_ of the `Future`
instead of the `Future` itself. This is simpler, but it means we can't
add more metadata in the future without a breaking change. This is an
internal class, so I think it's better to keep it simple.
* The type `T` in `Promise<T>`/`Future<T>` has to be copyable. This is
an inherent limitation of the need to be able to have
`Future<Future<T>>` unwrappable to `Future<T>`, while also letting a
`Future` have multiple continuations.
* The executor has to be copyable, for simplicity. ASIO has the same
requirement on executors, and it seems fine.
I added lots of tests. Some of them double as documentation for humans
and LLMs.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Introduces new concurrency primitives (`Promise`/`Future`, move-only
`Continuation`, and `WhenAll`/`WhenAny`) with mutex/atomic coordination
and executor-dispatched continuations, so correctness depends on subtle
threading and lifetime behavior. Changes are currently additive/internal
but could impact future async flows once adopted.
>
> **Overview**
> Adds a new internal async module `launchdarkly/async/promise.hpp`
implementing `Promise<T>`/`Future<T>` with executor-based `Then`
chaining (including flattening when a continuation returns another
`Future`) and a move-only `Continuation` wrapper to support move-only
captures.
>
> Introduces aggregation helpers `WhenAll` and `WhenAny`, a blocking
`WaitForResult` (timeout-based) for test/support scenarios, and wires
the new async headers into the internal library build. Adds extensive
unit tests covering chaining, move/copy behavior, concurrent resolution,
and ASIO executor usage.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
0dfce88. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent 1c85d89 commit 4b40bb4
3 files changed
Lines changed: 1066 additions & 0 deletions
0 commit comments