Skip to content

Commit

Permalink
Revert D62476139: coro toTaskInterruptOnCancel
Browse files Browse the repository at this point in the history
Differential Revision:
D62476139

Original commit changeset: 28cfea625721

Original Phabricator Diff: D62476139

fbshipit-source-id: d2302390d9774fb6dee8ac678bb12f488d0fe812
  • Loading branch information
mpsrig authored and facebook-github-bot committed Sep 12, 2024
1 parent 6efe30a commit 78f8111
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 89 deletions.
37 changes: 0 additions & 37 deletions third-party/folly/src/folly/coro/FutureUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@

#pragma once

#include <folly/CancellationToken.h>
#include <folly/experimental/coro/Baton.h>
#include <folly/experimental/coro/Coroutine.h>
#include <folly/experimental/coro/CurrentExecutor.h>
#include <folly/experimental/coro/Invoke.h>
#include <folly/experimental/coro/Task.h>
#include <folly/experimental/coro/Traits.h>
Expand Down Expand Up @@ -51,40 +48,6 @@ inline Task<void> toTask(folly::SemiFuture<Unit> a) {
co_yield co_result(co_await co_awaitTry(std::move(a)));
}

template <typename V>
Task<drop_unit_t<V>> toTaskInterruptOnCancel(folly::SemiFuture<V> sf) {
bool cancelled{false};
Baton baton;
Try<V> result;
auto f = std::move(sf).via(co_await co_current_executor);
f.setCallback_(
[&result, &baton](Executor::KeepAlive<>&&, Try<V>&& t) {
result = std::move(t);
baton.post();
},
// No user logic runs in the callback, we can avoid the cost of switching
// the context.
/* context */ nullptr);

{
CancellationCallback cancelCallback(
co_await co_current_cancellation_token, [&]() noexcept {
cancelled = true;
f.cancel();
});
co_await baton;
}
if (cancelled) {
co_yield co_cancelled;
}
co_yield co_result(std::move(result));
}

template <typename V>
Task<drop_unit_t<V>> toTaskInterruptOnCancel(folly::Future<V> f) {
return toTaskInterruptOnCancel(std::move(f).semi());
}

// Converts the given SemiAwaitable to a SemiFuture (without starting it)
template <typename SemiAwaitable>
folly::SemiFuture<
Expand Down
30 changes: 28 additions & 2 deletions third-party/folly/src/folly/coro/Sleep-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,41 @@

#pragma once

#include <folly/experimental/coro/FutureUtil.h>
#include <folly/CancellationToken.h>
#include <folly/experimental/coro/Baton.h>
#include <folly/experimental/coro/CurrentExecutor.h>

#if FOLLY_HAS_COROUTINES

namespace folly {
namespace coro {

inline Task<void> sleep(HighResDuration d, Timekeeper* tk) {
co_await co_nothrow(toTaskInterruptOnCancel(folly::futures::sleep(d, tk)));
bool cancelled{false};
folly::coro::Baton baton;
Try<Unit> result;
auto future = folly::futures::sleep(d, tk).toUnsafeFuture();
future.setCallback_(
[&result, &baton](Executor::KeepAlive<>&&, Try<Unit>&& t) {
result = std::move(t);
baton.post();
},
// No user logic runs in the callback, we can avoid the cost of switching
// the context.
/* context */ nullptr);

{
CancellationCallback cancelCallback(
co_await co_current_cancellation_token, [&]() noexcept {
cancelled = true;
future.cancel();
});
co_await baton;
}
if (cancelled) {
co_yield co_cancelled;
}
co_yield co_result(std::move(result));
}

inline Task<void> sleepReturnEarlyOnCancel(HighResDuration d, Timekeeper* tk) {
Expand Down
50 changes: 0 additions & 50 deletions third-party/folly/src/folly/coro/test/FutureUtilTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <folly/experimental/coro/AsyncGenerator.h>
#include <folly/experimental/coro/Baton.h>
#include <folly/experimental/coro/BlockingWait.h>
#include <folly/experimental/coro/Collect.h>
#include <folly/experimental/coro/FutureUtil.h>
#include <folly/experimental/coro/Task.h>
#include <folly/portability/GTest.h>
Expand Down Expand Up @@ -114,53 +113,4 @@ TEST(FutureUtilTest, VoidRoundtrip) {
task = folly::coro::toTask(std::move(semi));
folly::coro::blockingWait(std::move(task));
}

TEST(FutureUtilTest, ToTaskInterruptOnCancelFutureWithCancellation) {
auto [p, f] = folly::makePromiseContract<folly::Unit>();

// to verify that cancellation propagates into the future interrupt-handler
folly::exception_wrapper interrupt;
p.setInterruptHandler([&, p_ = &p](auto&& ew) {
interrupt = ew;
p_->setException(std::move(ew));
});

// to verify that deferred work runs
folly::Try<folly::Unit> touched;
auto f1 = std::move(f).defer([&](folly::Try<folly::Unit> t) { touched = t; });
ASSERT_FALSE(touched.tryGetExceptionObject()); // sanity check

// run the scenario within blocking-wait
auto result = folly::coro::blocking_wait(
std::invoke([&, f_ = &f1]() -> folly::coro::Task<folly::Try<void>> {
folly::CancellationSource csource;

co_return std::get<0>(co_await folly::coro::collectAllTry(

folly::coro::co_withCancellation(
csource.getToken(),
// a task that will be cancelled, wrapping a future
std::invoke([&]() -> folly::coro::Task<> {
EXPECT_FALSE(touched.tryGetExceptionObject()); // sanity check
co_await folly::coro::toTaskInterruptOnCancel(std::move(*f_));
})),

// a task that will do the cancelling, after waiting a bit
std::invoke([&]() -> folly::coro::Task<> {
EXPECT_FALSE(touched.tryGetExceptionObject()); // sanity check
co_await folly::coro::co_reschedule_on_current_executor;
EXPECT_FALSE(touched.tryGetExceptionObject()); // sanity check
csource.requestCancellation();
EXPECT_FALSE(touched.tryGetExceptionObject()); // sanity check
co_await folly::coro::co_reschedule_on_current_executor;
EXPECT_TRUE( // sanity check: events happen here
touched.tryGetExceptionObject<folly::FutureCancellation>());
})));
}));

EXPECT_TRUE(touched.tryGetExceptionObject<folly::FutureCancellation>());
EXPECT_TRUE(result.tryGetExceptionObject<folly::OperationCancelled>());
EXPECT_TRUE(interrupt.get_exception<folly::FutureCancellation>());
}

#endif

0 comments on commit 78f8111

Please sign in to comment.