diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 39af5460..48cca6f1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2958,13 +2958,16 @@ namespace MobileGL::MG_Backend::DirectGLES { #if MOBILEGL_BUILD_DISAGGREGATED // m-5 / codex 5: OnBackendContextDestroyed ran MGL_SERVER_STAGED_DROP_ALL(), which // frees every server shadow but does NOT null the hostBytes that name them - so a - // twin that SURVIVES a context loss (this is the block that repairs it) still - // carries a base into the freed allocation. The two other drop sites pair the drop - // with something that makes the base unreachable (Ops_H_Destroy retires the twin; - // the map-persistent site nulls hostBytes on the next line); DropAll did neither. - // Null it here, at the one place a surviving twin is re-armed, so no freed base - // reaches glBufferData/glBufferSubData before the next content record refills it. - resource->hostBytes = nullptr; + // twin that SURVIVES a context loss still carries a base into the freed allocation. + // Null it here, where a surviving twin is re-armed - but ONLY when the shadow is + // really gone. This block also runs on a twin's FIRST ensure (contextGeneration + // starts mismatched), and there the shadow a preceding resource_subdata just staged + // is still live; nulling it then would drop the reduced path's own bytes (it did, + // and TriangleScenario read the wrong VBO). HasShadow is the discriminator: false + // after DropAll, true after an ordinary Adopt. + if (!ServerStaged().HasShadow(resource)) { + resource->hostBytes = nullptr; + } #endif } @@ -3159,12 +3162,11 @@ namespace MobileGL::MG_Backend::DirectGLES { resource->persistentMapped = false; resource->persistentPtr = nullptr; resource->immutableStorage = false; -#if MOBILEGL_PIPE_PUSH - // m-5 / codex 5: mirror the handle arm - DropAll freed the shadow this base named - // on context loss, so a surviving twin repaired here must not carry it forward. - resource->hostBytes = nullptr; -#endif } + // m-5: the LEGACY arm (EnsureBufferResource) is reached only under monolith/push, where + // liveHostBase reads the frontend object's MappedData rather than a server shadow, so + // there is no freed server base to null here - the split freed-base hazard lives in + // EnsureBufferResourceForHandle above, guarded by HasShadow. // An immutable store nothing maps any more: a respecification of a buffer that // had been persistently mapped, which Ops_Respecify could not retire because it diff --git a/MobileGL/MG_Remote/Server/StagedShadow.h b/MobileGL/MG_Remote/Server/StagedShadow.h index 77e63b75..0a37f6f3 100644 --- a/MobileGL/MG_Remote/Server/StagedShadow.h +++ b/MobileGL/MG_Remote/Server/StagedShadow.h @@ -133,6 +133,19 @@ namespace MobileGL::MG_Remote::Server { return m_shadows.size(); } + // Is there STILL a live shadow for this key? The m-5 discriminator: after DropAll (context + // loss) the key is erased, so a twin's cached hostBytes names freed memory and must be + // nulled; after an ordinary Adopt the key is present and its base is live. The + // generation-reset block runs on a twin's FIRST ensure too (the generation starts + // mismatched), and there the shadow a preceding subdata just staged is present - so nulling + // must key on THIS answer, not on the generation change alone, or the reduced path drops + // its own bytes. + Bool HasShadow(const void* key) const { + if (!m_any.load(std::memory_order_acquire)) return false; + const std::lock_guard lock(m_mutex); + return m_shadows.find(key) != m_shadows.end(); + } + // Adjacent ranges merge - there is no gap between them, so the union really is one run. // Ranges with a gap do NOT merge, and that is the whole mechanism: it is what makes a // missing record detectable instead of papered over. diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index dd41a82f..455afa7c 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -617,6 +617,25 @@ TEST(StagedShadowTest, CoverageIsExactAndAGapIsNotCovered) { EXPECT_TRUE(store.IsCovered(&key, 0, 48)); } +// m-5's discriminator, at unit scope: HasShadow is TRUE for a key that was Adopted and FALSE once +// DropAll has run - which is exactly what tells the generation-reset block whether a twin's +// hostBytes names a live server shadow (keep it) or a freed one (null it). A version that answered +// "always live" would let a freed base reach the driver; "always gone" would drop a base a subdata +// just staged in the same generation (that regression really happened - TriangleScenario read a +// shifted VBO). Both directions are asserted here. +TEST(StagedShadowTest, HasShadowIsTrueAfterAdoptAndFalseAfterTheShadowIsDropped) { + Server::StagedShadowStore store(/*copies=*/true); + const int key = 0; + Vector bytes(16, Uint8{0x44}); + EXPECT_FALSE(store.HasShadow(&key)) << "nothing staged yet"; + store.Adopt(&key, 16, bytes.data(), 0, 16); + EXPECT_TRUE(store.HasShadow(&key)) << "a staged key must read live, or the reset block nulls a " + "base a subdata just filled"; + store.DropAll(); + EXPECT_FALSE(store.HasShadow(&key)) << "after DropAll the base is freed and must read gone, or " + "a surviving twin hands the driver a dangling pointer"; +} + TEST(StagedShadowTest, DropForgetsOneResourceAndDropAllForgetsEveryOne) { Server::StagedShadowStore store(/*copies=*/true); const int a = 0;