From 72760b0b9978c43360b40dcb35483ae98304065c Mon Sep 17 00:00:00 2001
From: Marco Poletti <poletti.marco@gmail.com>
Date: Wed, 25 May 2016 19:43:48 +0100
Subject: [PATCH] Allow recursive injection (Provider<T>::get() called while
 injecting another class U). Before this commit, recursive injection caused
 undefined behavior. Fixes #16.

---
 .../fixed_size_allocator.defn.h               |  9 ++-
 tests/CMakeLists.txt                          |  1 +
 tests/provider_get_during_injection_ok.cpp    | 66 +++++++++++++++++++
 tests/test_lists.bzl                          |  1 +
 4 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 tests/provider_get_during_injection_ok.cpp

diff --git a/include/fruit/impl/data_structures/fixed_size_allocator.defn.h b/include/fruit/impl/data_structures/fixed_size_allocator.defn.h
index bd26a6a0..552b7cd2 100644
--- a/include/fruit/impl/data_structures/fixed_size_allocator.defn.h
+++ b/include/fruit/impl/data_structures/fixed_size_allocator.defn.h
@@ -88,8 +88,15 @@ FixedSizeAllocator::constructObject(Args&&... args) {
   p += alignof(T) - misalignment;
   assert(std::uintptr_t(p) % alignof(T) == 0);
   T* x = reinterpret_cast<T*>(p);
-  new (x) T(std::forward<Args>(args)...);
   storage_last_used = p + sizeof(T) - 1;
+  
+  // This runs arbitrary code (T's constructor), which might end up calling
+  // constructObject recursively. We must make sure all invariants are satisfied before
+  // calling this.
+  new (x) T(std::forward<Args>(args)...);
+  
+  // We still run this later though, since if T's constructor throws we don't want to
+  // destruct this object in FixedSizeAllocator's destructor.
   if (!std::is_trivially_destructible<T>::value) {
     on_destruction.push_back(
         std::pair<destroy_t, void*>{destroyObject<T>, x});
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index a7acae6c..859cf0a0 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -102,6 +102,7 @@ normalized_component_type_in_injector_not_provided.cpp
 normalized_component_type_in_injector_not_provided_with_annotation.cpp
 normalized_component_type_required_and_provided.cpp
 normalized_component_type_required_and_provided_with_annotation.cpp
+provider_get_during_injection_ok.cpp
 provider_get_never_provided.cpp
 provider_get_not_provided.cpp
 provider_get_ok.cpp
diff --git a/tests/provider_get_during_injection_ok.cpp b/tests/provider_get_during_injection_ok.cpp
new file mode 100644
index 00000000..e99abd6e
--- /dev/null
+++ b/tests/provider_get_during_injection_ok.cpp
@@ -0,0 +1,66 @@
+// expect-success
+/*
+ * Copyright 2014 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <fruit/fruit.h>
+#include "test_macros.h"
+
+using fruit::Injector;
+using fruit::Component;
+using fruit::Provider;
+
+struct X {
+  INJECT(X()) = default;
+  void foo() {
+  }
+};
+
+struct Y {
+  X x;
+  INJECT(Y(Provider<X> xProvider))
+    : x(xProvider.get<X>()) {
+  }
+  
+  void foo() {
+    x.foo();
+  }
+};
+
+struct Z {
+  Y y;
+  INJECT(Z(Provider<Y> yProvider))
+  : y(yProvider.get<Y>()) {
+    
+  }
+  
+  void foo() {
+    y.foo();
+  }
+};
+
+Component<Z> getZComponent() {
+  return fruit::createComponent();
+}
+
+int main() {
+  Injector<Z> injector(getZComponent());
+  Provider<Z> provider(injector);
+  // During provider.get<Z>(), yProvider.get() is called, and during that xProvider.get()
+  // is called.
+  Z z = provider.get<Z>();
+  z.foo();
+  return 0;
+}
diff --git a/tests/test_lists.bzl b/tests/test_lists.bzl
index c315d707..8698ea2e 100644
--- a/tests/test_lists.bzl
+++ b/tests/test_lists.bzl
@@ -46,6 +46,7 @@ TEST_LISTS = {
         "normalized_component_repeated_type_with_different_annotation",
         "normalized_component_successful",
         "normalized_component_successful_with_annotations",
+        "provider_get_during_injection_ok",
         "provider_get_ok",
         "provider_get_ok_with_annotation",
         "register_constructor",