tighten control on Core creation/deletion

Summary:
Enforce two aspects of folly::futures::Core<T>:

- that it is heap-constructed (prevent clients from constructing it other than via `new`)
- that it cannot be deleted early (prevent clients from explicitly deleting it)

Reviewed By: yfeldblum

Differential Revision: D7900761

fbshipit-source-id: c1c5a149e3d96c4cea04c17909c2ffd42383973e

folly/futures/Future-inl.h

@@ -142,13 +142,13 @@ FutureBase<T>::FutureBase(Future<T>&& other) noexcept : core_(other.core_) {
 template <class T>
 template <class T2, typename>
 FutureBase<T>::FutureBase(T2&& val)
-    : core_(new futures::detail::Core<T>(Try<T>(std::forward<T2>(val)))) {}
+    : core_(CoreType::make(Try<T>(std::forward<T2>(val)))) {}
 
 template <class T>
 template <typename T2>
 FutureBase<T>::FutureBase(
     typename std::enable_if<std::is_same<Unit, T2>::value>::type*)
-    : core_(new futures::detail::Core<T>(Try<T>(T()))) {}
+    : core_(CoreType::make(Try<T>(T()))) {}
 
 template <class T>
 template <
@@ -156,9 +156,7 @@ template <
     typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
         type>
 FutureBase<T>::FutureBase(in_place_t, Args&&... args)
-    : core_(
-          new futures::detail::Core<T>(in_place, std::forward<Args>(args)...)) {
-}
+    : core_(CoreType::make(in_place, std::forward<Args>(args)...)) {}
 
 template <class T>
 void FutureBase<T>::assign(FutureBase<T>&& other) noexcept {
@@ -622,7 +620,7 @@ typename std::
 
 template <class T>
 SemiFuture<T> makeSemiFuture(Try<T>&& t) {
-  return SemiFuture<T>(new futures::detail::Core<T>(std::move(t)));
+  return SemiFuture<T>(SemiFuture<T>::CoreType::make(std::move(t)));
 }
 
 // This must be defined after the constructors to avoid a bug in MSVC
@@ -1160,7 +1158,7 @@ makeFuture(E const& e) {
 
 template <class T>
 Future<T> makeFuture(Try<T>&& t) {
-  return Future<T>(new futures::detail::Core<T>(std::move(t)));
+  return Future<T>(Future<T>::CoreType::make(std::move(t)));
 }
 
 // via

folly/futures/Promise-inl.h

@@ -31,8 +31,7 @@ Promise<T> Promise<T>::makeEmpty() noexcept {
 }
 
 template <class T>
-Promise<T>::Promise()
-    : retrieved_(false), core_(new futures::detail::Core<T>()) {}
+Promise<T>::Promise() : retrieved_(false), core_(CoreType::make()) {}
 
 template <class T>
 Promise<T>::Promise(Promise<T>&& other) noexcept

folly/futures/detail/Core.h

@@ -92,25 +92,17 @@ class Core final {
   static_assert(!std::is_void<T>::value,
                 "void futures are not supported. Use Unit instead.");
  public:
-  /// This must be heap-constructed. There's probably a way to enforce that in
-  /// code but since this is just internal detail code and I don't know how
-  /// off-hand, I'm punting.
-  Core() : result_(), fsm_(State::Start), attached_(2) {}
+  static Core* make() {
+    return new Core();
+  }
 
-  explicit Core(Try<T>&& t)
-    : result_(std::move(t)),
-      fsm_(State::OnlyResult),
-      attached_(1) {}
+  static Core* make(Try<T>&& t) {
+    return new Core(std::move(t));
+  }
 
   template <typename... Args>
-  explicit Core(in_place_t, Args&&... args) noexcept(
-      std::is_nothrow_constructible<T, Args&&...>::value)
-      : result_(in_place, in_place, std::forward<Args>(args)...),
-        fsm_(State::OnlyResult),
-        attached_(1) {}
-
-  ~Core() {
-    DCHECK(attached_ == 0);
+  static Core<T>* make(in_place_t, Args&&... args) {
+    return new Core<T>(in_place, std::forward<Args>(args)...);
   }
 
   // not copyable
@@ -276,6 +268,22 @@ class Core final {
   }
 
  private:
+  Core() : result_(), fsm_(State::Start), attached_(2) {}
+
+  explicit Core(Try<T>&& t)
+      : result_(std::move(t)), fsm_(State::OnlyResult), attached_(1) {}
+
+  template <typename... Args>
+  explicit Core(in_place_t, Args&&... args) noexcept(
+      std::is_nothrow_constructible<T, Args&&...>::value)
+      : result_(in_place, in_place, std::forward<Args>(args)...),
+        fsm_(State::OnlyResult),
+        attached_(1) {}
+
+  ~Core() {
+    DCHECK(attached_ == 0);
+  }
+
   // Helper class that stores a pointer to the `Core` object and calls
   // `derefCallback` and `detachOne` in the destructor.
   class CoreAndCallbackReference {