BatchDispatcher exception handling

Summary: error handling for batch dispatcher, for the case where dispatch function throws.

Reviewed By: yfeldblum

Differential Revision: D4016246

fbshipit-source-id: ac61b873a425128be7839a119591fca10608210a

diff --git a/folly/fibers/BatchDispatcher.h b/folly/fibers/BatchDispatcher.h
index dada4275165b69e8559ead78111f7efb22332622..65940ea66460ce8cf4c0c3d989d12aeb6add8ca1 100644 (file)
--- a/folly/fibers/BatchDispatcher.h
+++ b/folly/fibers/BatchDispatcher.h
@@ -105,13 +105,25 @@ class BatchDispatcher {
     state.values.swap(values);
     state.promises.swap(promises);
 
-    auto results = state.dispatchFunc(std::move(values));
-    if (results.size() != promises.size()) {
-      throw std::logic_error(
-          "Unexpected number of results returned from dispatch function");
-    }
-    for (size_t i = 0; i < promises.size(); i++) {
-      promises[i].setValue(std::move(results[i]));
+    try {
+      auto results = state.dispatchFunc(std::move(values));
+      if (results.size() != promises.size()) {
+        throw std::logic_error(
+            "Unexpected number of results returned from dispatch function");
+      }
+
+      for (size_t i = 0; i < promises.size(); i++) {
+        promises[i].setValue(std::move(results[i]));
+      }
+    } catch (const std::exception& ex) {
+      for (size_t i = 0; i < promises.size(); i++) {
+        promises[i].setException(
+            exception_wrapper(std::current_exception(), ex));
+      }
+    } catch (...) {
+      for (size_t i = 0; i < promises.size(); i++) {
+        promises[i].setException(exception_wrapper(std::current_exception()));
+      }
     }
   }
 

diff --git a/folly/fibers/test/FibersTest.cpp b/folly/fibers/test/FibersTest.cpp
index f5218dd285dbf6a444c1265755dbd05ef4e447da..eeee0b2e9ed8e38778ebce07a7fc8a25925984be 100644 (file)
--- a/folly/fibers/test/FibersTest.cpp
+++ b/folly/fibers/test/FibersTest.cpp
@@ -1728,6 +1728,31 @@ TEST(FiberManager, doubleBatchDispatchTest) {
   evb.loop();
 }
 
+template <typename ExecutorT>
+void batchDispatchExceptionHandling(ExecutorT& executor, int i) {
+  thread_local BatchDispatcher<int, int, ExecutorT> batchDispatcher(
+      executor, [=, &executor](std::vector<int> &&) -> std::vector<int> {
+        throw std::runtime_error("Surprise!!");
+      });
+
+  EXPECT_THROW(batchDispatcher.add(i).get(), std::runtime_error);
+}
+
+TEST(FiberManager, batchDispatchExceptionHandlingTest) {
+  folly::EventBase evb;
+  auto& executor = getFiberManager(evb);
+
+  // Launch multiple fibers with a single id.
+  executor.add([&]() {
+    int totalNumberOfElements = 5;
+    for (int i = 0; i < totalNumberOfElements; i++) {
+      executor.add(
+          [=, &executor]() { batchDispatchExceptionHandling(executor, i); });
+    }
+  });
+  evb.loop();
+}
+
 /**
  * Test that we can properly track fiber stack usage.
  *