diff --git a/src/xenia/base/testing/threading_test.cc b/src/xenia/base/testing/threading_test.cc index 2eec1e320..e2437dfb2 100644 --- a/src/xenia/base/testing/threading_test.cc +++ b/src/xenia/base/testing/threading_test.cc @@ -775,59 +775,64 @@ TEST_CASE("Create and Run Thread", "Thread") { Thread::CreationParameters params = {}; auto func = [] { Sleep(20ms); }; - // Create most basic case of thread - thread = Thread::Create(params, func); - REQUIRE(thread->native_handle() != nullptr); - REQUIRE_NOTHROW(thread->affinity_mask()); - REQUIRE(thread->name().empty()); - result = Wait(thread.get(), false, 50ms); - REQUIRE(result == WaitResult::kSuccess); + SECTION("Create most basic case of thread") { + thread = Thread::Create(params, func); + REQUIRE(thread->native_handle() != nullptr); + REQUIRE_NOTHROW(thread->affinity_mask()); + REQUIRE(thread->name().empty()); + result = Wait(thread.get(), false, 50ms); + REQUIRE(result == WaitResult::kSuccess); + } - // Add thread name - std::string new_name = "Test thread name"; - thread = Thread::Create(params, func); - auto name = thread->name(); - INFO(name.c_str()); - REQUIRE(name.empty()); - thread->set_name(new_name); - REQUIRE(thread->name() == new_name); - result = Wait(thread.get(), false, 50ms); - REQUIRE(result == WaitResult::kSuccess); + SECTION("Add thread name") { + std::string new_name = "Test thread name"; + thread = Thread::Create(params, func); + auto name = thread->name(); + INFO(name.c_str()); + REQUIRE(name.empty()); + thread->set_name(new_name); + REQUIRE(thread->name() == new_name); + result = Wait(thread.get(), false, 50ms); + REQUIRE(result == WaitResult::kSuccess); + } - // Use Terminate to end an infinitely looping thread - thread = Thread::Create(params, [] { - while (true) { - Sleep(1ms); - } - }); - result = Wait(thread.get(), false, 50ms); - REQUIRE(result == WaitResult::kTimeout); - thread->Terminate(-1); - result = Wait(thread.get(), false, 50ms); - REQUIRE(result == WaitResult::kSuccess); + SECTION("Use Terminate to end an infinitely looping thread") { + thread = Thread::Create(params, [] { + while (true) { + Sleep(1ms); + } + }); + result = Wait(thread.get(), false, 50ms); + REQUIRE(result == WaitResult::kTimeout); + thread->Terminate(-1); + result = Wait(thread.get(), false, 50ms); + REQUIRE(result == WaitResult::kSuccess); + } - // Call Exit from inside an infinitely looping thread - thread = Thread::Create(params, [] { - while (true) { + SECTION("Call Exit from inside an infinitely looping thread") { + thread = Thread::Create(params, [] { Thread::Exit(-1); - } - }); - result = Wait(thread.get(), false, 50ms); - REQUIRE(result == WaitResult::kSuccess); + FAIL("Function must not return"); + }); + result = Wait(thread.get(), false, 50ms); + REQUIRE(result == WaitResult::kSuccess); + } - // Call timeout wait on self - result = Wait(Thread::GetCurrentThread(), false, 50ms); - REQUIRE(result == WaitResult::kTimeout); + SECTION("Call timeout wait on self") { + result = Wait(Thread::GetCurrentThread(), false, 50ms); + REQUIRE(result == WaitResult::kTimeout); + } - params.stack_size = 16 * 1024 * 1024; - thread = Thread::Create(params, [] { - while (true) { + SECTION("16kb stack size") { + params.stack_size = 16 * 1024 * 1024; + thread = Thread::Create(params, [] { Thread::Exit(-1); - } - }); - REQUIRE(thread != nullptr); - result = Wait(thread.get(), false, 50ms); - REQUIRE(result == WaitResult::kSuccess); + FAIL("Function must not return"); + }); + REQUIRE(thread != nullptr); + result = Wait(thread.get(), false, 50ms); + REQUIRE(result == WaitResult::kSuccess); + } // TODO(bwrsandman): Test with different priorities // TODO(bwrsandman): Test setting and getting thread affinity diff --git a/src/xenia/base/threading_posix.cc b/src/xenia/base/threading_posix.cc index c0011d603..7766df05e 100644 --- a/src/xenia/base/threading_posix.cc +++ b/src/xenia/base/threading_posix.cc @@ -735,31 +735,44 @@ class PosixCondition : public PosixConditionBase { } void Terminate(int exit_code) { + bool is_current_thread = pthread_self() == thread_; { std::unique_lock 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; } - std::lock_guard 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(); + { + std::lock_guard lock(mutex_); + exit_code_ = exit_code; + signaled_ = true; + cond_.notify_all(); + } + if (is_current_thread) { + pthread_exit(reinterpret_cast(exit_code)); + } else { #ifdef XE_PLATFORM_ANDROID - if (pthread_kill(thread, GetSystemSignal(SignalType::kThreadTerminate)) != - 0) { - assert_always(); - } + if (pthread_kill(thread_, + GetSystemSignal(SignalType::kThreadTerminate)) != 0) { + assert_always(); + } #else - if (pthread_cancel(thread) != 0) { - assert_always(); - } + if (pthread_cancel(thread_) != 0) { + assert_always(); + } #endif + } } void WaitStarted() const { @@ -785,7 +798,6 @@ class PosixCondition : public PosixConditionBase { inline void post_execution() override { if (thread_) { pthread_join(thread_, nullptr); - thread_ = 0; } } pthread_t thread_; @@ -1122,13 +1134,12 @@ Thread* Thread::GetCurrentThread() { void Thread::Exit(int exit_code) { if (current_thread_) { 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 { // Should only happen with the main thread pthread_exit(reinterpret_cast(exit_code)); } + // Function must not return + assert_always(); } void set_name(const std::string_view name) {