Add folly::tryEmplaceWith() helper for safely initialising a Try<T> with...

Add folly::tryEmplaceWith() helper for safely initialising a Try<T> with result of calling a function

Summary:
Adds a new folly::tryEmplaceWith() function that is the equivalent of makeTryWith() but for in-place initialisation of an existing Try<T> object.

Using `tryEmplaceWith(t, func)` rather than `t = makeTryWith(func)` can potentially be more exception safe since the latter could potentially still throw if `Try<T>::operator=()` throws (ie. if T's move-constructor can throw).

Updated FiberManager's internals to make use of this to fix a potential exception-safety issue that could have caused a memory leak if the Try<T> move-constructor of the result of fiber threw an exception.

Reviewed By: yfeldblum

Differential Revision: D9511067

fbshipit-source-id: 827cc88ea1d4fcd505c8d8a9886b87b26db90544

folly/Try-inl.h

@@ -17,6 +17,7 @@
 #pragma once
 
 #include <folly/Utility.h>
+#include <folly/functional/Invoke.h>
 
 #include <stdexcept>
 #include <tuple>
@@ -261,6 +262,40 @@ void tryEmplace(Try<void>& t) noexcept {
   t.emplace();
 }
 
+template <typename T, typename Func>
+T* tryEmplaceWith(Try<T>& t, Func&& func) noexcept {
+  static_assert(
+      std::is_constructible<T, folly::invoke_result_t<Func>>::value,
+      "Unable to initialise a value of type T with the result of 'func'");
+  try {
+    return std::addressof(t.emplace(static_cast<Func&&>(func)()));
+  } catch (const std::exception& ex) {
+    t.emplaceException(std::current_exception(), ex);
+    return nullptr;
+  } catch (...) {
+    t.emplaceException(std::current_exception());
+    return nullptr;
+  }
+}
+
+template <typename Func>
+bool tryEmplaceWith(Try<void>& t, Func&& func) noexcept {
+  static_assert(
+      std::is_void<folly::invoke_result_t<Func>>::value,
+      "Func returns non-void. Cannot be used to emplace Try<void>");
+  try {
+    static_cast<Func&&>(func)();
+    t.emplace();
+    return true;
+  } catch (const std::exception& ex) {
+    t.emplaceException(std::current_exception(), ex);
+    return false;
+  } catch (...) {
+    t.emplaceException(std::current_exception());
+    return false;
+  }
+}
+
 namespace try_detail {
 
 /**

folly/Try.h

@@ -609,6 +609,37 @@ T* tryEmplace(Try<T>& t, Args&&... args) noexcept;
  */
 inline void tryEmplace(Try<void>& t) noexcept;
 
+/*
+ * Try to in-place construct a new value from the result of a function.
+ *
+ * If the function completes successfully then attempts to in-place construct
+ * a value of type, T, passing the result of the function as the only parameter.
+ *
+ * If either the call to the function completes with an exception or the
+ * constructor completes with an exception then the exception is caught and
+ * stored in the Try object.
+ *
+ * @returns A pointer to the newly constructed object if it completed
+ * successfully, otherwise returns nullptr if the operation completed with
+ * an exception.
+ */
+template <typename T, typename Func>
+T* tryEmplaceWith(Try<T>& t, Func&& func) noexcept;
+
+/*
+ * Specialization of tryEmplaceWith() for Try<void>.
+ *
+ * Calls func() and if it doesn't throw an exception then calls t.emplace().
+ * If func() throws then captures the exception in t using t.emplaceException().
+ *
+ * Func must be callable with zero parameters and must return void.
+ *
+ * @returns true if func() completed without an exception, false if func()
+ * threw an exception.
+ */
+template <typename Func>
+bool tryEmplaceWith(Try<void>& t, Func&& func) noexcept;
+
 /**
  * Tuple<Try<Type>...> -> std::tuple<Type...>
  *

folly/fibers/FiberManagerInternal-inl.h

@@ -370,7 +370,7 @@ struct FiberManager::AddTaskFinallyHelper {
 
     void operator()() {
       try {
-        finally_(std::move(*result_));
+        finally_(std::move(result_));
       } catch (...) {
         fm_.exceptionCallback_(
             std::current_exception(), "running Finally functor");
@@ -387,7 +387,7 @@ struct FiberManager::AddTaskFinallyHelper {
     friend class Func;
 
     G finally_;
-    folly::Optional<folly::Try<Result>> result_;
+    folly::Try<Result> result_;
     FiberManager& fm_;
   };
 
@@ -397,7 +397,7 @@ struct FiberManager::AddTaskFinallyHelper {
         : func_(std::move(func)), result_(finally.result_) {}
 
     void operator()() {
-      result_ = folly::makeTryWith(std::move(func_));
+      folly::tryEmplaceWith(result_, std::move(func_));
 
       if (allocateInBuffer) {
         this->~Func();
@@ -408,7 +408,7 @@ struct FiberManager::AddTaskFinallyHelper {
 
    private:
     F func_;
-    folly::Optional<folly::Try<Result>>& result_;
+    folly::Try<Result>& result_;
   };
 
   static constexpr bool allocateInBuffer =
@@ -473,7 +473,7 @@ invoke_result_t<F> FiberManager::runInMainContext(F&& func) {
 
   folly::Try<Result> result;
   auto f = [&func, &result]() mutable {
-    result = folly::makeTryWith(std::forward<F>(func));
+    folly::tryEmplaceWith(result, std::forward<F>(func));
   };
 
   immediateFunc_ = std::ref(f);

folly/test/TryTest.cpp

@@ -286,6 +286,52 @@ TEST(Try, tryEmplaceVoidTry) {
   EXPECT_FALSE(t.hasException());
 }
 
+TEST(Try, tryEmplaceWith) {
+  Try<std::string> t;
+  tryEmplaceWith(t, [] { return "hello"; });
+  EXPECT_EQ("hello", t.value());
+}
+
+TEST(Try, tryEmplaceWithFunctionThrows) {
+  struct MyException : std::exception {};
+  Try<int> t;
+  tryEmplaceWith(t, []() -> int { throw MyException{}; });
+  EXPECT_TRUE(t.hasException());
+  EXPECT_TRUE(t.hasException<MyException>());
+}
+
+TEST(Try, tryEmplaceWithConstructorThrows) {
+  struct MyException : std::exception {};
+  struct ThrowingConstructor {
+    int value_;
+    explicit ThrowingConstructor(bool shouldThrow) noexcept(false) : value_(0) {
+      if (shouldThrow) {
+        throw MyException{};
+      }
+    }
+  };
+
+  Try<ThrowingConstructor> t;
+  tryEmplaceWith(t, [] { return false; });
+  EXPECT_TRUE(t.hasValue());
+  tryEmplaceWith(t, [] { return true; });
+  EXPECT_TRUE(t.hasException());
+  EXPECT_TRUE(t.hasException<MyException>());
+}
+
+TEST(Try, tryEmplaceWithVoidTry) {
+  Try<void> t;
+  bool hasRun = false;
+  tryEmplaceWith(t, [&] { hasRun = true; });
+  EXPECT_TRUE(t.hasValue());
+  EXPECT_TRUE(hasRun);
+
+  struct MyException : std::exception {};
+  tryEmplaceWith(t, [&] { throw MyException{}; });
+  EXPECT_TRUE(t.hasException());
+  EXPECT_TRUE(t.hasException<MyException>());
+}
+
 TEST(Try, nothrow) {
   using F = HasCtors<false>;
   using T = HasCtors<true>;