From 27ce97fd42d758350c5100c4bbcb78de0a6d48b5 Mon Sep 17 00:00:00 2001 From: bunnei Date: Fri, 4 Jun 2021 19:26:48 -0700 Subject: hle: kernel: Refactor to allocate a ServiceThread per service handler. - Previously, we would allocate a thread per session, which adds new threads on CloneCurrentObject. - This results in race conditions with N sessions queuing requests to the same service interface. - Fixes Pokken Tournament DX crashes/softlocks, which were regressed by #6347. --- src/core/hle/service/ns/pl_u.cpp | 2 -- src/core/hle/service/service.cpp | 6 +++--- src/core/hle/service/service.h | 5 +++-- src/core/hle/service/sm/controller.cpp | 37 +++++++++------------------------- src/core/hle/service/sm/sm.cpp | 2 +- 5 files changed, 17 insertions(+), 35 deletions(-) (limited to 'src/core/hle/service') diff --git a/src/core/hle/service/ns/pl_u.cpp b/src/core/hle/service/ns/pl_u.cpp index 6e5ba26a3..74cc45f1e 100644 --- a/src/core/hle/service/ns/pl_u.cpp +++ b/src/core/hle/service/ns/pl_u.cpp @@ -254,8 +254,6 @@ void PL_U::GetSharedMemoryNativeHandle(Kernel::HLERequestContext& ctx) { LOG_DEBUG(Service_NS, "called"); // Create shared font memory object - auto& kernel = system.Kernel(); - std::memcpy(kernel.GetFontSharedMem().GetPointer(), impl->shared_font->data(), impl->shared_font->size()); diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index 7a15eeba0..4e1541630 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -93,8 +93,8 @@ namespace Service { ServiceFrameworkBase::ServiceFrameworkBase(Core::System& system_, const char* service_name_, u32 max_sessions_, InvokerFn* handler_invoker_) - : system{system_}, service_name{service_name_}, max_sessions{max_sessions_}, - handler_invoker{handler_invoker_} {} + : SessionRequestHandler(system_.Kernel(), service_name_), system{system_}, + service_name{service_name_}, max_sessions{max_sessions_}, handler_invoker{handler_invoker_} {} ServiceFrameworkBase::~ServiceFrameworkBase() { // Wait for other threads to release access before destroying @@ -111,7 +111,7 @@ void ServiceFrameworkBase::InstallAsService(SM::ServiceManager& service_manager) port_installed = true; } -Kernel::KClientPort& ServiceFrameworkBase::CreatePort(Kernel::KernelCore& kernel) { +Kernel::KClientPort& ServiceFrameworkBase::CreatePort() { const auto guard = LockService(); ASSERT(!port_installed); diff --git a/src/core/hle/service/service.h b/src/core/hle/service/service.h index 4c048173b..ec757753c 100644 --- a/src/core/hle/service/service.h +++ b/src/core/hle/service/service.h @@ -23,6 +23,7 @@ namespace Kernel { class HLERequestContext; class KClientPort; class KServerSession; +class ServiceThread; } // namespace Kernel namespace Service { @@ -41,7 +42,7 @@ class ServiceManager; static const int kMaxPortSize = 8; ///< Maximum size of a port name (8 characters) /// Arbitrary default number of maximum connections to an HLE service. -static const u32 DefaultMaxSessions = 10; +static const u32 DefaultMaxSessions = 64; /** * This is an non-templated base of ServiceFramework to reduce code bloat and compilation times, it @@ -74,7 +75,7 @@ public: void InvokeRequestTipc(Kernel::HLERequestContext& ctx); /// Creates a port pair and registers it on the kernel's global port registry. - Kernel::KClientPort& CreatePort(Kernel::KernelCore& kernel); + Kernel::KClientPort& CreatePort(); /// Handles a synchronization request for the service. ResultCode HandleSyncRequest(Kernel::KServerSession& session, diff --git a/src/core/hle/service/sm/controller.cpp b/src/core/hle/service/sm/controller.cpp index 5fa5e0512..8b9418e0f 100644 --- a/src/core/hle/service/sm/controller.cpp +++ b/src/core/hle/service/sm/controller.cpp @@ -28,42 +28,25 @@ void Controller::ConvertCurrentObjectToDomain(Kernel::HLERequestContext& ctx) { } void Controller::CloneCurrentObject(Kernel::HLERequestContext& ctx) { - // TODO(bunnei): This is just creating a new handle to the same Session. I assume this is wrong - // and that we probably want to actually make an entirely new Session, but we still need to - // verify this on hardware. - LOG_DEBUG(Service, "called"); - auto& kernel = system.Kernel(); - auto* session = ctx.Session()->GetParent(); - auto* port = session->GetParent()->GetParent(); + auto& parent_session = *ctx.Session()->GetParent(); + auto& parent_port = parent_session.GetParent()->GetParent()->GetClientPort(); + auto& session_manager = parent_session.GetServerSession().GetSessionRequestManager(); - // Reserve a new session from the process resource limit. - Kernel::KScopedResourceReservation session_reservation( - kernel.CurrentProcess()->GetResourceLimit(), Kernel::LimitableResource::Sessions); - if (!session_reservation.Succeeded()) { + // Create a session. + Kernel::KClientSession* session{}; + const ResultCode result = parent_port.CreateSession(std::addressof(session), session_manager); + if (result.IsError()) { + LOG_CRITICAL(Service, "CreateSession failed with error 0x{:08X}", result.raw); IPC::ResponseBuilder rb{ctx, 2}; - rb.Push(Kernel::ResultLimitReached); + rb.Push(result); } - // Create a new session. - auto* clone = Kernel::KSession::Create(kernel); - clone->Initialize(&port->GetClientPort(), session->GetName()); - - // Commit the session reservation. - session_reservation.Commit(); - - // Enqueue the session with the named port. - port->EnqueueSession(&clone->GetServerSession()); - - // Set the session request manager. - clone->GetServerSession().SetSessionRequestManager( - session->GetServerSession().GetSessionRequestManager()); - // We succeeded. IPC::ResponseBuilder rb{ctx, 2, 0, 1, IPC::ResponseBuilder::Flags::AlwaysMoveHandles}; rb.Push(ResultSuccess); - rb.PushMoveObjects(clone->GetClientSession()); + rb.PushMoveObjects(session); } void Controller::CloneCurrentObjectEx(Kernel::HLERequestContext& ctx) { diff --git a/src/core/hle/service/sm/sm.cpp b/src/core/hle/service/sm/sm.cpp index d8b20a3f2..bffa9ffcb 100644 --- a/src/core/hle/service/sm/sm.cpp +++ b/src/core/hle/service/sm/sm.cpp @@ -46,7 +46,7 @@ Kernel::KClientPort& ServiceManager::InterfaceFactory(ServiceManager& self, Core self.sm_interface = sm; self.controller_interface = std::make_unique(system); - return sm->CreatePort(system.Kernel()); + return sm->CreatePort(); } ResultVal ServiceManager::RegisterService(std::string name, -- cgit v1.2.3