swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-17 00:28:31 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (MG_Remote/Server, DirectGLES/BackendObject): ID-54 bind-once at the native layer - under an active transport BackendObject_DirectGLES skips the native eglMakeCurrent for a surface already current on the apply thread (its own creation bound it; measured 2 -> 1 native binds per process, 0 native releases, owner unchanged), and N-3 the held (dpy, draw, read, ctx) tuple is FORGOTTEN on ReleaseEGLResources, ReleaseEGLSurface of a surface it names, surface (re)creation and backend destruction so a recycled handle value after a destroy is a real bind again; ServerLoop.h's ownership prose states what is measured rather than what was hoped (review v2 item 10, N-3)

Browse Source
This commit is contained in:
swung0x48
2026-09-16 10:17:29 -04:00
parent ca92308bc0
commit a838c7879d
3 changed files with 133 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp
+25
View File
@@ -972,9 +972,34 @@ namespace MobileGL::MG_Backend::DirectGLES {
return false;
}
#if MOBILEGL_BUILD_DISAGGREGATED
// ID-54 / C7, the native half of "bind once per tuple" (v1, under ID-52/ID-59's grant for
// this file; #if-guarded so the pull build is byte-identical). Under an active transport
// this runs on the apply thread, which is the ONLY thread that ever binds the server's
// context, and the surface it is asked for was made natively current on this very thread
// by its own creation (InitPbufferSurface / InitWindowSurface -> DirectGLES::MakeCurrent).
// A second native eglMakeCurrent for the same surface is then a repeat: it rewrites the
// owner with the same thread, re-registers the same op table and invalidates seven caches
// that describe a context that did not change - the per-client-make-current storm the
// v2 review measured as "2 native binds per process, unchanged". So when the requested
// draw surface IS the active one and EGL itself says this thread holds the context
// (IsBackendContextCurrentOnThisThread re-verifies against eglGetCurrentContext), the
// native call is skipped and only the base class's bookkeeping below runs - which is
// still required: it is what InitCapabilities and SwapEGLBuffers' current-thread record
// hang off. Monolith transport in this build, and the pull build, bind exactly as before.
// Red once by making this arm unconditional: ServerLoopTest's C7 control reads 2 native
// binds at the EGL function table instead of 1.
const Bool nativelyCurrentAlready = MG_Config::Transport != MG_Config::TransportMode::Monolith &&
m_eglSurfaceInitialized && m_eglSurface == draw &&
DirectGLES::IsBackendContextCurrentOnThisThread();
if (!nativelyCurrentAlready && !DirectGLES::MakeCurrent()) {
return false;
}
#else
if (!DirectGLES::MakeCurrent()) {
return false;
}
#endif
if (!BackendObject::MakeEGLCurrent(dpy, draw, read, ctx)) {
(void)DirectGLES::ReleaseCurrent();
MobileGL/MG_Remote/Server/ServerLoop.cpp
+55 -5
View File
@@ -276,12 +276,22 @@ namespace MobileGL::MG_Remote::Server {
outcome.boundNatively = false;
return outcome;
case EglBindAction::RepeatNoOp:
// ID-54's "a no-op apart from the R-12 republish decision": the decision for an
// identical repeat is NO republish, because nothing ran that could have moved the
// caps - InitCapabilities runs inside the backend's MakeEGLCurrent, which this arm
// does not reach, so a republish here would re-send the snapshot the last real bind
// already sent. (c1's BackendObject_Remote::InitCapabilities does not lean on this
// either way: it asks the server through ServerInitCapabilities, which publishes.)
outcome.ok = true;
outcome.boundNatively = false;
return outcome;
case EglBindAction::NativeBind:
break;
}
// "Forwarded" is the honest word: the backend object decides for itself whether the
// driver needs a native eglMakeCurrent (BackendObject_DirectGLES skips it for a surface
// that is already current on this thread - its own creation bound it), and the C7
// control counts THAT at the EGL function table. This counter counts forwards.
outcome.ok = backend->MakeEGLCurrent(dpy, draw, read, ctx);
if (!outcome.ok) return outcome;
m_haveCurrentTuple = true;
@@ -294,6 +304,19 @@ namespace MobileGL::MG_Remote::Server {
return outcome;
}
void ServerLoop::ForgetCurrentTuple() {
m_haveCurrentTuple = false;
m_curDpy = EGL_NO_DISPLAY;
m_curDraw = EGL_NO_SURFACE;
m_curRead = EGL_NO_SURFACE;
m_curCtx = EGL_NO_CONTEXT;
}
void ServerLoop::ForgetCurrentTupleIfItNames(EGLSurface surface) {
if (!m_haveCurrentTuple) return;
if (m_curDraw == surface || m_curRead == surface) ForgetCurrentTuple();
}
void ServerLoop::ApplyThreadMain() {
m_applyThreadId.store(std::this_thread::get_id(), std::memory_order_release);
NameThisThread("mgl-srv-apply");
@@ -389,6 +412,9 @@ namespace MobileGL::MG_Remote::Server {
"which is the context owner");
m_backend.reset();
}
// N-3: the context died with the backend; a tuple that outlives it would make the next
// session's first make-current onto the same (recycled) handle values a RepeatNoOp.
ForgetCurrentTuple();
// C2: THE m_running CLEAR IS INSIDE THIS SAME CRITICAL SECTION AS THE FINAL DRAIN OF THE
// MAILBOX. It used to be a separate store after the block, and that gap was a lost-forever
@@ -578,6 +604,9 @@ namespace MobileGL::MG_Remote::Server {
// thread called Stop. No context was ever made current from another thread in that
// case, which is exactly the condition that makes this safe.
if (m_backend != nullptr) m_backend.reset();
// N-3, same reason as ApplyThreadMain's exit: no thread runs, so the apply-thread-only
// rule on the tuple has no other writer to race.
ForgetCurrentTuple();
m_running.store(false, std::memory_order_release);
return;
}
@@ -674,6 +703,9 @@ namespace MobileGL::MG_Remote::Server {
MG_Backend::BackendObject* backend = ServerBackendOrNull();
if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
ok = backend->CreateEGLWindowSurface(surface, *handle);
// N-3: BackendObject_DirectGLES destroys and recreates the native context to
// create a DIFFERENT surface, so whatever tuple was bound names a dead context.
if (ok) ServerLoopInstance().ForgetCurrentTuple();
return MOBILEGL_OK;
}
} args{surface, &handle};
@@ -706,6 +738,10 @@ namespace MobileGL::MG_Remote::Server {
MG_Backend::BackendObject* backend = ServerBackendOrNull();
if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
ok = backend->CreateEGLPbufferSurface(surface, width, height);
// N-3: as for the window surface - a (re)creation may have destroyed the context
// the held tuple named. The surface's own creation binds natively, so the client's
// make-current that follows is forwarded and deduped one layer down (ID-54).
if (ok) ServerLoopInstance().ForgetCurrentTuple();
return MOBILEGL_OK;
}
} args{surface, width, height};
@@ -722,11 +758,13 @@ namespace MobileGL::MG_Remote::Server {
MobileGLResult Run() {
MG_Backend::BackendObject* backend = ServerBackendOrNull();
if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
// C7 / ID-54: the apply thread binds the native context ONCE per tuple and holds
// it for life. ApplyMakeCurrent forwards a real bind only for a new tuple, treats
// an identical repeat as a no-op, and records a client release-current WITHOUT
// unbinding - so "the owner slot is written once" is true here even though
// DirectGLES::MakeCurrent itself has no shortcut.
// C7 / ID-54: the apply thread binds the native context ONCE per context
// lifetime and holds it for life. ApplyMakeCurrent forwards a bind only for a
// tuple it does not hold, treats an identical repeat as a no-op, and records a
// client release-current WITHOUT unbinding; the native call for a surface already
// current on this thread is skipped one layer down (BackendObject_DirectGLES's
// ID-54 arm), which is what makes "the owner slot is written once" TRUE and
// measured (ServerLoopTest's C7 control) rather than claimed.
const ServerLoop::MakeCurrentOutcome outcome =
ServerLoopInstance().ApplyMakeCurrent(backend, dpy, draw, read, ctx);
ok = outcome.ok;
@@ -789,6 +827,12 @@ namespace MobileGL::MG_Remote::Server {
MG_Backend::BackendObject* backend = ServerBackendOrNull();
if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
backend->ReleaseEGLSurface(surface);
// N-3: a released surface the held tuple names may have taken the context with
// it (BackendObject::ReleaseEGLSurface -> OnEGLSurfaceReleased -> DestroyEGLContext
// once nothing holds it current). Forgetting when the base class only DEFERRED
// the destroy costs one forwarded bind; remembering when it did not would cost a
// silent no-context-current.
ServerLoopInstance().ForgetCurrentTupleIfItNames(surface);
return MOBILEGL_OK;
}
} args{surface};
@@ -807,6 +851,12 @@ namespace MobileGL::MG_Remote::Server {
MG_Backend::BackendObject* backend = ServerBackendOrNull();
if (backend == nullptr) return MOBILEGL_ERR_NOT_INITIALIZED;
backend->ReleaseEGLResources();
// N-3: DestroyEGLContext just ran; the tuple names nothing. Without this a
// destroy-recreate with the same handle values (every EGL handle on this host
// is 0x1) classified as a RepeatNoOp, bound nothing, and republished no caps.
// Red once by deleting it: ServerLoopTest's recreate control reads
// NativeBindCount() == 1 where 2 is required.
ServerLoopInstance().ForgetCurrentTuple();
return MOBILEGL_OK;
}
} args{};
MobileGL/MG_Remote/Server/ServerLoop.h
+53 -20
View File
@@ -32,17 +32,30 @@
// before the client frees any emitter-owned Vector; and the join must be bounded (that test uses
// 5 s) so a regression is a red test and not a hung CI job.
//
// THE EGL OWNERSHIP MOVE. eglMakeCurrent runs ONCE per (dpy, draw, read, ctx) tuple on this
// thread and the context is then held for life. The "once" is NOT free at the DirectGLES layer -
// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7) -
// so ServerMakeEGLCurrent is where the dedup lives (ID-54): an identical repeat is a no-op apart
// from the R-12 republish decision, a different tuple is a real rebind, and a client
// release-current is RECORDED (NativeBindCount / ClientReleaseCount) but NOT forwarded - the apply
// thread keeps the context current until ~BackendObject_DirectGLES or context loss, which is what
// makes DirectGLES.cpp's six cache invalidations a one-off startup cost and the 16
// IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow() sites answer TRUE on the server. The
// client's nine EGL virtuals become BLOCKING control requests executed here. ReleaseEGLResources
// and ~BackendObject_DirectGLES MUST be blocking: MobileGL::Destroy() (MobileGL/Init.cpp:68)
// THE EGL OWNERSHIP MOVE, AS MEASURED (ID-54; review v2 item 10 and N-3). The native
// eglMakeCurrent for a surface runs ONCE PER CONTEXT LIFETIME on this thread and the context is
// then held for life. That "once" lives in TWO layers, because one is not enough:
// DirectGLES::MakeCurrent always calls native eglMakeCurrent and rewrites the owner (codex C7),
// and it is reached twice per bring-up - once from InitPbufferSurface/InitWindowSurface when the
// surface is CREATED, and once more from the client's first eglMakeCurrent. So (1)
// ServerMakeEGLCurrent classifies the request against the tuple it last bound
// (ClassifyEglMakeCurrent): an identical repeat is a no-op, and the R-12 republish decision for
// it is "nothing to republish"; a different tuple is a real forwarded bind; a client
// release-current is RECORDED (ClientReleaseCount) and NOT forwarded. And (2)
// BackendObject_DirectGLES::MakeEGLCurrent, under an active transport only, skips the native call
// when the requested draw surface is the one already natively current on this thread
// (IsBackendContextCurrentOnThisThread, which is EGL ground truth). Measured at the EGL function
// table by ServerLoopTest's C7 control on a real llvmpipe context: surface creation + two
// identical make-currents + a client release + a bind after the release = ONE native
// eglMakeCurrent, ZERO native releases, and the apply thread still the owner afterwards. So
// DirectGLES.cpp's six cache invalidations run once per context lifetime rather than once per
// client make-current, and the 16 IsBackendContextCurrentOnThisThread() / 16 CanTouchGLNow()
// sites answer TRUE on the server. The tuple is FORGOTTEN (N-3) on every event after which the
// native context it names may be gone - ReleaseEGLResources, ReleaseEGLSurface of the surface it
// names, a surface (re)creation, backend destruction - so a recycled handle value after a destroy
// is a real bind again and never a silent no-context-current. The client's nine EGL virtuals
// become BLOCKING control requests executed here. ReleaseEGLResources and
// ~BackendObject_DirectGLES MUST be blocking: MobileGL::Destroy() (MobileGL/Init.cpp:68)
// otherwise walks on while the server still holds the context.
//
// THE FALLBACK IS PRE-DECLARED, NOT INVENTED UNDER PRESSURE (R-1). If the context migration is
@@ -130,10 +143,13 @@ namespace MobileGL::MG_Remote::Server {
Uint64 DrainedRecords() const;
Uint64 ParkCount() const;
// C7 / ID-54 diagnostics, read by the C7 control. NativeBindCount is how many times
// ServerMakeEGLCurrent forwarded a REAL native bind (a new tuple); ClientReleaseCount how
// many client release-current requests were recorded-and-not-forwarded. Two identical
// binds must move the first by one and the second not at all.
// C7 / ID-54 diagnostics, read by ServerLoopTest's C7 and N-3 controls. NativeBindCount is
// how many times ApplyMakeCurrent FORWARDED a bind to the backend (a tuple it did not
// hold); ClientReleaseCount how many client release-current requests were recorded and
// not forwarded. Two identical binds must move the first by one and the second not at
// all. The number of native eglMakeCurrent calls the DRIVER saw is a different number -
// the backend object skips the native call for a surface already current (header block)
// - and the control reads that one at the EGL function table, not here.
Uint64 NativeBindCount() const;
Uint64 ClientReleaseCount() const;
@@ -148,6 +164,20 @@ namespace MobileGL::MG_Remote::Server {
MakeCurrentOutcome ApplyMakeCurrent(MG_Backend::BackendObject* backend, EGLDisplay dpy,
EGLSurface draw, EGLSurface read, EGLContext ctx);
// N-3: forget the tuple ApplyMakeCurrent last bound. Apply thread only, like the tuple
// itself (the forwarders that call these run their Args::Run there). Called on every
// event after which the native context that tuple named may no longer exist -
// ReleaseEGLResources, ReleaseEGLSurface of a surface the tuple names, a surface
// (re)creation (BackendObject_DirectGLES destroys the context to create a different
// surface), backend destruction - so the next make-current with the SAME handle values
// (EGL handles are recycled; on this host every one of them is literally 0x1) is
// classified as a real bind, not as a RepeatNoOp that binds nothing, runs no base-class
// bookkeeping and republishes no caps. Forgetting is always safe: the cost of a
// forgotten-but-still-current tuple is one forwarded bind the backend object dedups
// natively; the cost of a remembered-but-dead one is a silent no-context-current.
void ForgetCurrentTuple();
void ForgetCurrentTupleIfItNames(EGLSurface surface);
private:
void ApplyThreadMain();
// Part of the apply thread's park predicate: a posted control request must be able to
@@ -224,12 +254,15 @@ namespace MobileGL::MG_Remote::Server {
// owner slot g_backendContextOwnerThread (DirectGLES.cpp:11865) is stamped with whatever
// thread got there. So BackendObject_Remote's nine EGL virtuals - package c1's - call these
// twelve, each of which is a BLOCKING control request that runs the SERVER's backend object
// on mgl-srv-apply. eglMakeCurrent then runs ONCE, on that thread, and is never released:
// g_backendContextOwnerThread is written once, DirectGLES.cpp:11933-11953's six cache
// invalidations become a one-off startup cost instead of a per-migration storm, the
// per-frame EGL re-verification stamp is permanently true, and the 16
// on mgl-srv-apply. The native eglMakeCurrent then runs once per context lifetime on that
// thread (the surface's own creation binds; the client's make-currents onto that surface
// are deduped at both layers, header block above) and a client release is never forwarded,
// so g_backendContextOwnerThread is written once per context lifetime, DirectGLES.cpp's six
// cache invalidations run once per context lifetime rather than once per client
// make-current, the per-frame EGL re-verification stamp holds, and the 16
// IsBackendContextCurrentOnThisThread() sites plus the 16 CanTouchGLNow() sites answer TRUE
// on the server instead of silently degrading.
// on the server instead of silently degrading. Measured, not assumed: ServerLoopTest's C7
// control counts the driver's eglMakeCurrent calls at the EGL function table.
//
// TWO OF THEM MUST BLOCK OR THE PROCESS TEARS ITS OWN CONTEXT DOWN UNDER ITSELF:
// ReleaseEGLResources (reached from EGLImpl.cpp:326, which for DirectGLES runs