swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

P5f fr: retire all apply allocator scope exemptions

Browse Source
This commit is contained in:
swung0x48
2026-09-19 23:41:09 -04:00
parent eeaf14c1f7
commit 4066f84d11
3 changed files with 37 additions and 214 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/Pipe/SlotAllocator.cpp
+4 -90
View File
@@ -11,9 +11,6 @@
#if MOBILEGL_BUILD_DISAGGREGATED
#include <Config.h>
// P5e (id): the guard's exemption is keyed on whether the record being applied is BARRIERED
// (CONTRACT-P5E §4.4), and MGPipeApplierCurrentRecordIsBarriered() is where ApplyOne stamps
// that. Declarations only - this file names no applier state.
#include <MG_Pipe/PipeApply.h>
#include <MG_Remote/Server/ServerLoop.h>
#include <MG_Util/Debug/Log.h>
@@ -32,28 +29,15 @@ namespace MobileGL::MG_Pipe {
void MGPipeRefuseAllocatorFromApplyThread(const char* entry) {
if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return;
if (!MG_Remote::Server::ServerLoop::OnApplyThread()) return;
// P5f: frontend-keyed registry probes are no longer an exemption. The
// separate Magma allocator debt remains backend- and barrier-keyed until
// its remaining monolith glue is removed; it cannot exempt twin-table APIs.
if (MGPipeApplierCurrentRecordIsBarriered()) {
// Ruling 12: Magma's four P7 debts, and ONLY on a DirectVulkan server. The key is
// on the backend kind rather than on the site because the scope is a class anyone
// can construct, and an Espryt probe borrowing Magma's exemption is exactly the
// hole P5e is closing.
if (MagmaP7AllocatorDebtScope::ActiveOnApplyThread() &&
MG_Config::ActiveBackendType == BackendType::DirectVulkan) {
return;
}
}
// P5f (fr): all named allocator exemptions are retired. A barrier can
// stabilize a client allocator but cannot make it server-owned memory.
MGLOG_F("MGPipe: Fatal{RoleViolation, \"MGPipeSlots\"} - the apply thread called "
"MGPipeSlots().%s. With an active transport the client slot allocator is "
"client-only memory (CONTRACT-P5C §3.1, rule E; CONTRACT-P5E §4.4): a handle "
"arrives already minted in a record, and a server that resolves or mints one "
"off a frontend object's lifetime id is reading memory that will not exist on "
"its side of a real split. Only Magma's allocator debt scope admits it while the "
"record being applied is BARRIERED (barriered=%d) and, for Magma's P7 debt, "
"only on a DirectVulkan server",
"its side of a real split. No scope or backend exempts a barriered record "
"(barriered=%d)",
entry, MGPipeApplierCurrentRecordIsBarriered() ? 1 : 0);
std::abort();
}
@@ -71,76 +55,6 @@ namespace MobileGL::MG_Pipe {
std::abort();
}
namespace {
// NOT thread_local any more, and not atomic either (P5d round 3, package D).
//
// WHY THERE IS NO DATA RACE. Both depths have exactly ONE reader in the whole tree:
// MGPipeRefuseAllocatorFromApplyThread above, whose first two lines return unless the
// transport is active AND ServerLoop::OnApplyThread() is true. So the only thread whose
// depth could ever change an answer is the apply thread - and the four scope bodies
// below now increment and decrement ONLY when OnApplyThread() says so. That makes the
// apply thread the single writer and the single reader of both counters; no other
// thread touches them, and a monolith process never even reads them (the guard's first
// line returns on TransportMode::Monolith and there is no apply thread to make the
// second true). Single-writer-single-reader on one thread needs no lock, no atomic and
// no TLS.
//
// WHY IT WAS WORTH DOING. A shared-library thread_local costs an __emutls_get_address
// call per access. On the MONOLITH's GL thread that symbol is the TOP entry of the
// profile at 7.7%, and these two scopes' ctor/dtor are 32.7% of its samples
// (MGPipeFrontendKeyedRegistryScope 18.75 ctor + 13.95 dtor, the scope P5e renamed
// MagmaP7AllocatorDebtScope 8.4); on the split's apply thread emutls is 7.4%. The
// measurement predates the rename and the numbers are quoted as measured. The scope is
// constructed at ~20
// sites in DirectGLES.cpp, several inside StateBackendObjectRegistry::HandleOf, which
// is per-draw. The cost on the GL thread is now one inlined predicate per end.
//
// The query's meaning ON THE APPLY THREAD is unchanged, which is the only meaning
// the guard reads. Off it the answer is now always false, and the query is named
// ActiveOnApplyThread() rather than Active() so that a reader who needs the other
// meaning cannot ask for it by accident: whoever wants "is a scope open on THIS
// thread" has to add it, and move the counting back, rather than read a false that
// looks like an answer.
Uint32 g_magmaP7AllocatorDebtScopeDepth = 0;
}
MagmaP7AllocatorDebtScope::MagmaP7AllocatorDebtScope()
// Decided ONCE and remembered: the destructor must undo exactly what the constructor
// did, and re-asking the predicate would leak a count across a scope that straddled
// the apply thread's exit block.
: m_counted(MG_Remote::Server::ServerLoop::OnApplyThread()) {
if (m_counted) ++g_magmaP7AllocatorDebtScopeDepth;
}
MagmaP7AllocatorDebtScope::~MagmaP7AllocatorDebtScope() {
if (m_counted) --g_magmaP7AllocatorDebtScopeDepth;
}
// The DEPTH is not backend-keyed and deliberately so: the backend kind is read by the
// GUARD, once, at the moment it decides. Counting only on DirectVulkan would make the
// depth's meaning depend on a global that a test can move between the constructor and the
// destructor, which is the m_counted bug one level out.
Bool MagmaP7AllocatorDebtScope::ActiveOnApplyThread() {
return g_magmaP7AllocatorDebtScopeDepth != 0;
}
namespace {
// Same counter, same argument, same single reader - see the block above.
Uint32 g_frontendKeyedRegistryScopeDepth = 0;
}
MGPipeFrontendKeyedRegistryScope::MGPipeFrontendKeyedRegistryScope()
: m_counted(MG_Remote::Server::ServerLoop::OnApplyThread()) {
if (m_counted) ++g_frontendKeyedRegistryScopeDepth;
}
MGPipeFrontendKeyedRegistryScope::~MGPipeFrontendKeyedRegistryScope() {
if (m_counted) --g_frontendKeyedRegistryScopeDepth;
}
Bool MGPipeFrontendKeyedRegistryScope::ActiveOnApplyThread() {
return g_frontendKeyedRegistryScopeDepth != 0;
}
#endif
namespace {
MobileGL/MG_Impl/Pipe/SlotAllocator.h
+10 -66
View File
@@ -173,17 +173,8 @@ namespace MobileGL::MG_Pipe {
// pull build's bytes do not move (G1).
void MGPipeRefuseAllocatorFromApplyThread(const char* entry);
// ---- P5e (id, CONTRACT-P5E §4.4): THE CONDITION UNDER WHICH A NAMED SCOPE EXEMPTS -----
//
// "This thread is applying a record the client is NOT parked behind." Both scopes below
// exempt a probe only while it is FALSE, because the client's wait is the whole of what
// makes a frontend read from the apply thread safe: behind a barriered record the client
// is blocked in WaitForApplied and its memory is stable; behind an unbarriered one it is
// running ahead and the same read is torn or stale BY CONSTRUCTION, which is why rule F
// has no "count it" arm and this answer does not consult MOBILEGL_IPC_STRICT_ERRORS.
//
// False in a monolith process and on any thread but the apply thread, so a GL-thread
// caller is never refused by it.
// Shared predicate for remaining field/verb guards. Registry and allocator
// guards below refuse both barriered and unbarriered transport apply.
Bool MGPipeApplierIsUnbarrieredApply();
// P5f (fr): frontend-object twin lookup, minting and weak-state access are refused
@@ -192,71 +183,24 @@ namespace MobileGL::MG_Pipe {
// glue. This separate guard also prevents borrowing Magma's allocator exemption.
void MGPipeRefuseFrontendKeyedRegistryFromApplyThread(const char* entry);
// The NAMED EXEMPTION to the rule above (CONTRACT-P5C §3.1, as amended by CONTRACT-P5E
// §4.4): the family of sites whose handle-carrying records the client does not EMIT yet.
// set_shader_buffers and set_stream_output_targets exist in the catalogue but are P4b/sb's
// to emit (SetHashSuppressor.h says so), so the buffer binding-point ensures and the
// GPU-written announcement they feed have no record handle to resolve from today. Inside
// this scope ONE read-only lifetime-id probe stays legal WHILE THE CURRENT RECORD IS
// BARRIERED; the scope is the debt's measurable, greppable form, and it retires with P7's
// server-side binding table. Every other apply-thread allocator access stays Fatal.
//
// THE DEPTH IS COUNTED ONLY ON THE APPLY THREAD (P5d round 3, package D). The counter's
// only reader is MGPipeRefuseAllocatorFromApplyThread, which returns before it looks unless
// ServerLoop::OnApplyThread() is true, so a depth kept on any OTHER thread could never
// change an answer - it was pure cost. The query is called ActiveOnApplyThread() and not
// Active() BECAUSE OF THAT (review round 3): a name that promised "is a scope open" would
// now be quietly answering "is a scope open ON THE APPLY THREAD", and the next reader to
// come along - a P4b/P7 probe deciding on the GL thread whether to emit a record - would
// read a truthful-looking false and take the wrong branch with nothing to warn it. The name
// carries the precondition so a second reader has to notice it. m_counted remembers what the constructor decided so
// the destructor undoes exactly what the constructor did; asking the predicate twice would
// leak a count for a scope that outlived the apply thread. On the GL thread (and in every
// monolith process) both ends are now one inlined predicate and a branch instead of an
// emutls call, which is where 32.7% of the monolith GL thread's __emutls_get_address - its
// top symbol at 7.7% - was going.
//
// P5e (id), ruling 12: RENAMED FROM MGPipeReverseAnnouncementScope AND KEYED ON THE
// BACKEND KIND. The rename is the finding: what is left inside it after P5e is not "the
// reverse announcement" - Espryt's half of that moved to the frontend-keyed scope with the
// rest of the registry debt - it is MAGMA'S FOUR APPLY-THREAD ALLOCATOR TOUCHES, which P7
// retires (VulkanRenderer.cpp's two hidden-resource shutdowns and its named-blit endpoint
// resolve, plus MGPipeAnnounceBufferGpuWritten in ResourceTracker.h). Naming the scope
// after the debt rather than after one of its sites is what makes "has P7 landed yet" a
// grep; naming it after Magma is what stops a new Espryt site being wrapped in it.
//
// THE EXEMPTION IS REFUSED WHEN THE SERVER BACKEND IS NOT DirectVulkan, and that is the
// point of the key rather than a safety belt. Magma stays lockstep for the whole of P5e
// (§6.1: the DirectVulkan arm never publishes kCapRunAheadApply, so every record there is
// barriered and every probe inside this scope keeps P5C's semantics). Espryt is what P5e
// is retiring the lockstep for, so an Espryt probe must not be able to borrow Magma's
// exemption - even by wrapping itself in Magma's scope.
// P5f (fr): these historical scope names remain greppable markers around
// monolith glue until P3b/P4b remove those bodies. They hold no state and grant
// no exemption: allocator and frontend-registry access always refuse apply.
class MagmaP7AllocatorDebtScope {
public:
MagmaP7AllocatorDebtScope();
~MagmaP7AllocatorDebtScope();
MagmaP7AllocatorDebtScope() = default;
~MagmaP7AllocatorDebtScope() = default;
MagmaP7AllocatorDebtScope(const MagmaP7AllocatorDebtScope&) = delete;
MagmaP7AllocatorDebtScope& operator=(const MagmaP7AllocatorDebtScope&) = delete;
static Bool ActiveOnApplyThread();
private:
Bool m_counted;
};
// P5f (fr): retained as a greppable marker around monolith-only legacy registry
// calls. This scope no longer exempts transport apply from either registry or
// allocator guards, even behind a barrier. P3b/P4b can remove these markers with
// the surrounding monolith glue; adding one cannot make a new wire fallback legal.
class MGPipeFrontendKeyedRegistryScope {
public:
MGPipeFrontendKeyedRegistryScope();
~MGPipeFrontendKeyedRegistryScope();
MGPipeFrontendKeyedRegistryScope() = default;
~MGPipeFrontendKeyedRegistryScope() = default;
MGPipeFrontendKeyedRegistryScope(const MGPipeFrontendKeyedRegistryScope&) = delete;
MGPipeFrontendKeyedRegistryScope& operator=(const MGPipeFrontendKeyedRegistryScope&) = delete;
static Bool ActiveOnApplyThread();
private:
Bool m_counted;
};
#endif
} // namespace MobileGL::MG_Pipe
MobileGL/MG_Test/Wire/RemoteClientTest.cpp
+23 -58
View File
@@ -1898,29 +1898,9 @@ MGL_BUFFER_GUARD_TEST(BufferSyncPersistentMappedRangeFromTheApplyThreadIsFatalBy
MGL_BUFFER_GUARD_TEST(BufferHasDefinedContentFromTheApplyThreadIsFatalByName, 707, (void)buffer.HasDefinedContent())
#undef MGL_BUFFER_GUARD_TEST
// P5d round 3 (package D): the two NAMED EXEMPTION scopes of CONTRACT-P5C §3.1 / §5.4 stopped
// keeping their depth in a thread_local and now count ONLY while ServerLoop::OnApplyThread() is
// true (SlotAllocator.cpp). The claim that makes that legal is "the apply-thread guard is the
// counter's only reader, so a depth kept on any other thread cannot change an answer" - and
// these three cases are that claim in executable form. They are the red-once for the change:
//
// (a) drop the `if (m_counted)` from the constructor (or make it unconditional again on a
// thread that is not the apply thread) and (c) goes red - a GL-thread scope would start
// exempting the apply thread, which is exactly the leak the thread_local used to prevent;
// (b) drop the increment altogether and (a) goes red - the exemption stops exempting and
// the debt's own sites (DirectGLES' HandleOf probes) abort the server;
// (c) drop the guard's `MagmaP7AllocatorDebtScope::ActiveOnApplyThread()` /
// `FrontendKeyedRegistryScope::ActiveOnApplyThread()` rows and (b) - the control - stops
// being the only aborting arm.
//
// P5e (id), CONTRACT-P5E §4.4 REPLACED (b). The case that used to sit there -
// `AnAllocatorProbeInsideAnExemptionScopeOnTheApplyThreadIsAllowed` - asserted that a scope
// exempts UNCONDITIONALLY, which is exactly the rule §4.4 retires: an exemption is a debt the
// CLIENT'S WAIT pays for, so it holds only while the record being applied is barriered. Its
// replacement below drives the same scope with the barriered flag forced false and expects the
// abort. The green half of the old case did not need a test of its own: every Espryt
// `integration-split` entry runs the frontend-keyed scope's surviving sites on the apply thread
// behind barriered records, so a guard that stopped exempting would take the whole lane down.
// Historical P5d/P5e probes remain named for compatibility. P5f retires both
// scope exemptions, including barriered records; all allocator access on apply is
// client-memory access regardless of whether that memory happens to be stable.
TEST(RemoteGuards, AnAllocatorProbeFromTheApplyThreadOutsideEveryScopeIsFatalByName) {
const auto child = RunInChild([] {
StartControlSession();
@@ -1935,24 +1915,8 @@ TEST(RemoteGuards, AnAllocatorProbeFromTheApplyThreadOutsideEveryScopeIsFatalByN
ExpectNamedAbort(child, "Fatal{RoleViolation, \"MGPipeSlots\"}");
}
// P5e (id) RED-ONCE, CONTRACT-P5E §4.4. The exemption scope is OPEN and the probe is the one
// the debt is named for - `HandleOf`, the frontend-keyed registry's own entry - and it still
// aborts, because the record being applied is unbarriered: the client is running ahead and the
// allocator's free list and lifetimeId -> slot map are moving under the read.
//
// THE HOOK IS THE APPLIER FIELD ITSELF and that is deliberate: `CurrentRecordBarriered` is what
// PipeApplier::ApplyOne stamps from MGPipeBarriered() on every record, so forcing it here drives
// the production path rather than a test-only branch beside it. Until ra lands the wait rule
// MGPipeBarriered answers true for every record, so this is the ONLY way to reach the arm - and
// the arm has to exist before ra, because ra is what makes it reachable in production.
//
// The red: put §4.4's `if (MGPipeApplierCurrentRecordIsBarriered())` back to an unconditional
// exemption in MGPipeRefuseAllocatorFromApplyThread and this case stops aborting - red by
// expectation, which is the shape a guard case fails in.
// EVERYTHING BUT THE PROBE RUNS ON THE CLIENT THREAD, which is the buffer/texture guard cases'
// shape one block down and is load-bearing here rather than tidy: a TextureObject2D's
// constructor MINTS a resource handle, and building it on the apply thread would abort at the
// mint with the same diagnostic - a case that passes for a reason other than the one it names.
// Construct on the client and run only HandleOf on apply. The false barrier stamp
// keeps the original P5e scenario; the tests below add the old barriered loophole.
TEST(RemoteGuards, AnAllocatorProbeInsideAnExemptionScopeFromTheApplyThreadIsFatalEvenAtTheOldDebtSites) {
struct Probe {
TestTextureTable* table;
@@ -1983,18 +1947,23 @@ TEST(RemoteGuards, AnAllocatorProbeInsideAnExemptionScopeFromTheApplyThreadIsFat
// P5f (fr): barriers and old named scopes no longer admit frontend identity.
// Construct the object and table on the client so each death comes from the exact
// probed member, not a constructor's resource_create or a missing record.
TEST(RemoteGuards, BarrieredFrontendRegistryScopeCannotExemptAllocator) {
const auto child = RunInChild([] {
StartControlSession();
Srv::ServerLoopInstance().RunProbeOnApplyThreadForTesting(+[](void*) -> MobileGLResult {
MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(true);
const MG_Pipe::MGPipeFrontendKeyedRegistryScope scope;
MG_Pipe::MGPipeRefuseAllocatorFromApplyThread("FindByLifetimeId");
return MOBILEGL_OK;
}, nullptr);
ClientSessionInstance().Stop();
});
ExpectNamedAbort(child, "Fatal{RoleViolation, \"MGPipeSlots\"}");
TEST(RemoteGuards, BarrieredLegacyScopesCannotExemptAllocator) {
for (const auto backend : {BackendType::DirectGLES, BackendType::DirectVulkan}) {
const auto child = RunInChild([backend] {
StartControlSession();
auto selectedBackend = backend;
Srv::ServerLoopInstance().RunProbeOnApplyThreadForTesting(+[](void* self) -> MobileGLResult {
MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(true);
MG_Config::ActiveBackendType = *static_cast<BackendType*>(self);
const MG_Pipe::MGPipeFrontendKeyedRegistryScope registryScope;
const MG_Pipe::MagmaP7AllocatorDebtScope magmaScope;
MGPipeSlots().FindByLifetimeId(MGPipeKind::Texture, 99);
return MOBILEGL_OK;
}, &selectedBackend);
ClientSessionInstance().Stop();
});
ExpectNamedAbort(child, "Fatal{RoleViolation, \"MGPipeSlots\"}");
}
}
TEST(RemoteGuards, BarrieredFrontendRegistryMembersRefuseBothLegacyScopes) {
@@ -2062,11 +2031,7 @@ TEST(RemoteGuards, HandleRegistryMembersWorkOnBarrieredAndUnbarrieredApply) {
EXPECT_EQ(WEXITSTATUS(child.Status), 0) << child.Log;
}
// THE ONE THAT PINS THE SINGLE-WRITER ARGUMENT. The scope is open on the GL thread for the
// whole of the posted request; the apply thread's probe must still abort, because the exemption
// belongs to the thread that entered the scope and never to another one. With the depth kept in
// a plain counter that every thread incremented, this case would go green - and a real
// apply-thread violation would be silently exempted for as long as any GL thread held a scope.
// A marker held by the GL thread never affects the apply guard either.
TEST(RemoteGuards, AnExemptionScopeHeldOnTheGLThreadDoesNotExemptTheApplyThread) {
const auto child = RunInChild([] {
StartControlSession();