Skip to content

Commit e41bf00

Browse files
coder137coder137
authored
Assert Fatal thread behavior (#206)
1 parent 69b0943 commit e41bf00

File tree

13 files changed

+133
-86
lines changed

13 files changed

+133
-86
lines changed

‎buildcc/lib/env/CMakeLists.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,22 @@ if (${TESTING})
4242
add_executable(test_storage test/test_storage.cpp)
4343
target_link_libraries(test_storage PRIVATE mock_env)
4444

45+
add_executable(test_assert_fatal test/test_assert_fatal.cpp)
46+
target_link_libraries(test_assert_fatal PRIVATE mock_env)
47+
4548
add_test(NAME test_static_project COMMAND test_static_project)
4649
add_test(NAME test_env_util COMMAND test_env_util)
4750
add_test(NAME test_task_state COMMAND test_task_state)
4851
add_test(NAME test_command COMMAND test_command)
4952
add_test(NAME test_storage COMMAND test_storage)
53+
add_test(NAME test_assert_fatal COMMAND test_assert_fatal)
5054
endif()
5155

5256
set(ENV_SRCS
5357
src/env.cpp
5458
src/assert_fatal.cpp
5559
src/logging.cpp
5660
include/env/assert_fatal.h
57-
include/env/assert_throw.h
5861
include/env/env.h
5962
include/env/logging.h
6063
include/env/util.h

‎buildcc/lib/env/include/env/assert_fatal.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@
2424
namespace buildcc::env {
2525

2626
/**
27-
* @brief During Release -> std::terminate
27+
* @brief During Release ->
28+
* NOT THREADED : std::exit
29+
* THREADED : throw std::exception (it is wrong to exit
30+
* when in a threaded state. We want to handle the exception and gracefully
31+
* exit)
2832
* During Unit Test -> throw std::exception
2933
*/
3034
[[noreturn]] void assert_handle_fatal();

‎buildcc/lib/env/include/env/assert_throw.h

Lines changed: 0 additions & 69 deletions
This file was deleted.

‎buildcc/lib/env/include/env/storage.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,19 +104,21 @@ class Storage {
104104

105105
template <typename T, typename... Params>
106106
static T &Add(const std::string &identifier, Params &&...params) {
107-
return internal_->Add<T, Params...>(identifier,
108-
std::forward<Params>(params)...);
107+
return Ref().Add<T, Params...>(identifier, std::forward<Params>(params)...);
109108
}
110109

111110
template <typename T>
112111
static const T &ConstRef(const std::string &identifier) {
113-
return internal_->ConstRef<T>(identifier);
112+
return Ref().ConstRef<T>(identifier);
114113
}
115114

116115
template <typename T> static T &Ref(const std::string &identifier) {
117-
return internal_->Ref<T>(identifier);
116+
return Ref().Ref<T>(identifier);
118117
}
119118

119+
private:
120+
static ScopedStorage &Ref();
121+
120122
private:
121123
static std::unique_ptr<ScopedStorage> internal_;
122124
};

‎buildcc/lib/env/src/assert_fatal.cpp

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,30 @@
1717
#include "env/assert_fatal.h"
1818

1919
#include <exception>
20+
#include <thread>
21+
22+
namespace {
23+
24+
std::thread::id main_id = std::this_thread::get_id();
25+
26+
bool IsRunningOnThread() {
27+
bool threaded = true;
28+
if (std::this_thread::get_id() == main_id) {
29+
threaded = false;
30+
}
31+
return threaded;
32+
}
33+
34+
} // namespace
2035

2136
namespace buildcc::env {
2237

23-
[[noreturn]] void assert_handle_fatal() { std::terminate(); }
38+
[[noreturn]] void assert_handle_fatal() {
39+
if (!IsRunningOnThread()) {
40+
std::exit(1);
41+
} else {
42+
throw std::exception();
43+
}
44+
}
2445

2546
} // namespace buildcc::env

‎buildcc/lib/env/src/storage.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,10 @@ namespace buildcc {
2020

2121
std::unique_ptr<ScopedStorage> Storage::internal_;
2222

23+
ScopedStorage &Storage::Ref() {
24+
env::assert_fatal(internal_ != nullptr,
25+
"Initialize Storage using the Storage::Init API");
26+
return *internal_;
2327
}
28+
29+
} // namespace buildcc

‎buildcc/lib/env/test/test_assert_fatal.cpp

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
#include <thread>
2+
3+
#include "taskflow/taskflow.hpp"
4+
5+
// NOTE, Make sure all these includes are AFTER the system and header includes
6+
#include "CppUTest/CommandLineTestRunner.h"
7+
#include "CppUTest/MemoryLeakDetectorNewMacros.h"
8+
#include "CppUTest/TestHarness.h"
9+
#include "CppUTest/Utest.h"
10+
#include "CppUTestExt/MockSupport.h"
11+
12+
namespace {
13+
14+
std::thread::id my_main_thread = std::this_thread::get_id();
15+
16+
bool IsRunningInThread() {
17+
bool threaded = true;
18+
if (std::this_thread::get_id() == my_main_thread) {
19+
threaded = false;
20+
}
21+
return threaded;
22+
}
23+
24+
void assert_handle_fatal() {
25+
if (IsRunningInThread()) {
26+
mock().actualCall("assert_handle_fatal_threaded");
27+
} else {
28+
mock().actualCall("assert_handle_fatal_main");
29+
}
30+
}
31+
32+
} // namespace
33+
34+
// clang-format off
35+
TEST_GROUP(AssertFatalTestGroup)
36+
{
37+
void teardown() {
38+
mock().clear();
39+
}
40+
};
41+
// clang-format on
42+
43+
TEST(AssertFatalTestGroup, AssertFatal_IsThreadedCheck) {
44+
CHECK_FALSE(IsRunningInThread());
45+
46+
tf::Taskflow tf;
47+
tf.emplace([]() { CHECK_TRUE(IsRunningInThread()); });
48+
49+
tf::Executor ex(1);
50+
ex.run(tf);
51+
ex.wait_for_all();
52+
}
53+
54+
TEST(AssertFatalTestGroup, AssertFatal_Threaded) {
55+
mock().expectOneCall("assert_handle_fatal_threaded");
56+
57+
tf::Taskflow tf;
58+
tf.emplace([]() { assert_handle_fatal(); });
59+
60+
tf::Executor ex(1);
61+
ex.run(tf);
62+
ex.wait_for_all();
63+
}
64+
65+
TEST(AssertFatalTestGroup, AssertFatal_NotThreaded) {
66+
mock().expectOneCall("assert_handle_fatal_main");
67+
assert_handle_fatal();
68+
}
69+
70+
int main(int ac, char **av) {
71+
return CommandLineTestRunner::RunAllTests(ac, av);
72+
}

‎buildcc/lib/env/test/test_storage.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,14 @@ TEST(StorageTestGroup, BasicUsage) {
8686
STRCMP_EQUAL(bigobj2.c_str(), "name2");
8787
}
8888

89+
TEST(StorageTestGroup, UsageWithoutInit) {
90+
buildcc::Storage::Deinit();
91+
92+
CHECK_THROWS(std::exception, buildcc::Storage::Add<int>("integer"));
93+
CHECK_THROWS(std::exception, buildcc::Storage::Ref<int>("integer"));
94+
CHECK_THROWS(std::exception, buildcc::Storage::ConstRef<int>("integer"));
95+
}
96+
8997
int main(int ac, char **av) {
9098
return CommandLineTestRunner::RunAllTests(ac, av);
9199
}

‎buildcc/lib/target/src/generator/task.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ void Generator::GenerateTask() {
5252
auto run_command = [this](const std::string &command) {
5353
try {
5454
bool success = env::Command::Execute(command);
55-
env::assert_throw(success, fmt::format("{} failed", command));
55+
env::assert_fatal(success, fmt::format("{} failed", command));
5656
} catch (...) {
5757
std::lock_guard<std::mutex> guard(task_state_mutex_);
5858
task_state_ = env::TaskState::FAILURE;
@@ -104,7 +104,7 @@ void Generator::GenerateTask() {
104104
if (dirty_) {
105105
try {
106106
serialization_.UpdateStore(user_);
107-
env::assert_throw(serialization_.StoreToFile(),
107+
env::assert_fatal(serialization_.StoreToFile(),
108108
fmt::format("Store failed for {}", name_));
109109
} catch (...) {
110110
task_state_ = env::TaskState::FAILURE;

‎buildcc/lib/target/src/target/friend/compile_pch.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ void AggregateToFile(const fs::path &filename,
5454
});
5555
bool success = buildcc::env::save_file(
5656
buildcc::path_as_string(filename).c_str(), constructed_output, false);
57-
buildcc::env::assert_throw(success, "Could not save pch file");
57+
buildcc::env::assert_fatal(success, "Could not save pch file");
5858
}
5959

6060
} // namespace
@@ -108,10 +108,10 @@ void CompilePch::BuildCompile() {
108108
const std::string p = fmt::format("{}", source_path_);
109109
const bool save =
110110
env::save_file(p.c_str(), {"//Generated by BuildCC"}, false);
111-
env::assert_throw(save, fmt::format("Could not save {}", p));
111+
env::assert_fatal(save, fmt::format("Could not save {}", p));
112112
}
113113
bool success = env::Command::Execute(command_);
114-
env::assert_throw(success, "Failed to compile pch");
114+
env::assert_fatal(success, "Failed to compile pch");
115115
}
116116
}
117117

‎buildcc/lib/target/src/target/friend/link_target.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ void LinkTarget::BuildLink() {
9898

9999
if (target_.dirty_) {
100100
bool success = env::Command::Execute(command_);
101-
env::assert_throw(success, "Failed to link target");
101+
env::assert_fatal(success, "Failed to link target");
102102
target_.serialization_.UpdateTargetCompiled();
103103
}
104104
}

‎buildcc/lib/target/src/target/tasks.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ void CompileObject::Task() {
124124
try {
125125
bool success = env::Command::Execute(
126126
GetObjectData(s.GetPathname()).command);
127-
env::assert_throw(success, "Could not compile source");
127+
env::assert_fatal(success, "Could not compile source");
128128
target_.serialization_.AddSource(s);
129129
} catch (...) {
130130
target_.SetTaskStateFailure();
@@ -170,7 +170,7 @@ void Target::EndTask() {
170170
if (dirty_) {
171171
try {
172172
serialization_.UpdateStore(user_);
173-
env::assert_throw(serialization_.StoreToFile(),
173+
env::assert_fatal(serialization_.StoreToFile(),
174174
fmt::format("Store failed for {}", GetName()));
175175
state_.BuildCompleted();
176176
} catch (...) {

‎buildcc/schema/include/schema/path.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
#include <unordered_set>
2626

2727
// Env
28-
#include "env/assert_throw.h"
28+
#include "env/assert_fatal.h"
2929

3030
// Third party
3131
#include "fmt/format.h"
@@ -51,7 +51,7 @@ class Path {
5151
std::filesystem::last_write_time(pathname, errcode)
5252
.time_since_epoch()
5353
.count();
54-
env::assert_throw(errcode.value() == 0,
54+
env::assert_fatal(errcode.value() == 0,
5555
fmt::format("{} not found", pathname));
5656

5757
return Path(pathname, last_write_timestamp);

0 commit comments

Comments
 (0)