Revert D62476139: coro toTaskInterruptOnCancel

Differential Revision: D62476139 Original commit changeset: 28cfea625721 Original Phabricator Diff: D62476139 fbshipit-source-id: d2302390d9774fb6dee8ac678bb12f488d0fe812
facebook · Sep 12, 2024 · 6b3f03b · 6b3f03b
1 parent 0efca4e
commit 6b3f03b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 93 deletions.
diff --git a/folly/coro/BUCK b/folly/coro/BUCK
@@ -283,10 +283,7 @@ cpp_library(
     name = "future_util",
     headers = ["FutureUtil.h"],
     exported_deps = [
-        "//folly:cancellation_token",
-        "//folly/experimental/coro:baton",
         "//folly/experimental/coro:coroutine",
-        "//folly/experimental/coro:current_executor",
         "//folly/experimental/coro:detail_traits",
         "//folly/experimental/coro:invoke",
         "//folly/experimental/coro:task",
@@ -489,8 +486,10 @@ cpp_library(
         "Sleep-inl.h",
     ],
     exported_deps = [
+        "//folly:cancellation_token",
+        "//folly/experimental/coro:baton",
         "//folly/experimental/coro:coroutine",
-        "//folly/experimental/coro:future_util",
+        "//folly/experimental/coro:current_executor",
         "//folly/experimental/coro:task",
         "//folly/futures:core",
     ],

diff --git a/folly/coro/FutureUtil.h b/folly/coro/FutureUtil.h
@@ -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>
@@ -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<

diff --git a/folly/coro/Sleep-inl.h b/folly/coro/Sleep-inl.h
@@ -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) {

diff --git a/folly/coro/test/FutureUtilTest.cpp b/folly/coro/test/FutureUtilTest.cpp
@@ -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>
@@ -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