Future: improve test with task discarding executors

Summary:
We have tests that check that the Future implementation deals
cleanly with executors discarding tasks. The existing tests destroy
the tasks immediately when they are passed to Executor::add. This
diff adds corresponding tests for the scenario where the task is
not destroyed right away, but after the call to Future::then has
completed.

This diff also adds a mechanism to detect that the passed callback
function is actually destroyed. We have tested already that the
promise returned by folly::then will be set to a BrokenPromise
exception when the executor discards the task. However, the task
passed to the executor is not only the callback we pass to
folly::then, as the Future implementation wraps it with some code
that stores the return value in a Promise. Existing tests check
that this Promise is destroyed. The added mechanism in this diff
checks that the orignal callback function itself gets destroyed.

Reviewed By: Krigpl

Differential Revision: D5002100

fbshipit-source-id: 4155f61b075d9fe8d1869ad229f4d350571ff4c6

diff --git a/folly/futures/test/ViaTest.cpp b/folly/futures/test/ViaTest.cpp
index bbbd96a27e7105439e2c7bd5ea2f4ad2bcce6a1f..2bdabfc04974cd167dd35af60f03a58704d8d6f5 100644 (file)
--- a/folly/futures/test/ViaTest.cpp
+++ b/folly/futures/test/ViaTest.cpp
@@ -473,19 +473,83 @@ TEST(Via, viaRaces) {
 }
 
 TEST(Via, viaDummyExecutorFutureSetValueFirst) {
 }
 
 TEST(Via, viaDummyExecutorFutureSetValueFirst) {

+  // The callback object will get destroyed when passed to the executor.
+
+  // A promise will be captured by the callback lambda so we can observe that
+  // it will be destroyed.
+  Promise<Unit> captured_promise;
+  auto captured_promise_future = captured_promise.getFuture();
+

   DummyDrivableExecutor x;
   DummyDrivableExecutor x;

-  auto future = makeFuture().via(&x).then([] { return 42; });
+  auto future = makeFuture().via(&x).then(
+      [c = std::move(captured_promise)] { return 42; });

 
 

-  EXPECT_THROW(future.get(), BrokenPromise);
+  EXPECT_THROW(future.get(std::chrono::seconds(5)), BrokenPromise);
+  EXPECT_THROW(
+      captured_promise_future.get(std::chrono::seconds(5)), BrokenPromise);

 }
 
 TEST(Via, viaDummyExecutorFutureSetCallbackFirst) {
 }
 
 TEST(Via, viaDummyExecutorFutureSetCallbackFirst) {

+  // The callback object will get destroyed when passed to the executor.
+
+  // A promise will be captured by the callback lambda so we can observe that
+  // it will be destroyed.
+  Promise<Unit> captured_promise;
+  auto captured_promise_future = captured_promise.getFuture();
+

   DummyDrivableExecutor x;
   Promise<Unit> trigger;
   DummyDrivableExecutor x;
   Promise<Unit> trigger;

-  auto future = trigger.getFuture().via(&x).then([] { return 42; });
+  auto future = trigger.getFuture().via(&x).then(
+      [c = std::move(captured_promise)] { return 42; });

   trigger.setValue();
 
   trigger.setValue();
 

-  EXPECT_THROW(future.get(), BrokenPromise);
+  EXPECT_THROW(future.get(std::chrono::seconds(5)), BrokenPromise);
+  EXPECT_THROW(
+      captured_promise_future.get(std::chrono::seconds(5)), BrokenPromise);
+}
+
+TEST(Via, viaExecutorDiscardsTaskFutureSetValueFirst) {
+  // The callback object will get destroyed when the ManualExecutor runs out
+  // of scope.
+
+  // A promise will be captured by the callback lambda so we can observe that
+  // it will be destroyed.
+  Promise<Unit> captured_promise;
+  auto captured_promise_future = captured_promise.getFuture();
+
+  Optional<Future<int>> future;
+  {
+    ManualExecutor x;
+    future = makeFuture().via(&x).then(
+        [c = std::move(captured_promise)] { return 42; });
+  }
+
+  EXPECT_THROW(future->get(std::chrono::seconds(5)), BrokenPromise);
+  EXPECT_THROW(
+      captured_promise_future.get(std::chrono::seconds(5)), BrokenPromise);
+}
+
+TEST(Via, viaExecutorDiscardsTaskFutureSetCallbackFirst) {
+  // The callback object will get destroyed when the ManualExecutor runs out
+  // of scope.
+
+  // A promise will be captured by the callback lambda so we can observe that
+  // it will be destroyed.
+  Promise<Unit> captured_promise;
+  auto captured_promise_future = captured_promise.getFuture();
+
+  Optional<Future<int>> future;
+  {
+    ManualExecutor x;
+    Promise<Unit> trigger;
+    future = trigger.getFuture().via(&x).then(
+        [c = std::move(captured_promise)] { return 42; });
+    trigger.setValue();
+  }
+
+  EXPECT_THROW(future->get(std::chrono::seconds(5)), BrokenPromise);
+  EXPECT_THROW(
+      captured_promise_future.get(std::chrono::seconds(5)), BrokenPromise);

 }
 
 TEST(ViaFunc, liftsVoid) {
 }
 
 TEST(ViaFunc, liftsVoid) {