[Base, Linux] Make thread exit non returning

- Use pthread_exit() instead of pthread_cancel() if current thread
- Modify tests to ensure Exit does not return
This commit is contained in:
Joel Linn 2021-05-25 02:15:36 +02:00 committed by Rick Gibbed
parent 8f44a14131
commit 0e019f96b4
2 changed files with 82 additions and 66 deletions

View File

@ -775,59 +775,64 @@ TEST_CASE("Create and Run Thread", "Thread") {
Thread::CreationParameters params = {}; Thread::CreationParameters params = {};
auto func = [] { Sleep(20ms); }; auto func = [] { Sleep(20ms); };
// Create most basic case of thread SECTION("Create most basic case of thread") {
thread = Thread::Create(params, func); thread = Thread::Create(params, func);
REQUIRE(thread->native_handle() != nullptr); REQUIRE(thread->native_handle() != nullptr);
REQUIRE_NOTHROW(thread->affinity_mask()); REQUIRE_NOTHROW(thread->affinity_mask());
REQUIRE(thread->name().empty()); REQUIRE(thread->name().empty());
result = Wait(thread.get(), false, 50ms); result = Wait(thread.get(), false, 50ms);
REQUIRE(result == WaitResult::kSuccess); REQUIRE(result == WaitResult::kSuccess);
}
// Add thread name SECTION("Add thread name") {
std::string new_name = "Test thread name"; std::string new_name = "Test thread name";
thread = Thread::Create(params, func); thread = Thread::Create(params, func);
auto name = thread->name(); auto name = thread->name();
INFO(name.c_str()); INFO(name.c_str());
REQUIRE(name.empty()); REQUIRE(name.empty());
thread->set_name(new_name); thread->set_name(new_name);
REQUIRE(thread->name() == new_name); REQUIRE(thread->name() == new_name);
result = Wait(thread.get(), false, 50ms); result = Wait(thread.get(), false, 50ms);
REQUIRE(result == WaitResult::kSuccess); REQUIRE(result == WaitResult::kSuccess);
}
// Use Terminate to end an infinitely looping thread SECTION("Use Terminate to end an infinitely looping thread") {
thread = Thread::Create(params, [] { thread = Thread::Create(params, [] {
while (true) { while (true) {
Sleep(1ms); Sleep(1ms);
} }
}); });
result = Wait(thread.get(), false, 50ms); result = Wait(thread.get(), false, 50ms);
REQUIRE(result == WaitResult::kTimeout); REQUIRE(result == WaitResult::kTimeout);
thread->Terminate(-1); thread->Terminate(-1);
result = Wait(thread.get(), false, 50ms); result = Wait(thread.get(), false, 50ms);
REQUIRE(result == WaitResult::kSuccess); REQUIRE(result == WaitResult::kSuccess);
}
// Call Exit from inside an infinitely looping thread SECTION("Call Exit from inside an infinitely looping thread") {
thread = Thread::Create(params, [] { thread = Thread::Create(params, [] {
while (true) {
Thread::Exit(-1); Thread::Exit(-1);
} FAIL("Function must not return");
}); });
result = Wait(thread.get(), false, 50ms); result = Wait(thread.get(), false, 50ms);
REQUIRE(result == WaitResult::kSuccess); REQUIRE(result == WaitResult::kSuccess);
}
// Call timeout wait on self SECTION("Call timeout wait on self") {
result = Wait(Thread::GetCurrentThread(), false, 50ms); result = Wait(Thread::GetCurrentThread(), false, 50ms);
REQUIRE(result == WaitResult::kTimeout); REQUIRE(result == WaitResult::kTimeout);
}
params.stack_size = 16 * 1024 * 1024; SECTION("16kb stack size") {
thread = Thread::Create(params, [] { params.stack_size = 16 * 1024 * 1024;
while (true) { thread = Thread::Create(params, [] {
Thread::Exit(-1); Thread::Exit(-1);
} FAIL("Function must not return");
}); });
REQUIRE(thread != nullptr); REQUIRE(thread != nullptr);
result = Wait(thread.get(), false, 50ms); result = Wait(thread.get(), false, 50ms);
REQUIRE(result == WaitResult::kSuccess); REQUIRE(result == WaitResult::kSuccess);
}
// TODO(bwrsandman): Test with different priorities // TODO(bwrsandman): Test with different priorities
// TODO(bwrsandman): Test setting and getting thread affinity // TODO(bwrsandman): Test setting and getting thread affinity

View File

@ -735,31 +735,44 @@ class PosixCondition<Thread> : public PosixConditionBase {
} }
void Terminate(int exit_code) { void Terminate(int exit_code) {
bool is_current_thread = pthread_self() == thread_;
{ {
std::unique_lock<std::mutex> lock(state_mutex_); std::unique_lock<std::mutex> lock(state_mutex_);
if (state_ == State::kFinished) {
if (is_current_thread) {
// This is really bad. Some thread must have called Terminate() on us
// just before we decided to terminate ourselves
assert_always();
for (;;) {
// Wait for pthread_cancel() to actually happen.
}
}
return;
}
state_ = State::kFinished; state_ = State::kFinished;
} }
std::lock_guard<std::mutex> lock(mutex_); {
std::lock_guard<std::mutex> lock(mutex_);
// Sometimes the thread can call terminate twice before stopping
if (thread_ == 0) return;
auto thread = thread_;
exit_code_ = exit_code;
signaled_ = true;
cond_.notify_all();
exit_code_ = exit_code;
signaled_ = true;
cond_.notify_all();
}
if (is_current_thread) {
pthread_exit(reinterpret_cast<void*>(exit_code));
} else {
#ifdef XE_PLATFORM_ANDROID #ifdef XE_PLATFORM_ANDROID
if (pthread_kill(thread, GetSystemSignal(SignalType::kThreadTerminate)) != if (pthread_kill(thread_,
0) { GetSystemSignal(SignalType::kThreadTerminate)) != 0) {
assert_always(); assert_always();
} }
#else #else
if (pthread_cancel(thread) != 0) { if (pthread_cancel(thread_) != 0) {
assert_always(); assert_always();
} }
#endif #endif
}
} }
void WaitStarted() const { void WaitStarted() const {
@ -785,7 +798,6 @@ class PosixCondition<Thread> : public PosixConditionBase {
inline void post_execution() override { inline void post_execution() override {
if (thread_) { if (thread_) {
pthread_join(thread_, nullptr); pthread_join(thread_, nullptr);
thread_ = 0;
} }
} }
pthread_t thread_; pthread_t thread_;
@ -1122,13 +1134,12 @@ Thread* Thread::GetCurrentThread() {
void Thread::Exit(int exit_code) { void Thread::Exit(int exit_code) {
if (current_thread_) { if (current_thread_) {
current_thread_->Terminate(exit_code); current_thread_->Terminate(exit_code);
// Sometimes the current thread keeps running after being cancelled.
// Prevent other calls from this thread from using current_thread_.
current_thread_ = nullptr;
} else { } else {
// Should only happen with the main thread // Should only happen with the main thread
pthread_exit(reinterpret_cast<void*>(exit_code)); pthread_exit(reinterpret_cast<void*>(exit_code));
} }
// Function must not return
assert_always();
} }
void set_name(const std::string_view name) { void set_name(const std::string_view name) {