diff --git a/.drone.star b/.drone.star index 50b5c9ec3..7a1af7633 100644 --- a/.drone.star +++ b/.drone.star @@ -277,7 +277,7 @@ def pipeline_linux_desktop(name, image, arch, cc, build_release_all): 'image': image, 'volumes': [volume_build('premake')], 'commands': [ - 'valgrind --error-exitcode=99 ./build/bin/Linux/Debug/xenia-base-tests', + 'valgrind --error-exitcode=99 ./build/bin/Linux/Debug/xenia-base-tests --durations yes', ], 'depends_on': ['build-premake-debug-tests'], }, @@ -287,7 +287,7 @@ def pipeline_linux_desktop(name, image, arch, cc, build_release_all): 'image': image, 'volumes': [volume_build('premake')], 'commands': [ - './build/bin/Linux/Release/xenia-base-tests', + './build/bin/Linux/Release/xenia-base-tests --success --durations yes', ], 'depends_on': ['build-premake-release-tests'], }, @@ -297,7 +297,7 @@ def pipeline_linux_desktop(name, image, arch, cc, build_release_all): 'image': image, 'volumes': [volume_build('cmake')], 'commands': [ - './build/bin/Linux/Release/xenia-base-tests', + './build/bin/Linux/Release/xenia-base-tests --success --durations yes', ], 'depends_on': ['build-cmake-release-tests'], }, diff --git a/src/xenia/app/emulator_window.cc b/src/xenia/app/emulator_window.cc index a4dbb8786..7e6d9e6b7 100644 --- a/src/xenia/app/emulator_window.cc +++ b/src/xenia/app/emulator_window.cc @@ -224,7 +224,7 @@ void EmulatorWindow::OnEmulatorInitialized() { // When the user can see that the emulator isn't initializing anymore (the // menu isn't disabled), enter fullscreen if requested. if (cvars::fullscreen) { - window_->SetFullscreen(true); + SetFullscreen(true); } } diff --git a/src/xenia/app/xenia_main.cc b/src/xenia/app/xenia_main.cc index c491e6510..44cc6a855 100644 --- a/src/xenia/app/xenia_main.cc +++ b/src/xenia/app/xenia_main.cc @@ -17,6 +17,7 @@ #include "xenia/app/discord/discord_presence.h" #include "xenia/app/emulator_window.h" +#include "xenia/base/assert.h" #include "xenia/base/cvar.h" #include "xenia/base/debugging.h" #include "xenia/base/logging.h" @@ -372,6 +373,7 @@ bool EmulatorApp::OnInitialize() { // Setup the emulator and run its loop in a separate thread. emulator_thread_quit_requested_.store(false, std::memory_order_relaxed); emulator_thread_event_ = xe::threading::Event::CreateAutoResetEvent(false); + assert_not_null(emulator_thread_event_); emulator_thread_ = std::thread(&EmulatorApp::EmulatorThread, this); return true; diff --git a/src/xenia/apu/audio_system.cc b/src/xenia/apu/audio_system.cc index c137a4853..371b0b82b 100644 --- a/src/xenia/apu/audio_system.cc +++ b/src/xenia/apu/audio_system.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -12,6 +12,7 @@ #include "xenia/apu/apu_flags.h" #include "xenia/apu/audio_driver.h" #include "xenia/apu/xma_decoder.h" +#include "xenia/base/assert.h" #include "xenia/base/byte_stream.h" #include "xenia/base/logging.h" #include "xenia/base/math.h" @@ -45,14 +46,17 @@ AudioSystem::AudioSystem(cpu::Processor* processor) for (size_t i = 0; i < kMaximumClientCount; ++i) { client_semaphores_[i] = xe::threading::Semaphore::Create(0, kMaximumQueuedFrames); + assert_not_null(client_semaphores_[i]); wait_handles_[i] = client_semaphores_[i].get(); } shutdown_event_ = xe::threading::Event::CreateAutoResetEvent(false); + assert_not_null(shutdown_event_); wait_handles_[kMaximumClientCount] = shutdown_event_.get(); xma_decoder_ = std::make_unique(processor_); resume_event_ = xe::threading::Event::CreateAutoResetEvent(false); + assert_not_null(resume_event_); } AudioSystem::~AudioSystem() { diff --git a/src/xenia/apu/xma_decoder.cc b/src/xenia/apu/xma_decoder.cc index cd04ebd91..3ad45e821 100644 --- a/src/xenia/apu/xma_decoder.cc +++ b/src/xenia/apu/xma_decoder.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2021 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -139,6 +139,7 @@ X_STATUS XmaDecoder::Setup(kernel::KernelState* kernel_state) { worker_running_ = true; work_event_ = xe::threading::Event::CreateAutoResetEvent(false); + assert_not_null(work_event_); worker_thread_ = kernel::object_ref( new kernel::XHostThread(kernel_state, 128 * 1024, 0, [this]() { WorkerThreadMain(); diff --git a/src/xenia/base/app_win32.manifest b/src/xenia/base/app_win32.manifest index 1867a1fd0..bad9294e6 100644 --- a/src/xenia/base/app_win32.manifest +++ b/src/xenia/base/app_win32.manifest @@ -1,5 +1,18 @@ + + + + + + + + + + + + + diff --git a/src/xenia/base/delay_scheduler.cc b/src/xenia/base/delay_scheduler.cc new file mode 100644 index 000000000..982d1d05a --- /dev/null +++ b/src/xenia/base/delay_scheduler.cc @@ -0,0 +1,111 @@ +/** + ****************************************************************************** + * Xenia : Xbox 360 Emulator Research Project * + ****************************************************************************** + * Copyright 2022 Ben Vanik. All rights reserved. * + * Released under the BSD license - see LICENSE in the root for more details. * + ****************************************************************************** + */ + +#include "xenia/base/delay_scheduler.h" +#include "xenia/base/assert.h" +#include "xenia/base/logging.h" + +namespace xe::internal { + +DelaySchedulerBase::~DelaySchedulerBase() { + if (call_on_destruct_) { + for (auto& slot : deletion_queue_) { + // No thread safety in destructors anyway + if (slot.state == State::kWaiting) { + callback_(slot.info); + } + } + } +} + +void DelaySchedulerBase::Collect() { + static_assert(atomic_state_t::is_always_lock_free, + "Locks are unsafe to use with thread suspension"); + + for (auto& slot : deletion_queue_) { + TryCollectOne_impl(slot); + } +} + +bool DelaySchedulerBase::TryCollectOne_impl(deletion_record_t& slot) { + auto now = clock::now(); + + auto state = State::kWaiting; + // Try to lock the waiting slot for reading the other fields + if (slot.state.compare_exchange_strong(state, State::kDeleting, + std::memory_order_acq_rel)) { + if (now > slot.due) { + // Time has passed, call back now + callback_(slot.info); + slot.info = nullptr; + slot.state.store(State::kFree, std::memory_order_release); + return true; + } else { + // Oops it's not yet due + slot.state.store(State::kWaiting, std::memory_order_release); + } + } + return false; +} + +void DelaySchedulerBase::ScheduleAt_impl( + void* info, const clock::time_point& timeout_time) { + bool first_pass = true; + + if (info == nullptr) { + return; + } + + for (;;) { + if (TryScheduleAt_impl(info, timeout_time)) { + return; + } + + if (first_pass) { + first_pass = false; + XELOGE( + "`DelayScheduler::ScheduleAt(...)` stalled: list is full! Find out " + "why or increase `MAX_QUEUE`."); + } + } +} + +bool DelaySchedulerBase::TryScheduleAt_impl( + void* info, const clock::time_point& timeout_time) { + if (info == nullptr) { + return false; + } + + for (auto& slot : deletion_queue_) { + // Clean up due item + TryCollectOne_impl(slot); + + if (TryScheduleAtOne_impl(slot, info, timeout_time)) { + return true; + } + } + return false; +} + +bool DelaySchedulerBase::TryScheduleAtOne_impl(deletion_record_t& slot, + void* info, + clock::time_point due) { + auto state = State::kFree; + if (slot.state.compare_exchange_strong(state, State::kInitializing, + std::memory_order_acq_rel)) { + slot.info = info; + slot.due = due; + slot.state.store(State::kWaiting, std::memory_order_release); + return true; + } + + return false; +} + +} // namespace xe::internal \ No newline at end of file diff --git a/src/xenia/base/delay_scheduler.h b/src/xenia/base/delay_scheduler.h new file mode 100644 index 000000000..33cf196c9 --- /dev/null +++ b/src/xenia/base/delay_scheduler.h @@ -0,0 +1,142 @@ +/** + ****************************************************************************** + * Xenia : Xbox 360 Emulator Research Project * + ****************************************************************************** + * Copyright 2022 Ben Vanik. All rights reserved. * + * Released under the BSD license - see LICENSE in the root for more details. * + ****************************************************************************** + */ + +#ifndef XENIA_BASE_DELAY_SCHEDULER_H_ +#define XENIA_BASE_DELAY_SCHEDULER_H_ + +#include +#include +#include +#include +#include + +namespace xe { + +namespace internal { +// Put the implementation in a non templated base class to reduce compile time +// and code duplication +class DelaySchedulerBase { + protected: // Types + enum class State : uint_least8_t { + kFree = 0, // Slot is unsued + kInitializing, // Slot is reserved and currently written to by a thread + kWaiting, // The slot contains a pointer scheduled for deletion + kDeleting // A thread is currently deleting the pointer + }; + using atomic_state_t = std::atomic; + using clock = std::chrono::steady_clock; + struct deletion_record_t { + atomic_state_t state; + void* info; + clock::time_point due; + }; + using deletion_queue_t = std::vector; + using callback_t = std::function; + + public: + /// Check all scheduled items in the queue and free any that are due using + /// `callback(info);`. Call this to reliably collect all due wait items in the + /// queue. + void Collect(); + + size_t size() { return deletion_queue_.size(); } + + protected: // Implementation + DelaySchedulerBase(size_t queue_size, callback_t callback, + bool call_on_destruct) + : deletion_queue_(queue_size), + callback_(callback), + call_on_destruct_(call_on_destruct) {} + virtual ~DelaySchedulerBase(); + + void ScheduleAt_impl(void* info, const clock::time_point& timeout_time); + [[nodiscard]] bool TryScheduleAt_impl(void* info, + const clock::time_point& timeout_time); + + /// Checks if the slot is due and if so, call back for it. + bool TryCollectOne_impl(deletion_record_t& slot); + + [[nodiscard]] bool TryScheduleAtOne_impl(deletion_record_t& slot, void* info, + clock::time_point due); + + private: + deletion_queue_t deletion_queue_; + callback_t callback_; + bool call_on_destruct_; +}; +} // namespace internal + +/// A lazy scheduler/timer. +/// Will wait at least the specified duration before invoking the callbacks but +/// might wait until it is destructed. Lockless thread-safe, will spinlock +/// though if the wait queue is full (except for `Try`... methods). Might use +/// any thread that calls any member to invoke callbacks of due wait items. +template +class DelayScheduler : internal::DelaySchedulerBase { + public: + DelayScheduler(size_t queue_size, std::function callback, + bool call_on_destruct) + : DelaySchedulerBase( + queue_size, + [callback](void* info) { callback(reinterpret_cast(info)); }, + call_on_destruct){}; + DelayScheduler(const DelayScheduler&) = delete; + DelayScheduler& operator=(const DelayScheduler&) = delete; + virtual ~DelayScheduler() {} + + // From base class: + // void Collect(); + + /// Schedule an object for deletion at some point after `timeout_time` using + /// `callback(info);`. Will collect any wait items it encounters which can be + /// 0 or all, use `Collect()` to collect all due wait items. Blocks until a + /// free wait slot is found. + template + void ScheduleAt( + INFO* info, + const std::chrono::time_point& timeout_time) { + ScheduleAt(info, + std::chrono::time_point_cast(timeout_time)); + } + /// Like `ScheduleAt` but does not block on full list. + template + [[nodiscard]] bool TryScheduleAt( + INFO* info, + const std::chrono::time_point& timeout_time) { + return TryScheduleAt( + info, std::chrono::time_point_cast(timeout_time)); + } + + void ScheduleAt(INFO* info, const clock::time_point& timeout_time) { + ScheduleAt_impl(info, timeout_time); + } + [[nodiscard]] bool TryScheduleAt(INFO* info, + const clock::time_point& timeout_time) { + return TryScheduleAt_impl(info, timeout_time); + } + + /// Schedule a callback at some point after `rel_time` has passed. + template + void ScheduleAfter(INFO* info, + const std::chrono::duration& rel_time) { + ScheduleAt(info, + clock::now() + std::chrono::ceil(rel_time)); + } + /// Like `ScheduleAfter` but does not block. + template + [[nodiscard]] bool TryScheduleAfter( + INFO* info, const std::chrono::duration& rel_time) { + return TryScheduleAt( + info, clock::now() + std::chrono::ceil(rel_time)); + } +}; + +} // namespace xe + +#endif diff --git a/src/xenia/base/filesystem_wildcard.cc b/src/xenia/base/filesystem_wildcard.cc index 1954060dc..bdaafacaa 100644 --- a/src/xenia/base/filesystem_wildcard.cc +++ b/src/xenia/base/filesystem_wildcard.cc @@ -19,6 +19,7 @@ namespace xe::filesystem { WildcardFlags WildcardFlags::FIRST(true, false, false); WildcardFlags WildcardFlags::LAST(false, true, false); WildcardFlags WildcardFlags::ANY(false, false, true); +WildcardFlags WildcardFlags::FIRST_AND_LAST(true, true, false); WildcardFlags::WildcardFlags() : FromStart(false), ToEnd(false), ExactLength(false) {} @@ -89,7 +90,8 @@ void WildcardEngine::PreparePattern(const std::string_view pattern) { } if (last != pattern.size()) { std::string str_str(pattern.substr(last)); - rules_.push_back(WildcardRule(str_str, WildcardFlags::LAST)); + rules_.push_back(WildcardRule( + str_str, last ? WildcardFlags::LAST : WildcardFlags::FIRST_AND_LAST)); } } diff --git a/src/xenia/base/filesystem_wildcard.h b/src/xenia/base/filesystem_wildcard.h index fdad44604..545321fee 100644 --- a/src/xenia/base/filesystem_wildcard.h +++ b/src/xenia/base/filesystem_wildcard.h @@ -30,6 +30,7 @@ class WildcardFlags { static WildcardFlags FIRST; static WildcardFlags LAST; static WildcardFlags ANY; + static WildcardFlags FIRST_AND_LAST; }; class WildcardRule { diff --git a/src/xenia/base/logging.cc b/src/xenia/base/logging.cc index bed2fe43b..ed636e428 100644 --- a/src/xenia/base/logging.cc +++ b/src/xenia/base/logging.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2021 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -224,6 +224,7 @@ class Logger { write_thread_ = xe::threading::Thread::Create({}, [this]() { WriteThread(); }); + assert_not_null(write_thread_); write_thread_->set_name("Logging Writer"); } diff --git a/src/xenia/base/socket_win.cc b/src/xenia/base/socket_win.cc index 35b96d470..2d3644dda 100644 --- a/src/xenia/base/socket_win.cc +++ b/src/xenia/base/socket_win.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2015 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -91,6 +91,7 @@ class Win32Socket : public Socket { // notifications. // Set true to start so we'll force a query of the socket on first run. event_ = xe::threading::Event::CreateManualResetEvent(true); + assert_not_null(event_); WSAEventSelect(socket_, event_->native_handle(), FD_READ | FD_CLOSE); // Keepalive for a looong time, as we may be paused by the debugger/etc. @@ -279,6 +280,7 @@ class Win32SocketServer : public SocketServer { accept_callback_(std::move(client)); } }); + assert_not_null(accept_thread_); return true; } diff --git a/src/xenia/base/testing/threading_test.cc b/src/xenia/base/testing/threading_test.cc index d1380d602..3a8b119c1 100644 --- a/src/xenia/base/testing/threading_test.cc +++ b/src/xenia/base/testing/threading_test.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** -* Copyright 2021 Ben Vanik. All rights reserved. * +* Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -19,6 +19,35 @@ namespace test { using namespace threading; using namespace std::chrono_literals; +// Helpers to wait on a predicate which do not depend on complex sync primitives +template +bool spin_wait_until( + const std::chrono::time_point& timeout_time, + Predicate stop_waiting) { + while (!stop_waiting()) { + if (std::chrono::steady_clock::now() >= timeout_time) { + return false; + } + // Needed for valgrind because it basically runs one thread: + MaybeYield(); + } + return true; +} +template +bool spin_wait_for(const std::chrono::duration& rel_time, + Predicate stop_waiting) { + return spin_wait_until(std::chrono::steady_clock::now() + rel_time, + stop_waiting); +} + +template +void spin_wait(Predicate stop_waiting) { + while (!stop_waiting()) { + // Needed for valgrind because it basically runs one thread: + MaybeYield(); + } +} + TEST_CASE("Fence") { std::unique_ptr pFence; std::unique_ptr pTimer; @@ -63,8 +92,7 @@ TEST_CASE("Fence") { std::thread(func), }); - Sleep(100ms); - REQUIRE(started.load() == threads.size()); + REQUIRE(spin_wait_for(1s, [&] { return started == threads.size(); })); REQUIRE(finished.load() == 0); pFence->Signal(); @@ -137,6 +165,7 @@ TEST_CASE("TlsHandle") { auto thread = Thread::Create({}, [&non_thread_local_value, &handle] { non_thread_local_value = threading::GetTlsValue(handle); }); + REQUIRE(thread); auto result = Wait(thread.get(), false, 50ms); REQUIRE(result == WaitResult::kSuccess); @@ -151,12 +180,21 @@ TEST_CASE("HighResolutionTimer") { // Smaller values are not as precise and fail the test const auto wait_time = 500ms; + const Thread* timer_thread = nullptr; + // Time the actual sleep duration { const auto interval = 50ms; std::atomic counter(0); auto start = std::chrono::steady_clock::now(); - auto cb = [&counter] { ++counter; }; + auto cb = [&counter, &timer_thread] { + if (counter == 0) { + timer_thread = Thread::GetCurrentThread(); + } else { + REQUIRE(Thread::GetCurrentThread() == timer_thread); + } + ++counter; + }; auto pTimer = HighResolutionTimer::CreateRepeating(interval, cb); Sleep(wait_time); pTimer.reset(); @@ -177,8 +215,14 @@ TEST_CASE("HighResolutionTimer") { std::atomic counter1(0); std::atomic counter2(0); auto start = std::chrono::steady_clock::now(); - auto cb1 = [&counter1] { ++counter1; }; - auto cb2 = [&counter2] { ++counter2; }; + auto cb1 = [&counter1, timer_thread] { + ++counter1; + REQUIRE(Thread::GetCurrentThread() == timer_thread); + }; + auto cb2 = [&counter2, timer_thread] { + ++counter2; + REQUIRE(Thread::GetCurrentThread() == timer_thread); + }; auto pTimer1 = HighResolutionTimer::CreateRepeating(interval1, cb1); auto pTimer2 = HighResolutionTimer::CreateRepeating(interval2, cb2); Sleep(wait_time); @@ -203,8 +247,11 @@ TEST_CASE("HighResolutionTimer") { TEST_CASE("Wait on Multiple Handles", "[wait]") { auto mutant = Mutant::Create(true); + REQUIRE(mutant); auto semaphore = Semaphore::Create(10, 10); + REQUIRE(semaphore); auto event_ = Event::CreateManualResetEvent(false); + REQUIRE(event_); auto thread = Thread::Create({}, [&mutant, &semaphore, &event_] { event_->Set(); Wait(mutant.get(), false, 25ms); @@ -231,7 +278,9 @@ TEST_CASE("Wait on Multiple Handles", "[wait]") { TEST_CASE("Signal and Wait") { WaitResult result; auto mutant = Mutant::Create(true); + REQUIRE(mutant); auto event_ = Event::CreateAutoResetEvent(false); + REQUIRE(event_); auto thread = Thread::Create({}, [&mutant, &event_] { Wait(mutant.get(), false); event_->Set(); @@ -246,6 +295,7 @@ TEST_CASE("Signal and Wait") { TEST_CASE("Wait on Event", "[event]") { auto evt = Event::CreateAutoResetEvent(false); + REQUIRE(evt); WaitResult result; // Call wait on unset Event @@ -264,6 +314,7 @@ TEST_CASE("Wait on Event", "[event]") { TEST_CASE("Reset Event", "[event]") { auto evt = Event::CreateAutoResetEvent(false); + REQUIRE(evt); WaitResult result; // Call wait on reset Event @@ -290,7 +341,11 @@ TEST_CASE("Wait on Multiple Events", "[event]") { Event::CreateAutoResetEvent(false), Event::CreateManualResetEvent(false), }; + for (auto& event : events) { + REQUIRE(event.get() != nullptr); + } + std::atomic_uint threads_started(0); std::array order = {0}; std::atomic_uint index(0); auto sign_in = [&order, &index](uint32_t id) { @@ -299,40 +354,53 @@ TEST_CASE("Wait on Multiple Events", "[event]") { }; auto threads = std::array{ - std::thread([&events, &sign_in] { - auto res = WaitAll({events[1].get(), events[3].get()}, false, 100ms); + std::thread([&events, &sign_in, &threads_started] { + set_name("1"); + threads_started++; + auto res = WaitAll({events[1].get(), events[3].get()}, false); + REQUIRE(res == WaitResult::kSuccess); if (res == WaitResult::kSuccess) { sign_in(1); } }), - std::thread([&events, &sign_in] { - auto res = WaitAny({events[0].get(), events[2].get()}, false, 100ms); + std::thread([&events, &sign_in, &threads_started] { + set_name("2"); + threads_started++; + auto res = WaitAny({events[0].get(), events[2].get()}, false); + REQUIRE(res.first == WaitResult::kSuccess); if (res.first == WaitResult::kSuccess) { sign_in(2); } }), - std::thread([&events, &sign_in] { - auto res = WaitAll({events[0].get(), events[2].get(), events[3].get()}, - false, 100ms); + std::thread([&events, &sign_in, &threads_started] { + set_name("3"); + threads_started++; + auto res = + WaitAll({events[0].get(), events[2].get(), events[3].get()}, false); + REQUIRE(res == WaitResult::kSuccess); if (res == WaitResult::kSuccess) { sign_in(3); } }), - std::thread([&events, &sign_in] { - auto res = WaitAny({events[1].get(), events[3].get()}, false, 100ms); + std::thread([&events, &sign_in, &threads_started] { + set_name("4"); + threads_started++; + auto res = WaitAny({events[1].get(), events[3].get()}, false); + REQUIRE(res.first == WaitResult::kSuccess); if (res.first == WaitResult::kSuccess) { sign_in(4); } }), }; - Sleep(10ms); + // wait for all threads starting up + REQUIRE(spin_wait_for(1s, [&] { return threads_started == 4; })); events[3]->Set(); // Signals thread id=4 and stays on for 1 and 3 - Sleep(10ms); + REQUIRE(spin_wait_for(1s, [&] { return index == 1; })); events[1]->Set(); // Signals thread id=1 - Sleep(10ms); + REQUIRE(spin_wait_for(1s, [&] { return index == 2; })); events[0]->Set(); // Signals thread id=2 - Sleep(10ms); + REQUIRE(spin_wait_for(1s, [&] { return index == 3; })); events[2]->Set(); // Partial signals thread id=3 events[0]->Set(); // Signals thread id=3 @@ -340,12 +408,13 @@ TEST_CASE("Wait on Multiple Events", "[event]") { t.join(); } + REQUIRE(index == 4); + INFO(order.data()); REQUIRE(order[0] == '4'); - // TODO(bwrsandman): Order is not always maintained on linux - // REQUIRE(order[1] == '1'); - // REQUIRE(order[2] == '2'); - // REQUIRE(order[3] == '3'); + REQUIRE(order[1] == '1'); + REQUIRE(order[2] == '2'); + REQUIRE(order[3] == '3'); } TEST_CASE("Wait on Semaphore", "[semaphore]") { @@ -355,6 +424,7 @@ TEST_CASE("Wait on Semaphore", "[semaphore]") { // Wait on semaphore with no room sem = Semaphore::Create(0, 5); + REQUIRE(sem); result = Wait(sem.get(), false, 10ms); REQUIRE(result == WaitResult::kTimeout); @@ -370,12 +440,14 @@ TEST_CASE("Wait on Semaphore", "[semaphore]") { // Set semaphore over maximum_count sem = Semaphore::Create(5, 5); + REQUIRE(sem); previous_count = -1; REQUIRE_FALSE(sem->Release(1, &previous_count)); REQUIRE(previous_count == -1); REQUIRE_FALSE(sem->Release(10, &previous_count)); REQUIRE(previous_count == -1); sem = Semaphore::Create(0, 5); + REQUIRE(sem); REQUIRE_FALSE(sem->Release(10, &previous_count)); REQUIRE(previous_count == -1); REQUIRE_FALSE(sem->Release(10, &previous_count)); @@ -389,6 +461,7 @@ TEST_CASE("Wait on Semaphore", "[semaphore]") { // Wait on fully available semaphore sem = Semaphore::Create(5, 5); + REQUIRE(sem); result = Wait(sem.get(), false, 10ms); REQUIRE(result == WaitResult::kSuccess); result = Wait(sem.get(), false, 10ms); @@ -404,26 +477,25 @@ TEST_CASE("Wait on Semaphore", "[semaphore]") { // Semaphore between threads sem = Semaphore::Create(5, 5); - Sleep(10ms); + REQUIRE(sem); // Occupy the semaphore with 5 threads std::atomic wait_count(0); - volatile bool threads_terminate(false); + std::atomic threads_terminate(false); auto func = [&sem, &wait_count, &threads_terminate] { auto res = Wait(sem.get(), false, 100ms); wait_count++; - while (!threads_terminate) { - } - if (res == WaitResult::kSuccess) { - sem->Release(1, nullptr); - } + + REQUIRE(spin_wait_for(2s, [&] { return threads_terminate.load(); })); + + REQUIRE(res == WaitResult::kSuccess); + sem->Release(1, nullptr); }; auto threads = std::array{ std::thread(func), std::thread(func), std::thread(func), std::thread(func), std::thread(func), }; // Wait for threads to finish semaphore calls - while (wait_count != 5) { - } + REQUIRE(spin_wait_for(1s, [&] { return wait_count == 5; })); // Attempt to acquire full semaphore with current (6th) thread result = Wait(sem.get(), false, 20ms); REQUIRE(result == WaitResult::kTimeout); @@ -436,18 +508,23 @@ TEST_CASE("Wait on Semaphore", "[semaphore]") { REQUIRE(result == WaitResult::kSuccess); sem->Release(1, &previous_count); REQUIRE(previous_count == 4); +} + +TEST_CASE("Invalid semaphore parameters", "[semaphore]") { + std::unique_ptr sem; // Test invalid construction parameters // These are invalid according to documentation - // TODO(bwrsandman): Many of these invalid invocations succeed sem = Semaphore::Create(-1, 5); - // REQUIRE(sem.get() == nullptr); + REQUIRE(sem == nullptr); sem = Semaphore::Create(10, 5); - // REQUIRE(sem.get() == nullptr); + REQUIRE(sem == nullptr); sem = Semaphore::Create(0, 0); - // REQUIRE(sem.get() == nullptr); + REQUIRE(sem == nullptr); sem = Semaphore::Create(0, -1); - // REQUIRE(sem.get() == nullptr); + REQUIRE(sem == nullptr); + sem = Semaphore::Create(-1, 0); + REQUIRE(sem == nullptr); } TEST_CASE("Wait on Multiple Semaphores", "[semaphore]") { @@ -459,6 +536,8 @@ TEST_CASE("Wait on Multiple Semaphores", "[semaphore]") { // Test Wait all which should fail sem0 = Semaphore::Create(0, 5); sem1 = Semaphore::Create(5, 5); + REQUIRE(sem0); + REQUIRE(sem1); all_result = WaitAll({sem0.get(), sem1.get()}, false, 10ms); REQUIRE(all_result == WaitResult::kTimeout); previous_count = -1; @@ -471,6 +550,8 @@ TEST_CASE("Wait on Multiple Semaphores", "[semaphore]") { // Test Wait all again which should succeed sem0 = Semaphore::Create(1, 5); sem1 = Semaphore::Create(5, 5); + REQUIRE(sem0); + REQUIRE(sem1); all_result = WaitAll({sem0.get(), sem1.get()}, false, 10ms); REQUIRE(all_result == WaitResult::kSuccess); previous_count = -1; @@ -483,6 +564,8 @@ TEST_CASE("Wait on Multiple Semaphores", "[semaphore]") { // Test Wait Any which should fail sem0 = Semaphore::Create(0, 5); sem1 = Semaphore::Create(0, 5); + REQUIRE(sem0); + REQUIRE(sem1); any_result = WaitAny({sem0.get(), sem1.get()}, false, 10ms); REQUIRE(any_result.first == WaitResult::kTimeout); REQUIRE(any_result.second == 0); @@ -496,6 +579,8 @@ TEST_CASE("Wait on Multiple Semaphores", "[semaphore]") { // Test Wait Any which should succeed sem0 = Semaphore::Create(0, 5); sem1 = Semaphore::Create(5, 5); + REQUIRE(sem0); + REQUIRE(sem1); any_result = WaitAny({sem0.get(), sem1.get()}, false, 10ms); REQUIRE(any_result.first == WaitResult::kSuccess); REQUIRE(any_result.second == 1); @@ -547,17 +632,18 @@ TEST_CASE("Wait on Mutant", "[mutant]") { REQUIRE_FALSE(mut->Release()); // Test mutants on other threads - auto thread1 = std::thread([&mut] { - Sleep(5ms); + std::atomic step(0); + auto thread1 = std::thread([&mut, &step] { mut = Mutant::Create(true); - Sleep(100ms); + step++; // 1 + REQUIRE(spin_wait_for(2s, [&] { return step == 2; })); mut->Release(); }); - Sleep(10ms); + REQUIRE(spin_wait_for(1s, [&] { return step == 1; })); REQUIRE_FALSE(mut->Release()); - Sleep(10ms); result = Wait(mut.get(), false, 50ms); REQUIRE(result == WaitResult::kTimeout); + step++; // 2 thread1.join(); result = Wait(mut.get(), false, 1ms); REQUIRE(result == WaitResult::kSuccess); @@ -568,16 +654,18 @@ TEST_CASE("Wait on Multiple Mutants", "[mutant]") { WaitResult all_result; std::pair any_result; std::unique_ptr mut0, mut1; + std::atomic step(0); // Test which should fail for WaitAll and WaitAny - auto thread0 = std::thread([&mut0, &mut1] { + auto thread0 = std::thread([&mut0, &mut1, &step] { mut0 = Mutant::Create(true); mut1 = Mutant::Create(true); - Sleep(50ms); + step++; // 1 + REQUIRE(spin_wait_for(2s, [&] { return step == 2; })); mut0->Release(); mut1->Release(); }); - Sleep(10ms); + REQUIRE(spin_wait_for(1s, [&] { return step == 1; })); all_result = WaitAll({mut0.get(), mut1.get()}, false, 10ms); REQUIRE(all_result == WaitResult::kTimeout); REQUIRE_FALSE(mut0->Release()); @@ -587,16 +675,19 @@ TEST_CASE("Wait on Multiple Mutants", "[mutant]") { REQUIRE(any_result.second == 0); REQUIRE_FALSE(mut0->Release()); REQUIRE_FALSE(mut1->Release()); + step++; // 2 thread0.join(); // Test which should fail for WaitAll but not WaitAny - auto thread1 = std::thread([&mut0, &mut1] { + step = 0; + auto thread1 = std::thread([&mut0, &mut1, &step] { mut0 = Mutant::Create(true); mut1 = Mutant::Create(false); - Sleep(50ms); + step++; // 1 + REQUIRE(spin_wait_for(2s, [&] { return step == 2; })); mut0->Release(); }); - Sleep(10ms); + REQUIRE(spin_wait_for(1s, [&] { return step == 1; })); all_result = WaitAll({mut0.get(), mut1.get()}, false, 10ms); REQUIRE(all_result == WaitResult::kTimeout); REQUIRE_FALSE(mut0->Release()); @@ -606,15 +697,18 @@ TEST_CASE("Wait on Multiple Mutants", "[mutant]") { REQUIRE(any_result.second == 1); REQUIRE_FALSE(mut0->Release()); REQUIRE(mut1->Release()); + step++; // 2 thread1.join(); // Test which should pass for WaitAll and WaitAny - auto thread2 = std::thread([&mut0, &mut1] { + step = 0; + auto thread2 = std::thread([&mut0, &mut1, &step] { mut0 = Mutant::Create(false); mut1 = Mutant::Create(false); - Sleep(50ms); + step++; // 1 + REQUIRE(spin_wait_for(2s, [&] { return step == 2; })); }); - Sleep(10ms); + REQUIRE(spin_wait_for(1s, [&] { return step == 1; })); all_result = WaitAll({mut0.get(), mut1.get()}, false, 10ms); REQUIRE(all_result == WaitResult::kSuccess); REQUIRE(mut0->Release()); @@ -624,6 +718,7 @@ TEST_CASE("Wait on Multiple Mutants", "[mutant]") { REQUIRE(any_result.second == 0); REQUIRE(mut0->Release()); REQUIRE_FALSE(mut1->Release()); + step++; // 2 thread2.join(); } @@ -633,6 +728,7 @@ TEST_CASE("Wait on Timer", "[timer]") { // Test Manual Reset timer = Timer::CreateManualResetTimer(); + REQUIRE(timer); result = Wait(timer.get(), false, 1ms); REQUIRE(result == WaitResult::kTimeout); REQUIRE(timer->SetOnce(1ms)); // Signals it @@ -643,6 +739,7 @@ TEST_CASE("Wait on Timer", "[timer]") { // Test Synchronization timer = Timer::CreateSynchronizationTimer(); + REQUIRE(timer); result = Wait(timer.get(), false, 1ms); REQUIRE(result == WaitResult::kTimeout); REQUIRE(timer->SetOnce(1ms)); // Signals it @@ -757,7 +854,7 @@ TEST_CASE("Set and Test Current Thread ID", "[thread]") { // TODO(bwrsandman): Test on Thread object } -TEST_CASE("Set and Test Current Thread Name", "Thread") { +TEST_CASE("Set and Test Current Thread Name", "[thread]") { auto current_thread = Thread::GetCurrentThread(); REQUIRE(current_thread); auto old_thread_name = current_thread->name(); @@ -773,18 +870,25 @@ TEST_CASE("Create and Run Thread", "[thread]") { std::unique_ptr thread; WaitResult result; Thread::CreationParameters params = {}; - auto func = [] { Sleep(20ms); }; + std::atomic fence(0); + auto func = [&fence] { + REQUIRE(spin_wait_for(1s, [&] { return fence == 1; })); + fence++; + }; SECTION("Create most basic case of thread") { + fence = 0; 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); + fence++; + result = Wait(thread.get(), false, 1s); REQUIRE(result == WaitResult::kSuccess); } SECTION("Add thread name") { + fence = 0; std::string new_name = "Test thread name"; thread = Thread::Create(params, func); auto name = thread->name(); @@ -792,7 +896,8 @@ TEST_CASE("Create and Run Thread", "[thread]") { REQUIRE(name.empty()); thread->set_name(new_name); REQUIRE(thread->name() == new_name); - result = Wait(thread.get(), false, 50ms); + fence++; + result = Wait(thread.get(), false, 1s); REQUIRE(result == WaitResult::kSuccess); } @@ -802,10 +907,10 @@ TEST_CASE("Create and Run Thread", "[thread]") { Sleep(1ms); } }); - result = Wait(thread.get(), false, 50ms); + result = Wait(thread.get(), false, 1s); REQUIRE(result == WaitResult::kTimeout); thread->Terminate(-1); - result = Wait(thread.get(), false, 50ms); + result = Wait(thread.get(), false, 1s); REQUIRE(result == WaitResult::kSuccess); } @@ -813,8 +918,10 @@ TEST_CASE("Create and Run Thread", "[thread]") { thread = Thread::Create(params, [] { Thread::Exit(-1); FAIL("Function must not return"); + while (true) + ; }); - result = Wait(thread.get(), false, 50ms); + result = Wait(thread.get(), false, 1s); REQUIRE(result == WaitResult::kSuccess); } @@ -823,14 +930,16 @@ TEST_CASE("Create and Run Thread", "[thread]") { REQUIRE(result == WaitResult::kTimeout); } - SECTION("16kb stack size") { + SECTION("16Mb stack size") { params.stack_size = 16_MiB; thread = Thread::Create(params, [] { Thread::Exit(-1); FAIL("Function must not return"); + while (true) + ; }); REQUIRE(thread != nullptr); - result = Wait(thread.get(), false, 50ms); + result = Wait(thread.get(), false, 1s); REQUIRE(result == WaitResult::kSuccess); } @@ -917,18 +1026,19 @@ TEST_CASE("Test Thread QueueUserCallback", "[thread]") { has_finished = -1; thread = Thread::Create(params, [&has_finished, &order] { // Not using Alertable so callback is not registered + order++; // 1 Sleep(90ms); + order++; // 2 has_finished = std::atomic_fetch_add_explicit( &order, 1, std::memory_order::memory_order_relaxed); }); - result = Wait(thread.get(), true, 50ms); - REQUIRE(result == WaitResult::kTimeout); + REQUIRE(!spin_wait_for(50ms, [&] { return order == 2; })); REQUIRE(is_modified == -1); thread->QueueUserCallback(callback); - result = Wait(thread.get(), true, 100ms); + result = Wait(thread.get(), true, 200ms); REQUIRE(result == WaitResult::kSuccess); REQUIRE(is_modified == -1); - REQUIRE(has_finished == 0); + REQUIRE(has_finished == 2); // With alertable order = 0; @@ -936,18 +1046,19 @@ TEST_CASE("Test Thread QueueUserCallback", "[thread]") { has_finished = -1; thread = Thread::Create(params, [&has_finished, &order] { // Using Alertable so callback is registered + order++; // 1 AlertableSleep(90ms); + order++; // 3 has_finished = std::atomic_fetch_add_explicit( &order, 1, std::memory_order::memory_order_relaxed); }); - result = Wait(thread.get(), true, 50ms); - REQUIRE(result == WaitResult::kTimeout); + REQUIRE(!spin_wait_for(50ms, [&] { return order == 2; })); REQUIRE(is_modified == -1); thread->QueueUserCallback(callback); - result = Wait(thread.get(), true, 100ms); + result = Wait(thread.get(), true, 200ms); REQUIRE(result == WaitResult::kSuccess); - REQUIRE(is_modified == 0); - REQUIRE(has_finished == 1); + REQUIRE(is_modified == 1); + REQUIRE(has_finished == 3); // Test Exit command with QueueUserCallback order = 0; @@ -957,17 +1068,18 @@ TEST_CASE("Test Thread QueueUserCallback", "[thread]") { is_modified = std::atomic_fetch_add_explicit( &order, 1, std::memory_order::memory_order_relaxed); // Using Alertable so callback is registered - AlertableSleep(200ms); - has_finished = std::atomic_fetch_add_explicit( - &order, 1, std::memory_order::memory_order_relaxed); + order++; // 2 + AlertableSleep(1s); + FAIL("Thread should have been terminated during alertable sleep"); + while (true) + ; }); - result = Wait(thread.get(), true, 100ms); - REQUIRE(result == WaitResult::kTimeout); + REQUIRE(!spin_wait_for(100ms, [&] { return order == 3; })); // timeout thread->QueueUserCallback([] { Thread::Exit(0); }); result = Wait(thread.get(), true, 500ms); REQUIRE(result == WaitResult::kSuccess); REQUIRE(is_modified == 0); - REQUIRE(has_finished == -1); + REQUIRE(order == 2); // TODO(bwrsandman): Test alertable wait returning kUserCallback by using IO // callbacks. diff --git a/src/xenia/base/threading_posix.cc b/src/xenia/base/threading_posix.cc index 813758a4b..f0c315573 100644 --- a/src/xenia/base/threading_posix.cc +++ b/src/xenia/base/threading_posix.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2020 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -10,7 +10,7 @@ #include "xenia/base/threading.h" #include "xenia/base/assert.h" -#include "xenia/base/logging.h" +#include "xenia/base/delay_scheduler.h" #include "xenia/base/platform.h" #include @@ -23,7 +23,6 @@ #include #include #include -#include #include #include @@ -34,6 +33,19 @@ #include "xenia/base/string_util.h" #endif +#if XE_PLATFORM_LINUX +// SIGEV_THREAD_ID in timer_create(...) is a Linux extension +#define XE_HAS_SIGEV_THREAD_ID 1 +#ifdef __GLIBC__ +#define sigev_notify_thread_id _sigev_un._tid +#endif +#if __GLIBC__ == 2 && __GLIBC_MINOR__ < 30 +#define gettid() syscall(SYS_gettid) +#endif +#else +#define XE_HAS_SIGEV_THREAD_ID 0 +#endif + namespace xe { namespace threading { @@ -66,6 +78,47 @@ void AndroidShutdown() { } #endif +// This is separately allocated for each (`HighResolution`)`Timer` object. It +// will be cleaned up some time (`timers_garbage_collector_delay`) after the +// posix timer was canceled because posix `timer_delete(...)` does not remove +// pending timer signals. +// https://stackoverflow.com/questions/49756114/linux-timer-pending-signal +struct timer_callback_info_t { + std::atomic_bool disarmed; +#if !XE_HAS_SIGEV_THREAD_ID + pthread_t target_thread; +#endif + std::function callback; + void* userdata; + + timer_callback_info_t(std::function callback) + : disarmed(false), +#if !XE_HAS_SIGEV_THREAD_ID + target_thread(), +#endif + callback(callback), + userdata(nullptr) { + } +}; + +// GC for timer signal info structs: +constexpr uint_fast8_t timers_garbage_collector_scale_ = +#if XE_HAS_SIGEV_THREAD_ID + 1; +#else + 2; +#endif +DelayScheduler timers_garbage_collector_( + 512 * timers_garbage_collector_scale_, + [](timer_callback_info_t* info) { + assert_not_null(info); + delete info; + }, + true); +// Delay we have to assume it takes to clear all pending signals (maximum): +constexpr auto timers_garbage_collector_delay = + std::chrono::milliseconds(100 * timers_garbage_collector_scale_); + template inline timespec DurationToTimeSpec( std::chrono::duration<_Rep, _Period> duration) { @@ -182,9 +235,20 @@ bool SetTlsValue(TlsHandle handle, uintptr_t value) { class PosixHighResolutionTimer : public HighResolutionTimer { public: explicit PosixHighResolutionTimer(std::function callback) - : callback_(std::move(callback)), valid_(false) {} + : valid_(false) { + callback_info_ = new timer_callback_info_t(std::move(callback)); + } ~PosixHighResolutionTimer() override { - if (valid_) timer_delete(timer_); + if (valid_) { + callback_info_->disarmed = true; + timer_delete(timerid_); + // Deliberately leaks memory when wait queue is full instead of blogs, + // check logs + static_cast(timers_garbage_collector_.TryScheduleAfter( + callback_info_, timers_garbage_collector_delay)); + } else { + delete callback_info_; + } } bool Initialize(std::chrono::milliseconds period) { @@ -195,22 +259,31 @@ class PosixHighResolutionTimer : public HighResolutionTimer { } // Create timer sigevent sev{}; +#if XE_HAS_SIGEV_THREAD_ID + sev.sigev_notify = SIGEV_SIGNAL | SIGEV_THREAD_ID; + sev.sigev_notify_thread_id = gettid(); +#else sev.sigev_notify = SIGEV_SIGNAL; + callback_info_->target_thread = pthread_self(); +#endif sev.sigev_signo = GetSystemSignal(SignalType::kHighResolutionTimer); - sev.sigev_value.sival_ptr = (void*)&callback_; - if (timer_create(CLOCK_MONOTONIC, &sev, &timer_) == -1) return false; + sev.sigev_value.sival_ptr = callback_info_; + if (timer_create(CLOCK_MONOTONIC, &sev, &timerid_) == -1) return false; // Start timer itimerspec its{}; its.it_value = DurationToTimeSpec(period); its.it_interval = its.it_value; - valid_ = timer_settime(timer_, 0, &its, nullptr) != -1; + valid_ = timer_settime(timerid_, 0, &its, nullptr) != -1; + if (!valid_) { + timer_delete(timerid_); + } return valid_; } private: - std::function callback_; - timer_t timer_; + timer_callback_info_t* callback_info_; + timer_t timerid_; bool valid_; // all values for timer_t are legal so we need this }; @@ -253,37 +326,37 @@ class PosixConditionBase { static std::pair WaitMultiple( std::vector&& handles, bool wait_all, std::chrono::milliseconds timeout) { - using iter_t = std::vector::const_iterator; - bool executed; - auto predicate = [](auto h) { return h->signaled(); }; + assert_true(handles.size() > 0); // Construct a condition for all or any depending on wait_all - auto operation = wait_all ? std::all_of - : std::any_of; - auto aggregate = [&handles, operation, predicate] { - return operation(handles.cbegin(), handles.cend(), predicate); - }; + std::function predicate; + { + using iter_t = std::vector::const_iterator; + const auto predicate_inner = [](auto h) { return h->signaled(); }; + const auto operation = + wait_all ? std::all_of + : std::any_of; + predicate = [&handles, operation, predicate_inner] { + return operation(handles.cbegin(), handles.cend(), predicate_inner); + }; + } // TODO(bwrsandman, Triang3l) This is controversial, see issue #1677 // This will probably cause a deadlock on the next thread doing any waiting // if the thread is suspended between locking and waiting std::unique_lock lock(PosixConditionBase::mutex_); - // Check if the aggregate lambda (all or any) is already satisfied - if (aggregate()) { - executed = true; + bool wait_success = true; + // If the timeout is infinite, wait without timeout. + // The predicate will be checked before beginning the wait + if (timeout == std::chrono::milliseconds::max()) { + PosixConditionBase::cond_.wait(lock, predicate); } else { - // If the aggregate is not yet satisfied and the timeout is infinite, - // wait without timeout. - if (timeout == std::chrono::milliseconds::max()) { - PosixConditionBase::cond_.wait(lock, aggregate); - executed = true; - } else { - // Wait with timeout. - executed = PosixConditionBase::cond_.wait_for(lock, timeout, aggregate); - } + // Wait with timeout. + wait_success = + PosixConditionBase::cond_.wait_for(lock, timeout, predicate); } - if (executed) { + if (wait_success) { auto first_signaled = std::numeric_limits::max(); for (auto i = 0u; i < handles.size(); ++i) { if (handles[i]->signaled()) { @@ -294,6 +367,7 @@ class PosixConditionBase { if (!wait_all) break; } } + assert_true(std::numeric_limits::max() != first_signaled); return std::make_pair(WaitResult::kSuccess, first_signaled); } else { return std::make_pair(WaitResult::kTimeout, 0); @@ -329,13 +403,7 @@ class PosixCondition : public PosixConditionBase { bool Signal() override { auto lock = std::unique_lock(mutex_); signal_ = true; - if (manual_reset_) { - cond_.notify_all(); - } else { - // FIXME(bwrsandman): Potential cause for deadlock - // See issue #1678 for possible fix and discussion - cond_.notify_one(); - } + cond_.notify_all(); return true; } @@ -402,7 +470,7 @@ class PosixCondition : public PosixConditionBase { --count_; // Free to be acquired by another thread if (count_ == 0) { - cond_.notify_one(); + cond_.notify_all(); } return true; } @@ -427,33 +495,47 @@ template <> class PosixCondition : public PosixConditionBase { public: explicit PosixCondition(bool manual_reset) - : callback_(), - timer_(nullptr), + : timer_(nullptr), + callback_info_(nullptr), signal_(false), manual_reset_(manual_reset) {} virtual ~PosixCondition() { Cancel(); } bool Signal() override { - CompletionRoutine(); + std::lock_guard lock(mutex_); + signal_ = true; + cond_.notify_all(); return true; } // TODO(bwrsandman): due_times of under 1ms deadlock under travis + // TODO(joellinn): This is likely due to deadlock on mutex_ if Signal() is + // called from signal_handler running in Thread A while thread A was still in + // Set(...) routine inside the lock bool Set(std::chrono::nanoseconds due_time, std::chrono::milliseconds period, std::function opt_callback = nullptr) { - std::lock_guard lock(mutex_); + Cancel(); - callback_ = std::move(opt_callback); + std::lock_guard lock(mutex_); + callback_info_ = new timer_callback_info_t(std::move(opt_callback)); + callback_info_->userdata = this; signal_ = false; // Create timer - if (timer_ == nullptr) { - sigevent sev{}; - sev.sigev_notify = SIGEV_SIGNAL; - sev.sigev_signo = GetSystemSignal(SignalType::kTimer); - sev.sigev_value.sival_ptr = this; - if (timer_create(CLOCK_MONOTONIC, &sev, &timer_) == -1) return false; + sigevent sev{}; +#if XE_HAS_SIGEV_THREAD_ID + sev.sigev_notify = SIGEV_SIGNAL | SIGEV_THREAD_ID; + sev.sigev_notify_thread_id = gettid(); +#else + sev.sigev_notify = SIGEV_SIGNAL; + callback_info_->target_thread = pthread_self(); +#endif + sev.sigev_signo = GetSystemSignal(SignalType::kTimer); + sev.sigev_value.sival_ptr = callback_info_; + if (timer_create(CLOCK_MONOTONIC, &sev, &timer_) == -1) { + delete callback_info_; + return false; } // Start timer @@ -463,30 +545,16 @@ class PosixCondition : public PosixConditionBase { return timer_settime(timer_, 0, &its, nullptr) == 0; } - void CompletionRoutine() { - // As the callback may reset the timer, store local. - std::function callback; - { - std::lock_guard lock(mutex_); - // Store callback - if (callback_) callback = callback_; - signal_ = true; - if (manual_reset_) { - cond_.notify_all(); - } else { - cond_.notify_one(); - } - } - // Call callback - if (callback) callback(); - } - bool Cancel() { std::lock_guard lock(mutex_); bool result = true; if (timer_) { + callback_info_->disarmed = true; result = timer_delete(timer_) == 0; timer_ = nullptr; + static_cast(timers_garbage_collector_.TryScheduleAfter( + callback_info_, timers_garbage_collector_delay)); + callback_info_ = nullptr; } return result; } @@ -502,8 +570,8 @@ class PosixCondition : public PosixConditionBase { signal_ = false; } } - std::function callback_; timer_t timer_; + timer_callback_info_t* callback_info_; volatile bool signal_; const bool manual_reset_; }; @@ -984,6 +1052,10 @@ class PosixSemaphore : public PosixConditionHandle { std::unique_ptr Semaphore::Create(int initial_count, int maximum_count) { + if (initial_count < 0 || initial_count > maximum_count || + maximum_count <= 0) { + return nullptr; + } return std::make_unique(initial_count, maximum_count); } @@ -1182,15 +1254,50 @@ static void signal_handler(int signal, siginfo_t* info, void* /*context*/) { switch (GetSystemSignalType(signal)) { case SignalType::kHighResolutionTimer: { assert_not_null(info->si_value.sival_ptr); - auto callback = - *static_cast*>(info->si_value.sival_ptr); - callback(); + auto timer_info = + reinterpret_cast(info->si_value.sival_ptr); + if (!timer_info->disarmed) { +#if XE_HAS_SIGEV_THREAD_ID + { +#else + if (pthread_self() != timer_info->target_thread) { + sigval info_inner{}; + info_inner.sival_ptr = timer_info; + const auto queueres = pthread_sigqueue( + timer_info->target_thread, + GetSystemSignal(SignalType::kHighResolutionTimer), info_inner); + assert_zero(queueres); + } else { +#endif + timer_info->callback(); + } + } } break; case SignalType::kTimer: { assert_not_null(info->si_value.sival_ptr); - auto pTimer = - static_cast*>(info->si_value.sival_ptr); - pTimer->CompletionRoutine(); + auto timer_info = + reinterpret_cast(info->si_value.sival_ptr); + if (!timer_info->disarmed) { + assert_not_null(timer_info->userdata); + auto timer = static_cast*>(timer_info->userdata); +#if XE_HAS_SIGEV_THREAD_ID + { +#else + if (pthread_self() != timer_info->target_thread) { + sigval info_inner{}; + info_inner.sival_ptr = timer_info; + const auto queueres = + pthread_sigqueue(timer_info->target_thread, + GetSystemSignal(SignalType::kTimer), info_inner); + assert_zero(queueres); + } else { +#endif + timer->Signal(); + if (timer_info->callback) { + timer_info->callback(); + } + } + } } break; case SignalType::kThreadSuspend: { assert_not_null(current_thread_); diff --git a/src/xenia/base/threading_win.cc b/src/xenia/base/threading_win.cc index 60c746e55..cf760d603 100644 --- a/src/xenia/base/threading_win.cc +++ b/src/xenia/base/threading_win.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2020 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -12,6 +12,9 @@ #include "xenia/base/platform_win.h" #include "xenia/base/threading.h" +#define LOG_LASTERROR() \ + { XELOGI("Win32 Error 0x{:08X} in " __FUNCTION__ "(...)", GetLastError()); } + typedef HANDLE (*SetThreadDescriptionFn)(HANDLE hThread, PCWSTR lpThreadDescription); @@ -63,7 +66,8 @@ static void set_name(HANDLE thread, const std::string_view name) { auto func = (SetThreadDescriptionFn)GetProcAddress(kernel, "SetThreadDescription"); if (func) { - func(thread, reinterpret_cast(xe::to_utf16(name).c_str())); + auto u16name = xe::to_utf16(name); + func(thread, reinterpret_cast(u16name.c_str())); } } raise_thread_name_exception(thread, std::string(name)); @@ -153,7 +157,9 @@ std::unique_ptr HighResolutionTimer::CreateRepeating( template class Win32Handle : public T { public: - explicit Win32Handle(HANDLE handle) : handle_(handle) {} + explicit Win32Handle(HANDLE handle) : handle_(handle) { + assert_not_null(handle); + } ~Win32Handle() override { CloseHandle(handle_); handle_ = nullptr; @@ -248,13 +254,25 @@ class Win32Event : public Win32Handle { }; std::unique_ptr Event::CreateManualResetEvent(bool initial_state) { - return std::make_unique( - CreateEvent(nullptr, TRUE, initial_state ? TRUE : FALSE, nullptr)); + HANDLE handle = + CreateEvent(nullptr, TRUE, initial_state ? TRUE : FALSE, nullptr); + if (handle) { + return std::make_unique(handle); + } else { + LOG_LASTERROR(); + return nullptr; + } } std::unique_ptr Event::CreateAutoResetEvent(bool initial_state) { - return std::make_unique( - CreateEvent(nullptr, FALSE, initial_state ? TRUE : FALSE, nullptr)); + HANDLE handle = + CreateEvent(nullptr, FALSE, initial_state ? TRUE : FALSE, nullptr); + if (handle) { + return std::make_unique(handle); + } else { + LOG_LASTERROR(); + return nullptr; + } } class Win32Semaphore : public Win32Handle { @@ -271,8 +289,14 @@ class Win32Semaphore : public Win32Handle { std::unique_ptr Semaphore::Create(int initial_count, int maximum_count) { - return std::make_unique( - CreateSemaphore(nullptr, initial_count, maximum_count, nullptr)); + HANDLE handle = + CreateSemaphore(nullptr, initial_count, maximum_count, nullptr); + if (handle) { + return std::make_unique(handle); + } else { + LOG_LASTERROR(); + return nullptr; + } } class Win32Mutant : public Win32Handle { @@ -283,8 +307,13 @@ class Win32Mutant : public Win32Handle { }; std::unique_ptr Mutant::Create(bool initial_owner) { - return std::make_unique( - CreateMutex(nullptr, initial_owner ? TRUE : FALSE, nullptr)); + HANDLE handle = CreateMutex(nullptr, initial_owner ? TRUE : FALSE, nullptr); + if (handle) { + return std::make_unique(handle); + } else { + LOG_LASTERROR(); + return nullptr; + } } class Win32Timer : public Win32Handle { @@ -344,11 +373,23 @@ class Win32Timer : public Win32Handle { }; std::unique_ptr Timer::CreateManualResetTimer() { - return std::make_unique(CreateWaitableTimer(NULL, TRUE, NULL)); + HANDLE handle = CreateWaitableTimer(NULL, TRUE, NULL); + if (handle) { + return std::make_unique(handle); + } else { + LOG_LASTERROR(); + return nullptr; + } } std::unique_ptr Timer::CreateSynchronizationTimer() { - return std::make_unique(CreateWaitableTimer(NULL, FALSE, NULL)); + HANDLE handle = CreateWaitableTimer(NULL, FALSE, NULL); + if (handle) { + return std::make_unique(handle); + } else { + LOG_LASTERROR(); + return nullptr; + } } class Win32Thread : public Win32Handle { @@ -450,15 +491,13 @@ std::unique_ptr Thread::Create(CreationParameters params, HANDLE handle = CreateThread(NULL, params.stack_size, ThreadStartRoutine, start_data, params.create_suspended ? CREATE_SUSPENDED : 0, NULL); - if (handle == INVALID_HANDLE_VALUE) { - // TODO(benvanik): pass back? - auto last_error = GetLastError(); - XELOGE("Unable to CreateThread: {}", last_error); + if (handle) { + return std::make_unique(handle); + } else { + LOG_LASTERROR(); delete start_data; return nullptr; } - - return std::make_unique(handle); } Thread* Thread::GetCurrentThread() { diff --git a/src/xenia/cpu/backend/x64/x64_emitter.cc b/src/xenia/cpu/backend/x64/x64_emitter.cc index 5cd6780be..97b14e03e 100644 --- a/src/xenia/cpu/backend/x64/x64_emitter.cc +++ b/src/xenia/cpu/backend/x64/x64_emitter.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2020 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -653,6 +653,7 @@ void X64Emitter::MovMem64(const Xbyak::RegExp& addr, uint64_t v) { static const vec128_t xmm_consts[] = { /* XMMZero */ vec128f(0.0f), /* XMMOne */ vec128f(1.0f), + /* XMMOnePD */ vec128d(1.0), /* XMMNegativeOne */ vec128f(-1.0f, -1.0f, -1.0f, -1.0f), /* XMMFFFF */ vec128i(0xFFFFFFFFu, 0xFFFFFFFFu, 0xFFFFFFFFu, 0xFFFFFFFFu), diff --git a/src/xenia/cpu/backend/x64/x64_emitter.h b/src/xenia/cpu/backend/x64/x64_emitter.h index 840e355fc..247d6175c 100644 --- a/src/xenia/cpu/backend/x64/x64_emitter.h +++ b/src/xenia/cpu/backend/x64/x64_emitter.h @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2019 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -22,7 +22,6 @@ // NOTE: must be included last as it expects windows.h to already be included. #include "third_party/xbyak/xbyak/xbyak.h" -#include "third_party/xbyak/xbyak/xbyak_bin2hex.h" #include "third_party/xbyak/xbyak/xbyak_util.h" namespace xe { @@ -49,6 +48,7 @@ enum RegisterFlags { enum XmmConst { XMMZero = 0, XMMOne, + XMMOnePD, XMMNegativeOne, XMMFFFF, XMMMaskX16Y16, diff --git a/src/xenia/cpu/backend/x64/x64_seq_vector.cc b/src/xenia/cpu/backend/x64/x64_seq_vector.cc index 4daea260b..7cf4650b5 100644 --- a/src/xenia/cpu/backend/x64/x64_seq_vector.cc +++ b/src/xenia/cpu/backend/x64/x64_seq_vector.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2018 Xenia Developers. All rights reserved. * + * Copyright 2022 Xenia Developers. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -1287,7 +1287,6 @@ static __m128i EmulateVectorRotateLeft(void*, __m128i src1, __m128i src2) { return _mm_load_si128(reinterpret_cast<__m128i*>(value)); } -// TODO(benvanik): AVX512 has a native variable rotate (rolv). struct VECTOR_ROTATE_LEFT_V128 : Sequence> { @@ -1318,7 +1317,9 @@ struct VECTOR_ROTATE_LEFT_V128 e.vmovaps(i.dest, e.xmm0); break; case INT32_TYPE: { - if (e.IsFeatureEnabled(kX64EmitAVX2)) { + if (e.IsFeatureEnabled(kX64EmitAVX512Ortho)) { + e.vprolvd(i.dest, i.src1, i.src2); + } else if (e.IsFeatureEnabled(kX64EmitAVX2)) { Xmm temp = i.dest; if (i.dest == i.src1 || i.dest == i.src2) { temp = e.xmm2; @@ -2683,4 +2684,4 @@ EMITTER_OPCODE_TABLE(OPCODE_UNPACK, UNPACK); } // namespace x64 } // namespace backend } // namespace cpu -} // namespace xe \ No newline at end of file +} // namespace xe diff --git a/src/xenia/cpu/backend/x64/x64_sequences.cc b/src/xenia/cpu/backend/x64/x64_sequences.cc index 62f6e9a3f..34cef4f7d 100644 --- a/src/xenia/cpu/backend/x64/x64_sequences.cc +++ b/src/xenia/cpu/backend/x64/x64_sequences.cc @@ -1,8 +1,8 @@ -/** +/** ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2020 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -2354,21 +2354,39 @@ EMITTER_OPCODE_TABLE(OPCODE_SQRT, SQRT_F32, SQRT_F64, SQRT_V128); // ============================================================================ // OPCODE_RSQRT // ============================================================================ +// Altivec guarantees an error of < 1/4096 for vrsqrtefp while AVX only gives +// < 1.5*2^-12 ≈ 1/2730 for vrsqrtps. struct RSQRT_F32 : Sequence> { static void Emit(X64Emitter& e, const EmitArgType& i) { - e.vrsqrtss(i.dest, i.src1); + if (e.IsFeatureEnabled(kX64EmitAVX512Ortho)) { + e.vrsqrt14ss(i.dest, i.src1, i.src1); + } else { + e.vmovaps(e.xmm0, e.GetXmmConstPtr(XMMOne)); + e.vsqrtss(e.xmm1, i.src1, i.src1); + e.vdivss(i.dest, e.xmm0, e.xmm1); + } } }; struct RSQRT_F64 : Sequence> { static void Emit(X64Emitter& e, const EmitArgType& i) { - e.vcvtsd2ss(i.dest, i.src1); - e.vrsqrtss(i.dest, i.dest); - e.vcvtss2sd(i.dest, i.dest); + if (e.IsFeatureEnabled(kX64EmitAVX512Ortho)) { + e.vrsqrt14sd(i.dest, i.src1, i.src1); + } else { + e.vmovapd(e.xmm0, e.GetXmmConstPtr(XMMOnePD)); + e.vsqrtsd(e.xmm1, i.src1, i.src1); + e.vdivsd(i.dest, e.xmm0, e.xmm1); + } } }; struct RSQRT_V128 : Sequence> { static void Emit(X64Emitter& e, const EmitArgType& i) { - e.vrsqrtps(i.dest, i.src1); + if (e.IsFeatureEnabled(kX64EmitAVX512Ortho)) { + e.vrsqrt14ps(i.dest, i.src1); + } else { + e.vmovaps(e.xmm0, e.GetXmmConstPtr(XMMOne)); + e.vsqrtps(e.xmm1, i.src1); + e.vdivps(i.dest, e.xmm0, e.xmm1); + } } }; EMITTER_OPCODE_TABLE(OPCODE_RSQRT, RSQRT_F32, RSQRT_F64, RSQRT_V128); @@ -2376,21 +2394,37 @@ EMITTER_OPCODE_TABLE(OPCODE_RSQRT, RSQRT_F32, RSQRT_F64, RSQRT_V128); // ============================================================================ // OPCODE_RECIP // ============================================================================ +// Altivec guarantees an error of < 1/4096 for vrefp while AVX only gives +// < 1.5*2^-12 ≈ 1/2730 for rcpps. This breaks camp, horse and random event +// spawning, breaks cactus collision as well as flickering grass in 5454082B struct RECIP_F32 : Sequence> { static void Emit(X64Emitter& e, const EmitArgType& i) { - e.vrcpss(i.dest, i.src1); + if (e.IsFeatureEnabled(kX64EmitAVX512Ortho)) { + e.vrcp14ss(i.dest, i.src1, i.src1); + } else { + e.vmovaps(e.xmm0, e.GetXmmConstPtr(XMMOne)); + e.vdivss(i.dest, e.xmm0, i.src1); + } } }; struct RECIP_F64 : Sequence> { static void Emit(X64Emitter& e, const EmitArgType& i) { - e.vcvtsd2ss(i.dest, i.src1); - e.vrcpss(i.dest, i.dest); - e.vcvtss2sd(i.dest, i.dest); + if (e.IsFeatureEnabled(kX64EmitAVX512Ortho)) { + e.vrcp14sd(i.dest, i.src1, i.src1); + } else { + e.vmovapd(e.xmm0, e.GetXmmConstPtr(XMMOnePD)); + e.vdivsd(i.dest, e.xmm0, i.src1); + } } }; struct RECIP_V128 : Sequence> { static void Emit(X64Emitter& e, const EmitArgType& i) { - e.vrcpps(i.dest, i.src1); + if (e.IsFeatureEnabled(kX64EmitAVX512Ortho)) { + e.vrcp14ps(i.dest, i.src1); + } else { + e.vmovaps(e.xmm0, e.GetXmmConstPtr(XMMOne)); + e.vdivps(i.dest, e.xmm0, i.src1); + } } }; EMITTER_OPCODE_TABLE(OPCODE_RECIP, RECIP_F32, RECIP_F64, RECIP_V128); diff --git a/src/xenia/cpu/testing/vector_rotate_left_test.cc b/src/xenia/cpu/testing/vector_rotate_left_test.cc index 406caa157..2f378246c 100644 --- a/src/xenia/cpu/testing/vector_rotate_left_test.cc +++ b/src/xenia/cpu/testing/vector_rotate_left_test.cc @@ -2,12 +2,11 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2014 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ -#include "third_party/xbyak/xbyak/xbyak_bin2hex.h" #include "xenia/cpu/testing/util.h" using namespace xe; @@ -23,16 +22,17 @@ TEST_CASE("VECTOR_ROTATE_LEFT_I8", "[instr]") { }); test.Run( [](PPCContext* ctx) { - ctx->v[4] = vec128b(B00000001); + ctx->v[4] = vec128b(0b00000001); ctx->v[5] = vec128b(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15); }, [](PPCContext* ctx) { auto result = ctx->v[3]; - REQUIRE(result == vec128b(B00000001, B00000010, B00000100, B00001000, - B00010000, B00100000, B01000000, B10000000, - B00000001, B00000010, B00000100, B00001000, - B00010000, B00100000, B01000000, B10000000)); + REQUIRE(result == + vec128b(0b00000001, 0b00000010, 0b00000100, 0b00001000, + 0b00010000, 0b00100000, 0b01000000, 0b10000000, + 0b00000001, 0b00000010, 0b00000100, 0b00001000, + 0b00010000, 0b00100000, 0b01000000, 0b10000000)); }); } @@ -62,7 +62,7 @@ TEST_CASE("VECTOR_ROTATE_LEFT_I32", "[instr]") { test.Run( [](PPCContext* ctx) { ctx->v[4] = vec128i(0x00000001, 0x00000001, 0x80000000, 0x80000000); - ctx->v[5] = vec128i(0, 1, 1, 2); + ctx->v[5] = vec128i(0, 1, 33, 2); }, [](PPCContext* ctx) { auto result = ctx->v[3]; diff --git a/src/xenia/gpu/command_processor.cc b/src/xenia/gpu/command_processor.cc index 7a4ac4e23..0fe9d7cbf 100644 --- a/src/xenia/gpu/command_processor.cc +++ b/src/xenia/gpu/command_processor.cc @@ -41,7 +41,9 @@ CommandProcessor::CommandProcessor(GraphicsSystem* graphics_system, trace_writer_(graphics_system->memory()->physical_membase()), worker_running_(true), write_ptr_index_event_(xe::threading::Event::CreateAutoResetEvent(false)), - write_ptr_index_(0) {} + write_ptr_index_(0) { + assert_not_null(write_ptr_index_event_); +} CommandProcessor::~CommandProcessor() = default; diff --git a/src/xenia/gpu/d3d12/pipeline_cache.cc b/src/xenia/gpu/d3d12/pipeline_cache.cc index 01b0b9cd8..75a096f48 100644 --- a/src/xenia/gpu/d3d12/pipeline_cache.cc +++ b/src/xenia/gpu/d3d12/pipeline_cache.cc @@ -149,6 +149,7 @@ bool PipelineCache::Initialize() { creation_threads_busy_ = 0; creation_completion_event_ = xe::threading::Event::CreateManualResetEvent(true); + assert_not_null(creation_completion_event_); creation_completion_set_event_ = false; creation_threads_shutdown_from_ = SIZE_MAX; if (cvars::d3d12_pipeline_creation_threads != 0) { @@ -164,6 +165,7 @@ bool PipelineCache::Initialize() { for (size_t i = 0; i < creation_thread_count; ++i) { std::unique_ptr creation_thread = xe::threading::Thread::Create({}, [this, i]() { CreationThread(i); }); + assert_not_null(creation_thread); creation_thread->set_name("D3D12 Pipelines"); creation_threads_.push_back(std::move(creation_thread)); } @@ -540,9 +542,11 @@ void PipelineCache::InitializeShaderStorage( } while (shader_translation_threads.size() < shader_translation_threads_needed) { - shader_translation_threads.push_back(xe::threading::Thread::Create( - {}, shader_translation_thread_function)); - shader_translation_threads.back()->set_name("Shader Translation"); + auto thread = xe::threading::Thread::Create( + {}, shader_translation_thread_function); + assert_not_null(thread); + thread->set_name("Shader Translation"); + shader_translation_threads.push_back(std::move(thread)); } // Request ucode information gathering and translation of all the needed // shaders. @@ -606,6 +610,7 @@ void PipelineCache::InitializeShaderStorage( xe::threading::Thread::Create({}, [this, creation_thread_index]() { CreationThread(creation_thread_index); }); + assert_not_null(creation_thread); creation_thread->set_name("D3D12 Pipelines"); creation_threads_.push_back(std::move(creation_thread)); } @@ -771,6 +776,8 @@ void PipelineCache::InitializeShaderStorage( storage_write_thread_shutdown_ = false; storage_write_thread_ = xe::threading::Thread::Create({}, [this]() { StorageWriteThread(); }); + assert_not_null(storage_write_thread_); + storage_write_thread_->set_name("D3D12 Storage writer"); } void PipelineCache::ShutdownShaderStorage() { diff --git a/src/xenia/gpu/trace_player.cc b/src/xenia/gpu/trace_player.cc index 54db1156c..a141c9b8d 100644 --- a/src/xenia/gpu/trace_player.cc +++ b/src/xenia/gpu/trace_player.cc @@ -30,6 +30,7 @@ TracePlayer::TracePlayer(GraphicsSystem* graphics_system) kMemoryProtectRead | kMemoryProtectWrite); playback_event_ = xe::threading::Event::CreateAutoResetEvent(false); + assert_not_null(playback_event_); } TracePlayer::~TracePlayer() { delete[] edram_snapshot_; } diff --git a/src/xenia/kernel/util/shim_utils.h b/src/xenia/kernel/util/shim_utils.h index 0b6551615..579305a72 100644 --- a/src/xenia/kernel/util/shim_utils.h +++ b/src/xenia/kernel/util/shim_utils.h @@ -84,18 +84,18 @@ inline uint64_t get_arg_64(PPCContext* ppc_context, uint8_t index) { return SHIM_MEM_64(stack_address); } -inline std::string TranslateAnsiString(const Memory* memory, - const X_ANSI_STRING* ansi_string) { +inline std::string_view TranslateAnsiString(const Memory* memory, + const X_ANSI_STRING* ansi_string) { if (!ansi_string || !ansi_string->length) { return ""; } - return std::string( + return std::string_view( memory->TranslateVirtual(ansi_string->pointer), ansi_string->length); } -inline std::string TranslateAnsiStringAddress(const Memory* memory, - uint32_t guest_address) { +inline std::string_view TranslateAnsiStringAddress(const Memory* memory, + uint32_t guest_address) { if (!guest_address) { return ""; } @@ -440,7 +440,7 @@ inline void AppendParam(StringBuffer* string_buffer, if (record) { auto name_string = kernel_memory()->TranslateVirtual(record->name_ptr); - std::string name = + std::string_view name = name_string == nullptr ? "(null)" : util::TranslateAnsiString(kernel_memory(), name_string); diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_io.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_io.cc index 2e2022bc6..3e7a700ef 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_io.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_io.cc @@ -614,8 +614,7 @@ dword_result_t NtOpenSymbolicLinkObject_entry( auto object_name = kernel_memory()->TranslateVirtual(object_attrs->name_ptr); - std::string target_path = - util::TranslateAnsiString(kernel_memory(), object_name); + auto target_path = util::TranslateAnsiString(kernel_memory(), object_name); // Enforce that the path is ASCII. if (!IsValidPath(target_path, false)) { diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_ob.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_ob.cc index 1eec0e430..e09096eb0 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_ob.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_ob.cc @@ -7,6 +7,7 @@ ****************************************************************************** */ +#include "xenia/base/assert.h" #include "xenia/base/logging.h" #include "xenia/kernel/kernel_state.h" #include "xenia/kernel/util/shim_utils.h" @@ -31,10 +32,15 @@ dword_result_t ObOpenObjectByName_entry(lpunknown_t obj_attributes_ptr, // r5 = 0 // r6 = out_ptr (handle?) - auto name = util::TranslateAnsiStringAddress( - kernel_memory(), - xe::load_and_swap(kernel_memory()->TranslateVirtual( - obj_attributes_ptr.guest_address() + 4))); + if (!obj_attributes_ptr) { + return X_STATUS_INVALID_PARAMETER; + } + + auto obj_attributes = kernel_memory()->TranslateVirtual( + obj_attributes_ptr); + assert_true(obj_attributes->name_ptr != 0); + auto name = util::TranslateAnsiStringAddress(kernel_memory(), + obj_attributes->name_ptr); X_HANDLE handle = X_INVALID_HANDLE_VALUE; X_STATUS result = @@ -144,10 +150,10 @@ DECLARE_XBOXKRNL_EXPORT1(ObDereferenceObject, kNone, kImplemented); dword_result_t ObCreateSymbolicLink_entry(pointer_t path_ptr, pointer_t target_ptr) { - auto path = util::TranslateAnsiString(kernel_memory(), path_ptr); - auto target = util::TranslateAnsiString(kernel_memory(), target_ptr); - path = xe::utf8::canonicalize_guest_path(path); - target = xe::utf8::canonicalize_guest_path(target); + auto path = xe::utf8::canonicalize_guest_path( + util::TranslateAnsiString(kernel_memory(), path_ptr)); + auto target = xe::utf8::canonicalize_guest_path( + util::TranslateAnsiString(kernel_memory(), target_ptr)); if (xe::utf8::starts_with(path, u8"\\??\\")) { path = path.substr(4); // Strip the full qualifier diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc index f913643eb..9da95a84d 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_threading.cc @@ -74,10 +74,12 @@ object_ref LookupNamedObject(KernelState* kernel_state, if (!obj_attributes_ptr) { return nullptr; } - auto name = util::TranslateAnsiStringAddress( - kernel_state->memory(), - xe::load_and_swap( - kernel_state->memory()->TranslateVirtual(obj_attributes_ptr + 4))); + auto obj_attributes = + kernel_state->memory()->TranslateVirtual( + obj_attributes_ptr); + assert_true(obj_attributes->name_ptr != 0); + auto name = util::TranslateAnsiStringAddress(kernel_state->memory(), + obj_attributes->name_ptr); if (!name.empty()) { X_HANDLE handle = X_INVALID_HANDLE_VALUE; X_RESULT result = @@ -453,7 +455,7 @@ dword_result_t NtCreateEvent_entry( existing_object->RetainHandle(); *handle_ptr = existing_object->handle(); } - return X_STATUS_SUCCESS; + return X_STATUS_OBJECT_NAME_EXISTS; } else { return X_STATUS_INVALID_HANDLE; } @@ -574,7 +576,7 @@ DECLARE_XBOXKRNL_EXPORT1(KeReleaseSemaphore, kThreading, kImplemented); dword_result_t NtCreateSemaphore_entry(lpdword_t handle_ptr, lpvoid_t obj_attributes_ptr, dword_t count, dword_t limit) { - // Check for an existing timer with the same name. + // Check for an existing semaphore with the same name. auto existing_object = LookupNamedObject(kernel_state(), obj_attributes_ptr); if (existing_object) { @@ -583,14 +585,20 @@ dword_result_t NtCreateSemaphore_entry(lpdword_t handle_ptr, existing_object->RetainHandle(); *handle_ptr = existing_object->handle(); } - return X_STATUS_SUCCESS; + return X_STATUS_OBJECT_NAME_EXISTS; } else { return X_STATUS_INVALID_HANDLE; } } auto sem = object_ref(new XSemaphore(kernel_state())); - sem->Initialize((int32_t)count, (int32_t)limit); + if (!sem->Initialize((int32_t)count, (int32_t)limit)) { + if (handle_ptr) { + *handle_ptr = 0; + } + sem->ReleaseHandle(); + return X_STATUS_INVALID_PARAMETER; + } // obj_attributes may have a name inside of it, if != NULL. if (obj_attributes_ptr) { @@ -639,7 +647,7 @@ dword_result_t NtCreateMutant_entry( existing_object->RetainHandle(); *handle_out = existing_object->handle(); } - return X_STATUS_SUCCESS; + return X_STATUS_OBJECT_NAME_EXISTS; } else { return X_STATUS_INVALID_HANDLE; } @@ -701,7 +709,7 @@ dword_result_t NtCreateTimer_entry(lpdword_t handle_ptr, existing_object->RetainHandle(); *handle_ptr = existing_object->handle(); } - return X_STATUS_SUCCESS; + return X_STATUS_OBJECT_NAME_EXISTS; } else { return X_STATUS_INVALID_HANDLE; } diff --git a/src/xenia/kernel/xevent.cc b/src/xenia/kernel/xevent.cc index f46fdb5fb..03c947c7e 100644 --- a/src/xenia/kernel/xevent.cc +++ b/src/xenia/kernel/xevent.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -30,6 +30,7 @@ void XEvent::Initialize(bool manual_reset, bool initial_state) { } else { event_ = xe::threading::Event::CreateAutoResetEvent(initial_state); } + assert_not_null(event_); } void XEvent::InitializeNative(void* native_ptr, X_DISPATCH_HEADER* header) { @@ -53,6 +54,7 @@ void XEvent::InitializeNative(void* native_ptr, X_DISPATCH_HEADER* header) { } else { event_ = xe::threading::Event::CreateAutoResetEvent(initial_state); } + assert_not_null(event_); } int32_t XEvent::Set(uint32_t priority_increment, bool wait) { @@ -112,6 +114,7 @@ object_ref XEvent::Restore(KernelState* kernel_state, } else { evt->event_ = xe::threading::Event::CreateAutoResetEvent(false); } + assert_not_null(evt->event_); if (signaled) { evt->event_->Set(); diff --git a/src/xenia/kernel/xfile.cc b/src/xenia/kernel/xfile.cc index b9c134c01..f46a9e137 100644 --- a/src/xenia/kernel/xfile.cc +++ b/src/xenia/kernel/xfile.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2020 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -26,10 +26,12 @@ XFile::XFile(KernelState* kernel_state, vfs::File* file, bool synchronous) file_(file), is_synchronous_(synchronous) { async_event_ = threading::Event::CreateAutoResetEvent(false); + assert_not_null(async_event_); } XFile::XFile() : XObject(kObjectType) { async_event_ = threading::Event::CreateAutoResetEvent(false); + assert_not_null(async_event_); } XFile::~XFile() { diff --git a/src/xenia/kernel/xiocompletion.cc b/src/xenia/kernel/xiocompletion.cc index 638a0a3ef..9f0411bfe 100644 --- a/src/xenia/kernel/xiocompletion.cc +++ b/src/xenia/kernel/xiocompletion.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2015 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -15,6 +15,7 @@ namespace kernel { XIOCompletion::XIOCompletion(KernelState* kernel_state) : XObject(kernel_state, kObjectType) { notification_semaphore_ = threading::Semaphore::Create(0, kMaxNotifications); + assert_not_null(notification_semaphore_); } XIOCompletion::~XIOCompletion() = default; diff --git a/src/xenia/kernel/xmutant.cc b/src/xenia/kernel/xmutant.cc index 4328fc342..0a67f2183 100644 --- a/src/xenia/kernel/xmutant.cc +++ b/src/xenia/kernel/xmutant.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -28,6 +28,7 @@ void XMutant::Initialize(bool initial_owner) { assert_false(mutant_); mutant_ = xe::threading::Mutant::Create(initial_owner); + assert_not_null(mutant_); } void XMutant::InitializeNative(void* native_ptr, X_DISPATCH_HEADER* header) { diff --git a/src/xenia/kernel/xnotifylistener.cc b/src/xenia/kernel/xnotifylistener.cc index 7c054db70..fc2d24c98 100644 --- a/src/xenia/kernel/xnotifylistener.cc +++ b/src/xenia/kernel/xnotifylistener.cc @@ -2,13 +2,14 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ #include "xenia/kernel/xnotifylistener.h" +#include "xenia/base/assert.h" #include "xenia/base/byte_stream.h" #include "xenia/base/logging.h" #include "xenia/kernel/kernel_state.h" @@ -25,6 +26,7 @@ void XNotifyListener::Initialize(uint64_t mask, uint32_t max_version) { assert_false(wait_handle_); wait_handle_ = xe::threading::Event::CreateManualResetEvent(false); + assert_not_null(wait_handle_); mask_ = mask; max_version_ = max_version; diff --git a/src/xenia/kernel/xobject.cc b/src/xenia/kernel/xobject.cc index 4909dbaa1..bf189c2af 100644 --- a/src/xenia/kernel/xobject.cc +++ b/src/xenia/kernel/xobject.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -172,7 +172,7 @@ void XObject::SetAttributes(uint32_t obj_attributes_ptr) { memory(), xe::load_and_swap( memory()->TranslateVirtual(obj_attributes_ptr + 4))); if (!name.empty()) { - name_ = std::move(name); + name_ = std::string(name); kernel_state_->object_table()->AddNameMapping(name_, handles_[0]); } } @@ -410,7 +410,9 @@ object_ref XObject::GetNativeObject(KernelState* kernel_state, case 5: // SemaphoreObject { auto sem = new XSemaphore(kernel_state); - sem->InitializeNative(native_ptr, header); + auto success = sem->InitializeNative(native_ptr, header); + // Can't report failure to the guest at late initialization: + assert_true(success); object = sem; } break; case 3: // ProcessObject diff --git a/src/xenia/kernel/xsemaphore.cc b/src/xenia/kernel/xsemaphore.cc index a582fe6d6..34b960b25 100644 --- a/src/xenia/kernel/xsemaphore.cc +++ b/src/xenia/kernel/xsemaphore.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -20,22 +20,24 @@ XSemaphore::XSemaphore(KernelState* kernel_state) XSemaphore::~XSemaphore() = default; -void XSemaphore::Initialize(int32_t initial_count, int32_t maximum_count) { +bool XSemaphore::Initialize(int32_t initial_count, int32_t maximum_count) { assert_false(semaphore_); CreateNative(sizeof(X_KSEMAPHORE)); maximum_count_ = maximum_count; semaphore_ = xe::threading::Semaphore::Create(initial_count, maximum_count); + return !!semaphore_; } -void XSemaphore::InitializeNative(void* native_ptr, X_DISPATCH_HEADER* header) { +bool XSemaphore::InitializeNative(void* native_ptr, X_DISPATCH_HEADER* header) { assert_false(semaphore_); auto semaphore = reinterpret_cast(native_ptr); maximum_count_ = semaphore->limit; semaphore_ = xe::threading::Semaphore::Create(semaphore->header.signal_state, semaphore->limit); + return !!semaphore_; } int32_t XSemaphore::ReleaseSemaphore(int32_t release_count) { @@ -85,6 +87,7 @@ object_ref XSemaphore::Restore(KernelState* kernel_state, sem->semaphore_ = threading::Semaphore::Create(free_count, sem->maximum_count_); + assert_not_null(sem->semaphore_); return object_ref(sem); } diff --git a/src/xenia/kernel/xsemaphore.h b/src/xenia/kernel/xsemaphore.h index b9e78a0b1..40261398b 100644 --- a/src/xenia/kernel/xsemaphore.h +++ b/src/xenia/kernel/xsemaphore.h @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -30,8 +30,9 @@ class XSemaphore : public XObject { explicit XSemaphore(KernelState* kernel_state); ~XSemaphore() override; - void Initialize(int32_t initial_count, int32_t maximum_count); - void InitializeNative(void* native_ptr, X_DISPATCH_HEADER* header); + [[nodiscard]] bool Initialize(int32_t initial_count, int32_t maximum_count); + [[nodiscard]] bool InitializeNative(void* native_ptr, + X_DISPATCH_HEADER* header); int32_t ReleaseSemaphore(int32_t release_count); diff --git a/src/xenia/kernel/xsymboliclink.cc b/src/xenia/kernel/xsymboliclink.cc index 0d0a2ad43..e282a8879 100644 --- a/src/xenia/kernel/xsymboliclink.cc +++ b/src/xenia/kernel/xsymboliclink.cc @@ -24,8 +24,8 @@ XSymbolicLink::~XSymbolicLink() {} void XSymbolicLink::Initialize(const std::string_view path, const std::string_view target) { - path_ = path; - target_ = target; + path_ = std::string(path); + target_ = std::string(target); // TODO(gibbed): kernel_state_->RegisterSymbolicLink(this); } diff --git a/src/xenia/kernel/xthread.cc b/src/xenia/kernel/xthread.cc index 9c269c6b7..b842c2c08 100644 --- a/src/xenia/kernel/xthread.cc +++ b/src/xenia/kernel/xthread.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2020 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -1057,6 +1057,7 @@ object_ref XThread::Restore(KernelState* kernel_state, // Release the self-reference to the thread. thread->ReleaseHandle(); }); + assert_not_null(thread->thread_); // Notify processor we were recreated. thread->emulator()->processor()->OnThreadCreated( diff --git a/src/xenia/kernel/xtimer.cc b/src/xenia/kernel/xtimer.cc index fe66a73b7..e98add6d3 100644 --- a/src/xenia/kernel/xtimer.cc +++ b/src/xenia/kernel/xtimer.cc @@ -2,7 +2,7 @@ ****************************************************************************** * Xenia : Xbox 360 Emulator Research Project * ****************************************************************************** - * Copyright 2013 Ben Vanik. All rights reserved. * + * Copyright 2022 Ben Vanik. All rights reserved. * * Released under the BSD license - see LICENSE in the root for more details. * ****************************************************************************** */ @@ -35,6 +35,7 @@ void XTimer::Initialize(uint32_t timer_type) { assert_always(); break; } + assert_not_null(timer_); } X_STATUS XTimer::SetTimer(int64_t due_time, uint32_t period_ms, diff --git a/src/xenia/ui/window.h b/src/xenia/ui/window.h index 564eaeaea..f3f3bb348 100644 --- a/src/xenia/ui/window.h +++ b/src/xenia/ui/window.h @@ -280,7 +280,7 @@ class Window { // Desired state stored by the common Window, modifiable both externally and // by the implementation (including from SetFullscreen itself). - virtual bool IsFullscreen() const { return fullscreen_; } + bool IsFullscreen() const { return fullscreen_; } void SetFullscreen(bool new_fullscreen); // Desired state stored by the common Window, externally modifiable, read-only