diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index e1a66c9d..27412201 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -357,7 +358,11 @@ namespace MobileGL::MG_Remote::Server { entry = producer; } - ServerSession* g_active = nullptr; + std::atomic g_active{nullptr}; + // One callback table and one process segment resolver: an in-progress Accept + // reserves their owner too. CAS closes the race between distinct session objects + // without making concurrent sessions a supported mode. + std::atomic g_sessionOwner{nullptr}; } // namespace @@ -369,7 +374,7 @@ namespace MobileGL::MG_Remote::Server { return *instance; } - ServerSession* ServerSession::Active() { return g_active; } + ServerSession* ServerSession::Active() { return g_active.load(std::memory_order_acquire); } ServerSession::~ServerSession() { Close(); } @@ -407,9 +412,24 @@ namespace MobileGL::MG_Remote::Server { Bool ServerSession::Accepted() const { return m_accepted; } MobileGLResult ServerSession::Accept(Transport::ITransport& transport) { - if (m_accepted) { - return MOBILEGL_ERR_INVALID_ARGUMENT; + ServerSession* expectedOwner = nullptr; + if (!g_sessionOwner.compare_exchange_strong(expectedOwner, this, + std::memory_order_acq_rel, std::memory_order_acquire)) { + if (expectedOwner == this) return MOBILEGL_ERR_INVALID_ARGUMENT; + MGLOG_F("MGPipe: Fatal{RoleViolation, \"callback-double-install\"} - another " + "ServerSession already owns or is accepting the process reverse channel"); + std::abort(); } + struct ReleaseFailedClaim { + ServerSession* self; + ~ReleaseFailedClaim() { + if (!self->Accepted()) { + ServerSession* expected = self; + g_sessionOwner.compare_exchange_strong(expected, nullptr, + std::memory_order_release, std::memory_order_relaxed); + } + } + } releaseOnFailure{this}; m_transport = &transport; // MOBILEGL_IPC_RING_MB / _STAGE_MB unless SetSegmentSizes overrode them. Unconditional // on purpose - see SizesFromConfig. @@ -538,7 +558,6 @@ namespace MobileGL::MG_Remote::Server { m_segments.InstallProcessResolver(); m_accepted = true; - g_active = this; // ---- P5c ev: the four reverse-channel producers are THIS session's (CONTRACT-P5C // ยง4.1), installed before the apply thread can apply a record that produces one and @@ -552,6 +571,7 @@ namespace MobileGL::MG_Remote::Server { InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnGpuWritten, &ServerOnGpuWritten); InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnSurfaceChanged, &ServerOnSurfaceChanged); + g_active.store(this, std::memory_order_release); LogMemory("accept"); @@ -624,9 +644,9 @@ namespace MobileGL::MG_Remote::Server { } void ServerSession::Close() { - if (g_active == this) { - g_active = nullptr; - } + ServerSession* expectedActive = this; + g_active.compare_exchange_strong(expectedActive, nullptr, + std::memory_order_acq_rel, std::memory_order_acquire); if (m_accepted) { // Uninstall AFTER the apply thread has joined, never before: a record still in // flight can still resolve a segment offset (table 3's fourth column). @@ -653,6 +673,9 @@ namespace MobileGL::MG_Remote::Server { m_shm.Close(); m_transport = nullptr; m_accepted = false; + ServerSession* expectedOwner = this; + g_sessionOwner.compare_exchange_strong(expectedOwner, nullptr, + std::memory_order_release, std::memory_order_relaxed); } Transport::RingConsumer& ServerSession::CommandRing() { return m_commands; } diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index ef0b73ee..7c137733 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -2375,6 +2375,20 @@ TEST(P5fReverseChannel, GlErrorCallbackRejectsDoubleInstallation) { EXPECT_NE(ReadLog().find("callback-double-install"), std::string::npos); } +TEST(P5fReverseChannel, ASecondSessionCannotClaimTheSameReverseChannel) { + EXPECT_EXIT({ + FvSession owner; + if (!owner.Handshake()) std::_Exit(7); + std::unique_ptr client, server; + Transport::InProcessTransport::CreatePair(client, server); + Server::ServerSession other; + // The owner refusal precedes receiving a Hello or replacing the segment resolver. + (void)other.Accept(*server); + std::_Exit(9); + }, ::testing::KilledBySignal(SIGABRT), ""); + EXPECT_NE(ReadLog().find("another ServerSession already owns"), std::string::npos); +} + TEST(P5fReverseChannel, RecordErrorWithoutCallbackIsNamedFatal) { EXPECT_EXIT({ MG_Config::Ipc.StrictErrors = true;