From 74ba390db5b7b11758eedf2665b0f3c345f8bcfc Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 24 Sep 2026 14:03:47 +0000 Subject: [PATCH] [MG_Backend, MG_Test] (P12 review): the ended server session's texture and sampler unit shadows are emptied with its twins, so the next session's binds are not skipped against freed twins Major. DropEveryTwinForEndedServerSession drops every twin with InProcessTeardown() answering true, which is exactly the case in which a texture / sampler twin's destructor does NOT scrub itself out of TextureImpl::g_boundTexturesCache / SamplerImpl::g_boundSamplersCache. The shadows kept raw pointers to freed twins; the next in-process session's twin allocated at a recycled address read "already bound" and skipped its glBindTexture / glBindSampler on a context where nothing was bound, so its uploads went to texture 0 and it rendered black. The drop now empties both shadows and puts the active-unit shadow back to 0 (the new context's GL_TEXTURE0). SanityTest EsprytServerSession.TheServerBackendsDestructionEmptiesTheTextureAndSamplerUnitShadows (split build only; the pull build is unchanged, G1). --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 10 +++++ MobileGL/MG_Test/SanityTest.cpp | 45 +++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 4577f7a9..f298226e 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -421,6 +421,16 @@ namespace MobileGL::MG_Backend::DirectGLES { TextureImpl::g_backendTextureObjects = {}; RenderbufferImpl::g_backendRenderbufferObjects = {}; BufferImpl::g_backendBufferResources = {}; + // P12 review fix (major): AND THE UNIT SHADOWS THAT POINT AT THEM. A texture or sampler twin + // scrubs itself out of g_boundTexturesCache / g_boundSamplersCache in its destructor - but + // not under InProcessTeardown(), which answered true for every twin just dropped. Left + // behind, those raw pointers name freed twins; the next session's twin allocated at the same + // address then reads "already bound" and skips its glBindTexture / glBindSampler on a + // context where nothing is bound, so its uploads land on texture 0 and it samples black. + // The new context's active unit is GL_TEXTURE0, so the active-unit shadow goes back to 0 too. + TextureImpl::g_boundTexturesCache = {}; + TextureImpl::g_activeTextureUnit = 0; + SamplerImpl::g_boundSamplersCache = {}; g_serverSessionTwinTeardown = false; MGLOG_I("DirectGLES: the ended server session's twins are dropped (every kind, no driver call); the next " "session in this process starts from empty twin tables"); diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index c77b17ac..c210723c 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -5495,8 +5495,53 @@ TEST(EsprytServerSession, TheServerBackendsDestructionDropsEveryTwinTheSessionBu GL::BufferImpl::g_backendBufferResources = {}; MG_Config::Transport = previousTransport; } + +// P12 review fix (major): THE UNIT SHADOWS GO WITH THE TWINS THEY POINT AT. A texture / sampler twin +// scrubs itself out of g_boundTexturesCache / g_boundSamplersCache in its destructor, except under +// InProcessTeardown() - which answers true for every twin the session end drops. The shadows then +// held raw pointers to freed twins, and the next session's twin allocated at a recycled address read +// "already bound" and skipped its glBindTexture / glBindSampler on the new context: its uploads went +// to texture 0 and it rendered black. The shadow entries here stand for the ended session's binds (the +// addresses are never dereferenced), and the active-unit shadow for its last glActiveTexture. Red with +// the three resets in DropEveryTwinForEndedServerSession deleted: every expectation below fails. +TEST(EsprytServerSession, TheServerBackendsDestructionEmptiesTheTextureAndSamplerUnitShadows) { + using namespace MobileGL; + namespace GL = MG_Backend::DirectGLES; + const auto previousTransport = MG_Config::Transport; + const auto previousTextures = GL::TextureImpl::g_boundTexturesCache; + const auto previousSamplers = GL::SamplerImpl::g_boundSamplersCache; + const auto previousUnit = GL::TextureImpl::g_activeTextureUnit; + alignas(64) static unsigned char endedTexture[64]; + alignas(64) static unsigned char endedSampler[64]; + const auto texture2DSlot = static_cast(TextureTarget::Texture2D); + + MG_Config::Transport = MG_Config::TransportMode::Spawn; + GL::TextureImpl::g_boundTexturesCache[0][texture2DSlot] = + reinterpret_cast(endedTexture); + GL::TextureImpl::g_boundTexturesCache[3][texture2DSlot] = + reinterpret_cast(endedTexture); + GL::SamplerImpl::g_boundSamplersCache[3] = reinterpret_cast(endedSampler); + GL::TextureImpl::g_activeTextureUnit = 3; + { GL::BackendObject_DirectGLES serverBackend; } + EXPECT_EQ(GL::TextureImpl::g_boundTexturesCache[0][texture2DSlot], nullptr) + << "unit 0's texture shadow still names the ended session's twin: a twin recycled at that address " + "skips its glBindTexture and the next session's upload lands on texture 0"; + EXPECT_EQ(GL::TextureImpl::g_boundTexturesCache[3][texture2DSlot], nullptr); + EXPECT_EQ(GL::SamplerImpl::g_boundSamplersCache[3], nullptr) + << "unit 3's sampler shadow still names the ended session's sampler twin"; + EXPECT_EQ(GL::TextureImpl::g_activeTextureUnit, 0u) + << "the new context's active unit is GL_TEXTURE0, and a shadow saying 3 skips glActiveTexture(3)"; + + GL::TextureImpl::g_boundTexturesCache = previousTextures; + GL::SamplerImpl::g_boundSamplersCache = previousSamplers; + GL::TextureImpl::g_activeTextureUnit = previousUnit; + MG_Config::Transport = previousTransport; +} #else TEST(EsprytServerSession, TheServerBackendsDestructionDropsEveryTwinTheSessionBuilt) { GTEST_SKIP() << "a server backend exists only in the split build (MOBILEGL_BUILD_DISAGGREGATED)"; } +TEST(EsprytServerSession, TheServerBackendsDestructionEmptiesTheTextureAndSamplerUnitShadows) { + GTEST_SKIP() << "a server backend exists only in the split build (MOBILEGL_BUILD_DISAGGREGATED)"; +} #endif