What is the expected behavior of Future::thenInline?
- Dominant language
- C++
- Stars
- 30.5k
- Forks
- 5.9k
- PR merge metrics
- No merged PRs in 30d
Description
Hi, team
I have this PR to change the behavior of `Future::thenInline` when the previous Promise finished early, which I think is bug #2087 , could you please take a look, we could discuss this in the PR.
Also, when the previous callback returns a Future that is actually fulfilled in another executor, what should be the expected behavior? I would expect it to be executed in line with where the promise was fulfilled, as the below test illustrated, but this case failed, and we actually scheduled the inline callback to other executors.
```
TEST(Then, DISABLED_thenInlineFollowContinuationReturnsNonImmediateFuture) {
CPUThreadPoolExecutor e1{4};
uint64_t threadId1 = 0;
folly::makeFuture()
.via(&e1)
.thenValue([&threadId1](auto&&) {
return folly::via(folly::getGlobalCPUExecutor())
.then([&threadId1](auto&&) {
threadId1 = folly::getCurrentThreadID();
return true;
});
})
.thenInline([&threadId1](auto&&) {
auto threadId2 = folly::getCurrentThreadID();
ASSERT_EQ(threadId1, threadId2);
})
.wait();
}
```
It seems intentional per the below logic, but I don't really understand as it is not inline behavior:
https://github.com/facebook/folly/blob/main/folly/futures/detail/Core.cpp#L578-L584
```
auto currentKeepAlive = std::move(currentExecutor).stealKeepAlive();
if (addCompletingKA.get() == currentKeepAlive.get()) {
keepAliveFunc(std::move(currentKeepAlive));
} else {
std::move(currentKeepAlive).add(std::move(keepAliveFunc));
}
```
I don't see many tests around `Future::thenInline` in our codebase, seems like the behavior is not well-defined yet.
Thanks.
Contributor guide
Assessment
This issue has not been assessed yet.