coder137 · coder137 · Apr 22, 2022 · Apr 22, 2022 · Apr 22, 2022 · Apr 22, 2022
diff --git a/buildcc/lib/env/CMakeLists.txt b/buildcc/lib/env/CMakeLists.txt
@@ -42,19 +42,22 @@ if (${TESTING})
     add_executable(test_storage test/test_storage.cpp)
     target_link_libraries(test_storage PRIVATE mock_env)
 
+    add_executable(test_assert_fatal test/test_assert_fatal.cpp)
+    target_link_libraries(test_assert_fatal PRIVATE mock_env)
+
     add_test(NAME test_static_project COMMAND test_static_project)
     add_test(NAME test_env_util COMMAND test_env_util)
     add_test(NAME test_task_state COMMAND test_task_state)
     add_test(NAME test_command COMMAND test_command)
     add_test(NAME test_storage COMMAND test_storage)
+    add_test(NAME test_assert_fatal COMMAND test_assert_fatal)
 endif()
 
 set(ENV_SRCS
     src/env.cpp
     src/assert_fatal.cpp
     src/logging.cpp
     include/env/assert_fatal.h
-    include/env/assert_throw.h
     include/env/env.h
     include/env/logging.h
     include/env/util.h

diff --git a/buildcc/lib/env/include/env/assert_fatal.h b/buildcc/lib/env/include/env/assert_fatal.h
@@ -24,7 +24,11 @@
 namespace buildcc::env {
 
 /**
- * @brief During Release -> std::terminate
+ * @brief During Release ->
+ * NOT THREADED : std::exit
+ * THREADED : throw std::exception (it is wrong to exit
+ * when in a threaded state. We want to handle the exception and gracefully
+ * exit)
  * During Unit Test -> throw std::exception
  */
 [[noreturn]] void assert_handle_fatal();

diff --git a/buildcc/lib/env/include/env/assert_throw.h b/buildcc/lib/env/include/env/assert_throw.h
diff --git a/buildcc/lib/env/include/env/storage.h b/buildcc/lib/env/include/env/storage.h
@@ -104,19 +104,21 @@ class Storage {
 
   template <typename T, typename... Params>
   static T &Add(const std::string &identifier, Params &&...params) {
-    return internal_->Add<T, Params...>(identifier,
-                                        std::forward<Params>(params)...);
+    return Ref().Add<T, Params...>(identifier, std::forward<Params>(params)...);
   }
 
   template <typename T>
   static const T &ConstRef(const std::string &identifier) {
-    return internal_->ConstRef<T>(identifier);
+    return Ref().ConstRef<T>(identifier);
   }
 
   template <typename T> static T &Ref(const std::string &identifier) {
-    return internal_->Ref<T>(identifier);
+    return Ref().Ref<T>(identifier);
   }
 
+private:
+  static ScopedStorage &Ref();
+
 private:
   static std::unique_ptr<ScopedStorage> internal_;
 };

diff --git a/buildcc/lib/env/src/assert_fatal.cpp b/buildcc/lib/env/src/assert_fatal.cpp
@@ -17,9 +17,30 @@
 #include "env/assert_fatal.h"
 
 #include <exception>
+#include <thread>
+
+namespace {
+
+std::thread::id main_id = std::this_thread::get_id();
+
+bool IsRunningOnThread() {
+  bool threaded = true;
+  if (std::this_thread::get_id() == main_id) {
+    threaded = false;
+  }
+  return threaded;
+}
+
+} // namespace
 
 namespace buildcc::env {
 
-[[noreturn]] void assert_handle_fatal() { std::terminate(); }
+[[noreturn]] void assert_handle_fatal() {
+  if (!IsRunningOnThread()) {
+    std::exit(1);
+  } else {
+    throw std::exception();
+  }
+}
 
 } // namespace buildcc::env
diff --git a/buildcc/lib/env/src/storage.cpp b/buildcc/lib/env/src/storage.cpp
@@ -20,4 +20,10 @@ namespace buildcc {
 
 std::unique_ptr<ScopedStorage> Storage::internal_;
 
+ScopedStorage &Storage::Ref() {
+  env::assert_fatal(internal_ != nullptr,
+                    "Initialize Storage using the Storage::Init API");
+  return *internal_;
 }
+
+} // namespace buildcc
diff --git a/buildcc/lib/env/test/test_assert_fatal.cpp b/buildcc/lib/env/test/test_assert_fatal.cpp
@@ -0,0 +1,72 @@
+#include <thread>
+
+#include "taskflow/taskflow.hpp"
+
+// NOTE, Make sure all these includes are AFTER the system and header includes
+#include "CppUTest/CommandLineTestRunner.h"
+#include "CppUTest/MemoryLeakDetectorNewMacros.h"
+#include "CppUTest/TestHarness.h"
+#include "CppUTest/Utest.h"
+#include "CppUTestExt/MockSupport.h"
+
+namespace {
+
+std::thread::id my_main_thread = std::this_thread::get_id();
+
+bool IsRunningInThread() {
+  bool threaded = true;
+  if (std::this_thread::get_id() == my_main_thread) {
+    threaded = false;
+  }
+  return threaded;
+}
+
+void assert_handle_fatal() {
+  if (IsRunningInThread()) {
+    mock().actualCall("assert_handle_fatal_threaded");
+  } else {
+    mock().actualCall("assert_handle_fatal_main");
+  }
+}
+
+} // namespace
+
+// clang-format off
+TEST_GROUP(AssertFatalTestGroup)
+{
+  void teardown() {
+      mock().clear();
+  }
+};
+// clang-format on
+
+TEST(AssertFatalTestGroup, AssertFatal_IsThreadedCheck) {
+  CHECK_FALSE(IsRunningInThread());
+
+  tf::Taskflow tf;
+  tf.emplace([]() { CHECK_TRUE(IsRunningInThread()); });
+
+  tf::Executor ex(1);
+  ex.run(tf);
+  ex.wait_for_all();
+}
+
+TEST(AssertFatalTestGroup, AssertFatal_Threaded) {
+  mock().expectOneCall("assert_handle_fatal_threaded");
+
+  tf::Taskflow tf;
+  tf.emplace([]() { assert_handle_fatal(); });
+
+  tf::Executor ex(1);
+  ex.run(tf);
+  ex.wait_for_all();
+}
+
+TEST(AssertFatalTestGroup, AssertFatal_NotThreaded) {
+  mock().expectOneCall("assert_handle_fatal_main");
+  assert_handle_fatal();
+}
+
+int main(int ac, char **av) {
+  return CommandLineTestRunner::RunAllTests(ac, av);
+}
diff --git a/buildcc/lib/env/test/test_storage.cpp b/buildcc/lib/env/test/test_storage.cpp
@@ -86,6 +86,14 @@ TEST(StorageTestGroup, BasicUsage) {
   STRCMP_EQUAL(bigobj2.c_str(), "name2");
 }
 
+TEST(StorageTestGroup, UsageWithoutInit) {
+  buildcc::Storage::Deinit();
+
+  CHECK_THROWS(std::exception, buildcc::Storage::Add<int>("integer"));
+  CHECK_THROWS(std::exception, buildcc::Storage::Ref<int>("integer"));
+  CHECK_THROWS(std::exception, buildcc::Storage::ConstRef<int>("integer"));
+}
+
 int main(int ac, char **av) {
   return CommandLineTestRunner::RunAllTests(ac, av);
 }
diff --git a/buildcc/lib/target/src/generator/task.cpp b/buildcc/lib/target/src/generator/task.cpp
@@ -52,7 +52,7 @@ void Generator::GenerateTask() {
     auto run_command = [this](const std::string &command) {
       try {
         bool success = env::Command::Execute(command);
-        env::assert_throw(success, fmt::format("{} failed", command));
+        env::assert_fatal(success, fmt::format("{} failed", command));
       } catch (...) {
         std::lock_guard<std::mutex> guard(task_state_mutex_);
         task_state_ = env::TaskState::FAILURE;
@@ -104,7 +104,7 @@ void Generator::GenerateTask() {
       if (dirty_) {
         try {
           serialization_.UpdateStore(user_);
-          env::assert_throw(serialization_.StoreToFile(),
+          env::assert_fatal(serialization_.StoreToFile(),
                             fmt::format("Store failed for {}", name_));
         } catch (...) {
           task_state_ = env::TaskState::FAILURE;

diff --git a/buildcc/lib/target/src/target/friend/compile_pch.cpp b/buildcc/lib/target/src/target/friend/compile_pch.cpp
@@ -54,7 +54,7 @@ void AggregateToFile(const fs::path &filename,
                });
   bool success = buildcc::env::save_file(
       buildcc::path_as_string(filename).c_str(), constructed_output, false);
-  buildcc::env::assert_throw(success, "Could not save pch file");
+  buildcc::env::assert_fatal(success, "Could not save pch file");
 }
 
 } // namespace
@@ -108,10 +108,10 @@ void CompilePch::BuildCompile() {
       const std::string p = fmt::format("{}", source_path_);
       const bool save =
           env::save_file(p.c_str(), {"//Generated by BuildCC"}, false);
-      env::assert_throw(save, fmt::format("Could not save {}", p));
+      env::assert_fatal(save, fmt::format("Could not save {}", p));
     }
     bool success = env::Command::Execute(command_);
-    env::assert_throw(success, "Failed to compile pch");
+    env::assert_fatal(success, "Failed to compile pch");
   }
 }
 

diff --git a/buildcc/lib/target/src/target/friend/link_target.cpp b/buildcc/lib/target/src/target/friend/link_target.cpp
@@ -98,7 +98,7 @@ void LinkTarget::BuildLink() {
 
   if (target_.dirty_) {
     bool success = env::Command::Execute(command_);
-    env::assert_throw(success, "Failed to link target");
+    env::assert_fatal(success, "Failed to link target");
     target_.serialization_.UpdateTargetCompiled();
   }
 }

diff --git a/buildcc/lib/target/src/target/tasks.cpp b/buildcc/lib/target/src/target/tasks.cpp
@@ -124,7 +124,7 @@ void CompileObject::Task() {
               try {
                 bool success = env::Command::Execute(
                     GetObjectData(s.GetPathname()).command);
-                env::assert_throw(success, "Could not compile source");
+                env::assert_fatal(success, "Could not compile source");
                 target_.serialization_.AddSource(s);
               } catch (...) {
                 target_.SetTaskStateFailure();
@@ -170,7 +170,7 @@ void Target::EndTask() {
     if (dirty_) {
       try {
         serialization_.UpdateStore(user_);
-        env::assert_throw(serialization_.StoreToFile(),
+        env::assert_fatal(serialization_.StoreToFile(),
                           fmt::format("Store failed for {}", GetName()));
         state_.BuildCompleted();
       } catch (...) {

diff --git a/buildcc/schema/include/schema/path.h b/buildcc/schema/include/schema/path.h
@@ -25,7 +25,7 @@
 #include <unordered_set>
 
 // Env
-#include "env/assert_throw.h"
+#include "env/assert_fatal.h"
 
 // Third party
 #include "fmt/format.h"
@@ -51,7 +51,7 @@ class Path {
         std::filesystem::last_write_time(pathname, errcode)
             .time_since_epoch()
             .count();
-    env::assert_throw(errcode.value() == 0,
+    env::assert_fatal(errcode.value() == 0,
                       fmt::format("{} not found", pathname));
 
     return Path(pathname, last_write_timestamp);