From 2dd453dae0804e58c9673bf26280f7a3cea11fb6 Mon Sep 17 00:00:00 2001 From: Daan Dehing Date: Sat, 10 Oct 2026 18:07:35 +0000 Subject: [PATCH] fix(agc): drain the registration preparation pool before the device is reset Background registration preparations (#2583) copy the device's SpirvTarget, whose capability and extension spans point into the device's shader profile. Driver::stop reset the device without waiting for the detached pool, so a preparation still queued or running read freed extension names and agc_driver crashed at exit on RADV. Driver::stop now closes the pool after joining the queue workers: it stops queueing and waits until no task is queued or running. A preparation submitted after that runs on the caller's thread, which holds its own reference to the device, like the existing fallback for a pool without threads. Fixes #2891 Co-Authored-By: Claude Opus 5.5 --- .../include/Driver/Shaders/ShaderRegistry.hpp | 3 + .../Execution/src/Driver/Lifecycle.cpp | 2 + .../src/Driver/Shaders/ShaderRegistry.cpp | 41 ++++++++-- .../execution/RegistrationPrepareShutdown.cpp | 76 +++++++++++++++++++ 4 files changed, 114 insertions(+), 8 deletions(-) create mode 100644 core/libs/prx/libSceAgcDriver/tests/execution/RegistrationPrepareShutdown.cpp diff --git a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp index ac19eec56..25fd58e2d 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp +++ b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -92,6 +93,8 @@ ShaderSnapshot PrepareNullPixelProgram(const VulkanDevice& device); std::optional RegisteredFloatMode(const ShaderSnapshot& snapshot); std::shared_ptr RegisteredProgram(const ShaderRegistry& registry, std::uint64_t address, std::initializer_list types); void PublishRegisteredShader(std::shared_ptr& registry, const std::shared_ptr& snapshot); +void SubmitRegistrationPreparation(std::function task); +void CloseRegistrationPreparation(); void ResolvePreparedGraphics(const ShaderSnapshot& front, const std::shared_ptr& fragment, std::uint32_t primitiveType, const ShaderRecompiler::SpirvTarget& target); diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Lifecycle.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Lifecycle.cpp index ac5927eed..1dbe1cfd1 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Lifecycle.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Lifecycle.cpp @@ -1,5 +1,6 @@ #include "prx/libSceAgcDriver/Execution/include/Driver/Driver.hpp" #include "prx/libSceAgcDriver/Execution/include/Driver/Diagnostics.hpp" +#include "prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp" #include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp" #include "prx/libSceAgcDriver/Execution/include/WorkerSampler.hpp" #include "prx/libSceAgcDriver/Execution/include/ProfileOutput.hpp" @@ -44,6 +45,7 @@ void Driver::stop() { if (worker.thread.joinable()) worker.thread.join(); } StopWorkerSampler(); + CloseRegistrationPreparation(); std::lock_guard gpuLock(GuestMemory::GpuMutex()); device.Reset(); replacedDevices.clear(); diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp index 1ae54ed52..d01847729 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp @@ -551,15 +551,22 @@ public: } void Submit(std::function task) { - if (threads == 0) { - task(); - return; - } { - std::lock_guard lock(mutex); - tasks.push_back(std::move(task)); + std::unique_lock lock(mutex); + if (threads != 0 && !closed) { + tasks.push_back(std::move(task)); + lock.unlock(); + ready.notify_one(); + return; + } } - ready.notify_one(); + task(); + } + + void Close() { + std::unique_lock lock(mutex); + closed = true; + idle.wait(lock, [&] { return tasks.empty() && running == 0; }); } private: @@ -587,16 +594,26 @@ private: pool.ready.wait(lock, [&] { return !pool.tasks.empty(); }); task = std::move(pool.tasks.front()); pool.tasks.pop_front(); + ++pool.running; } task(); + task = nullptr; + { + std::lock_guard lock(pool.mutex); + --pool.running; + if (pool.tasks.empty() && pool.running == 0) pool.idle.notify_all(); + } } return nullptr; } std::mutex mutex; std::condition_variable ready; + std::condition_variable idle; std::deque> tasks; unsigned threads = 0; + unsigned running = 0; + bool closed = false; }; bool AsyncRegistrationPrepare() { @@ -605,7 +622,7 @@ bool AsyncRegistrationPrepare() { } void PrepareRegisteredInBackground(std::shared_ptr snapshot, std::shared_ptr plan) { - RegistrationPreparePool::Instance().Submit([snapshot = std::move(snapshot), plan = std::move(plan)] { + SubmitRegistrationPreparation([snapshot = std::move(snapshot), plan = std::move(plan)] { PerformanceContext timingContext(FrameTiming::Preparation()); std::vector entries; std::exception_ptr failure; @@ -631,6 +648,14 @@ void PrepareRegisteredInBackground(std::shared_ptr snapsho } +void SubmitRegistrationPreparation(std::function task) { + RegistrationPreparePool::Instance().Submit(std::move(task)); +} + +void CloseRegistrationPreparation() { + RegistrationPreparePool::Instance().Close(); +} + std::vector PrepareGraphicsStages(const DrawDecode& decoded, const ShaderRecompiler::SpirvTarget& target) { ShaderPreparationTransaction transaction; require(decoded.programs.size() == decoded.roles.size(), "graphics ABI program roles are incomplete"); diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/RegistrationPrepareShutdown.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/RegistrationPrepareShutdown.cpp new file mode 100644 index 000000000..94f3c4606 --- /dev/null +++ b/core/libs/prx/libSceAgcDriver/tests/execution/RegistrationPrepareShutdown.cpp @@ -0,0 +1,76 @@ +#include "prx/libSceAgcDriver/Execution/include/Driver/Driver.hpp" +#include "prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp" +#include "prx/libc/include/Shutdown.hpp" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +void Require(bool condition, const std::string& message) { + if (!condition) throw std::runtime_error("registration prepare shutdown: " + message); +} + +void CheckShutdownWaits() { + std::mutex mutex; + std::condition_variable changed; + bool started = false; + bool released = false; + std::atomic finished{false}; + std::atomic returned{false}; + std::atomic finishedAtReturn{false}; + AgcDriver::DriverDetail::Driver::Get(); + AgcDriver::DriverDetail::SubmitRegistrationPreparation([&] { + std::unique_lock lock(mutex); + started = true; + changed.notify_all(); + changed.wait(lock, [&] { return released; }); + finished.store(true); + }); + { + std::unique_lock lock(mutex); + changed.wait(lock, [&] { return started; }); + } + std::thread shutdown([&] { + LibcRunShutdown_nid_postfix(); + finishedAtReturn.store(finished.load()); + returned.store(true); + }); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + const bool returnedEarly = returned.load(); + { + std::lock_guard lock(mutex); + released = true; + } + changed.notify_all(); + shutdown.join(); + Require(!returnedEarly, "shutdown returned while a registration preparation was still running"); + Require(finishedAtReturn.load(), "shutdown returned before the running registration preparation finished"); +} + +void CheckLateSubmissionRunsInline() { + const auto caller = std::this_thread::get_id(); + std::thread::id ran; + AgcDriver::DriverDetail::SubmitRegistrationPreparation([&] { ran = std::this_thread::get_id(); }); + Require(ran == caller, "a registration preparation submitted after shutdown did not run on the caller's thread"); +} + +} + +int main() { + try { + CheckShutdownWaits(); + CheckLateSubmissionRunsInline(); + std::puts("registration prepare shutdown tests passed"); + return 0; + } catch (const std::exception& error) { + std::cerr << error.what() << '\n'; + return 1; + } +}