mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 01:53:24 +09:00
[Fix] (MG_Remote/Client): a DECLINED reply is an answer, not a corruption - the fence and query consumers stop folding status 1 into the abort, and the uncarried-bytes self-check spares a session that has latched device-lost
- ReadFenceReply (EmitTables.cpp) and ReadQueryReply (QueryEmit.inc) tested `status != kStatusOk`,
which is two values in one branch. ReplySink's own header says "DECLINED IS A REAL ANSWER, not
a failure" (PipeWireCodec.h:457-460), and the escape rows are held to the other half of the same
rule - "ERROR IS NOT A DECLINE ... an escape may not fold 2 into `false`" (WireTables.cpp:446-450).
- BOTH VALUES OF 1 ARE REACHABLE WITHOUT A BYTE OF CORRUPTION: the server declines these five rows
by design whenever it cannot answer them (PipeWireCodec.cpp:1820-1830 for the two fence rows,
:1841-1861 for the three query rows - a zero-length answer to a kWaitReply row, R-5), and the
client produces the same status itself once the session stops carrying records
(ClientSession.cpp:1837 the device-lost latch, :2025 a doorbell that died during teardown, where
the comment says outright that "the verb did not happen" is reported as DECLINED and not ERROR).
- WHAT THE FOLD COST: after a clean device loss Minecraft 26.2's next glClientWaitSync - the
chunk-upload fences - died of Fatal{ProtocolCorruption, "Fence.reply"} (rc=134) instead of taking
the no-op the design promises ("GL calls become no-ops", design/08-runtime-and-platform.md:33).
The query rows had the same shape waiting for them.
- The no-op is the one this frontend already gives for a sync it cannot wait on: GL_Sync.cpp:103-106,
PipeApplier.cpp:175-177 and DirectGLES.cpp:16489 all answer GL_ALREADY_SIGNALED. A query row
answers its own "no answer yet" value - availability false, Produced 0 (which the caller already
returns as false with the out-parameter untouched), timestamp 0.
- ERROR and a short OK keep the named Fatal they always had: a transport fault is not the server
saying no, and an OK answer that arrived empty is a reply nobody can read. ReadPixels keeps its
deliberate Fatal{ReadbackDeclined}: unlike these rows a blocking readback cannot succeed short of
complete pixels, and that ruling is untouched.
- PipeFill.cpp's UncarriedInitialBytes self-check now spares a session that has latched device-lost.
The resource_subdata it counts is DECLINED by design there, so "it emitted nothing" is the latch
working rather than bytes this client forgot to carry - and there is no server left for them to
exist on. P12's exit gate (a) is that killing the server gives a CLEAN device-lost latch, not a
corpse at the next glBufferData that carries data.
- Four cases in RemoteClientControls.inc drive the production consumers over a live session whose
peer declines them. Red-once run for all three Fatals (Fence.reply, Query.reply,
UncarriedInitialBytes), and ErrorFenceReplyStillRefusesByName is the control that keeps the
tolerance from becoming blanket. RemoteClientTest 166/166; the host unit lane 2610/2610.
- Split only: both consumer files compile under MOBILEGL_BUILD_DISAGGREGATED and the PipeFill hunk
sits inside its `#if`, so a pull build preprocesses to exactly what it had (G1).
This commit is contained in:
BZLZHH
@@ -1112,7 +1112,19 @@ namespace MobileGL::MG_Pipe {
|
||||
// below and this aborts by name on the first glBufferData that carries data.
|
||||
const Uint64 before = MG_Remote::Client::ClientWireRecordsEmitted();
|
||||
MGPipeEmitResourceSubData(buffer, 0, static_cast<SizeT>(buffer.GetSize()));
|
||||
if (MG_Remote::Client::ClientWireRecordsEmitted() == before) {
|
||||
// EXCEPT ON A SESSION THAT HAS STOPPED CARRYING RECORDS (P12). Once the device-lost
|
||||
// latch is set every verb is DECLINED by design (CONTRACT-P6 5.3, ClientSession.cpp
|
||||
// :1837) - the resource_subdata above included - so "it emitted nothing" is the latch
|
||||
// working, not bytes this client forgot to carry: there is no server left for them to
|
||||
// exist on, and P12's own exit gate (a) is that killing the server gives a CLEAN
|
||||
// device-lost latch rather than a corpse. Aborting here turned every clean loss into a
|
||||
// client SIGABRT at its next glBufferData that carries data.
|
||||
//
|
||||
// THE SELF-CHECK KEEPS ITS FULL FORCE ON A LIVE SESSION, which is the case it was
|
||||
// written for: there, a follow-up that emits nothing is still the R-13.3 gap it names
|
||||
// - and DeviceLost() is false.
|
||||
if (MG_Remote::Client::ClientWireRecordsEmitted() == before &&
|
||||
!MG_Remote::Client::ClientSession::DeviceLost()) {
|
||||
// UncarriedInitialBytes, not a second word for the same family: WireTables.cpp:418
|
||||
// already dies of exactly this - a respecify that crossed with no initial bytes -
|
||||
// under that name, and two words for one family is the vocabulary drift a6
|
||||
|
||||
@@ -1577,6 +1577,43 @@ namespace MobileGL::MG_Remote::Client {
|
||||
Uint64 replyBytes = 0;
|
||||
session.EmitAndWait(op, payload, bytes, nullptr, 0, &result, sizeof(result),
|
||||
&status, &replyBytes);
|
||||
// A DECLINED SYNC REPLY IS AN ANSWER, NOT A CORRUPTION (P12). ReplySink opens the
|
||||
// three-value space with "DECLINED IS A REAL ANSWER, not a failure"
|
||||
// (PipeWireCodec.h:457-460) and the escape rows are held to it the other way round -
|
||||
// "ERROR IS NOT A DECLINE", an escape may not fold 2 into false
|
||||
// (WireTables.cpp:446-450). This site folded 1 AND 2 into one abort.
|
||||
//
|
||||
// BOTH VALUES OF 1 ARE REACHABLE WITHOUT A BYTE OF CORRUPTION:
|
||||
// - the SERVER declines these two rows by design whenever it cannot answer them -
|
||||
// PostReply(op, seq, ok ? kStatusOk : kStatusDeclined, ok ? &result : nullptr,
|
||||
// ok ? sizeof(result) : 0) at PipeWireCodec.cpp:1820-1830, which is a legitimate
|
||||
// zero-length answer to a kWaitReply row (R-5);
|
||||
// - the CLIENT produces the same status itself once the session stops carrying
|
||||
// records: the device-lost latch (ClientSession.cpp:1837) and a doorbell that died
|
||||
// during teardown (ClientSession.cpp:2025, where the comment says outright that
|
||||
// "the verb did not happen" is reported as DECLINED and not as ERROR).
|
||||
//
|
||||
// AND THE NO-OP IS THE ONE THIS FRONTEND ALREADY GIVES for a fence it cannot wait on:
|
||||
// GL_Sync.cpp:103-106 answers GL_ALREADY_SIGNALED for a sync with no backend handle,
|
||||
// PipeApplier.cpp:175-177 answers GL_ALREADY_SIGNALED for a fence with no native
|
||||
// object (a fence that provably did no work), and DirectGLES.cpp:16489 does the same
|
||||
// for an absent sync. A lost device is that same class of fact.
|
||||
//
|
||||
// WHAT ABORTING HERE COSTS, MEASURED: after a clean device loss Minecraft 26.2's next
|
||||
// glClientWaitSync - the chunk-upload fences - reached this line with DECLINED and died
|
||||
// of Fatal{ProtocolCorruption, "Fence.reply"}, rc=134, instead of the no-op the design
|
||||
// promises ("GL calls become no-ops", design/08-runtime-and-platform.md:33).
|
||||
if (status == Wire::ReplySink::kStatusDeclined) {
|
||||
MGLOG_E_ONCE("MG_Remote client: the peer DECLINED a %s reply - the verb did not "
|
||||
"happen (a lost device, or a teardown); answering the no-op the "
|
||||
"frontend uses for a fence it cannot wait on",
|
||||
Wire::WireOpName(op));
|
||||
return op == MG_Pipe::MGPWireOp::FenceWait ? static_cast<Uint32>(GL_ALREADY_SIGNALED)
|
||||
: 1u;
|
||||
}
|
||||
// ERROR AND A SHORT OK ARE STILL THE PROTOCOL CORRUPTIONS THEY ALWAYS WERE: a
|
||||
// transport fault is not the server saying no, and an OK answer that arrived empty is
|
||||
// a reply nobody can read.
|
||||
if (status != Wire::ReplySink::kStatusOk || replyBytes != sizeof(result))
|
||||
Wire::WireProtocolFatal("Fence.reply", "missing or malformed sync result");
|
||||
return result;
|
||||
|
||||
@@ -16,6 +16,30 @@
|
||||
Int32 status = Wire::ReplySink::kStatusError;
|
||||
Uint64 returned = 0;
|
||||
session.EmitAndWait(op, request, size, nullptr, 0, &result, sizeof(result), &status, &returned);
|
||||
// A DECLINED QUERY IS AN ANSWER TOO (P12), for ReadFenceReply's reason and on the
|
||||
// same three-value space (PipeWireCodec.h:457-460). The server declines these three
|
||||
// rows by design whenever it cannot answer them - QueryAvailable :1841-1847,
|
||||
// QueryResult :1848-1854 and QueryTimestamp :1855-1859 all post kStatusDeclined with
|
||||
// a zero-length payload - and the client produces the same status once the session
|
||||
// stops carrying records (ClientSession.cpp:1837 is the device-lost latch, :2025 a
|
||||
// doorbell that died during teardown). `returned != sizeof(result)` is guaranteed for
|
||||
// such an answer, so the guard below used to abort on every one of them.
|
||||
//
|
||||
// THE ZEROED RESULT IS THE NO-OP EACH ROW ALREADY HAS, and it is what a query has
|
||||
// when there is no answer YET rather than no answer ever:
|
||||
// - QueryAvailable -> 0 -> false, "the result is not available" (the same answer an
|
||||
// unfinished query gives - GL_QUERY_RESULT_AVAILABLE exists to be told that);
|
||||
// - QueryResult -> Produced = 0 -> the caller returns false with *value untouched
|
||||
// (:70 spells the healthy analogue of the same answer);
|
||||
// - QueryTimestamp -> 0, no timestamp to report.
|
||||
// ERROR stays fatal below, and so does an OK reply that arrived short.
|
||||
if (status == Wire::ReplySink::kStatusDeclined) {
|
||||
MGLOG_E_ONCE("MG_Remote client: the peer DECLINED a %s reply - the verb did not "
|
||||
"happen (a lost device, or a teardown); answering the row's "
|
||||
"'no answer yet' value",
|
||||
Wire::WireOpName(op));
|
||||
return result;
|
||||
}
|
||||
if (status != Wire::ReplySink::kStatusOk || returned != sizeof(result))
|
||||
Wire::WireProtocolFatal("Query.reply", "missing or malformed backend query reply");
|
||||
return result;
|
||||
|
||||
@@ -736,4 +736,130 @@ TEST(RemoteClientControls, ADrawOnASessionWithoutTheRunAheadBitStaysStampedBarri
|
||||
});
|
||||
ExpectChildSuccess(child);
|
||||
}
|
||||
|
||||
// =====================================================================================
|
||||
// P12: a DECLINED reply is an answer, not a corruption
|
||||
// =====================================================================================
|
||||
//
|
||||
// ReplySink's status space is three-valued and its own header says what the values mean:
|
||||
// 0 OK, 1 DECLINED IS A REAL ANSWER, 2 ERROR (PipeWireCodec.h:457-460). The escape rows are
|
||||
// held to the other half of the same rule - "ERROR IS NOT A DECLINE", an escape may not fold
|
||||
// 2 into `false` (WireTables.cpp:446-450). The fence and query consumers folded BOTH 1 and 2
|
||||
// into a single abort (EmitTables.cpp ReadFenceReply, QueryEmit.inc ReadQueryReply), so a
|
||||
// client that had latched device-lost - where every verb is DECLINED by design,
|
||||
// ClientSession.cpp:1837 - died of Fatal{ProtocolCorruption, "Fence.reply"} on its next
|
||||
// glClientWaitSync instead of going quiet the way the design promises ("GL calls become
|
||||
// no-ops", design/08-runtime-and-platform.md:33).
|
||||
//
|
||||
// THE PEER BELOW IS THE SERVER'S OWN DECLINE, not a substitution of the status byte: these
|
||||
// verbs answer false when the server cannot answer them, and the codec posts exactly that as
|
||||
// DECLINED with a zero-length payload (PipeWireCodec.cpp:1820-1830 for the two fence rows,
|
||||
// :1841-1861 for the three query rows). The CLIENT's own decline is pinned separately, by
|
||||
// LongFenceWaitBudgetStillWakesOnShutdown above.
|
||||
//
|
||||
// THE RED, RUN ONCE: take the `if (status == Wire::ReplySink::kStatusDeclined)` arm out of
|
||||
// ReadFenceReply and the first case below dies of Fatal{ProtocolCorruption, "Fence.reply"}
|
||||
// instead of asserting the no-op - which is the split client's Minecraft 26.2 crash.
|
||||
struct DecliningSyncPeer : Codec::WireVerbSink {
|
||||
Bool OnFenceCreate(const MG_Pipe::MGPHandleOnly&) override { return true; }
|
||||
Bool OnFenceStatus(const MG_Pipe::MGPHandleOnly&, Uint32&) override { return false; }
|
||||
Bool OnFenceWait(const MGPFenceWait&, Uint32&) override { return false; }
|
||||
Bool OnQueryCreate(const MGPQueryDesc&) override { return true; }
|
||||
Bool OnQueryBegin(const MGPQueryDesc&) override { return true; }
|
||||
Bool OnQueryCounter(const MGPQueryDesc&) override { return true; }
|
||||
Bool OnQueryAvailable(const MG_Pipe::MGPHandleOnly&, Uint32&) override { return false; }
|
||||
Bool OnQueryResult(const MGPQueryResultRequest&, Codec::QueryResultReply&) override { return false; }
|
||||
Bool OnQueryTimestamp(const MGPTimestampRequest&, Int64&) override { return false; }
|
||||
void Install() {
|
||||
const auto rc = Srv::ServerLoopInstance().RunProbeOnApplyThreadForTesting([](void* self) {
|
||||
auto& decoder = Srv::ServerSessionInstance().Applier().*PeerMember(DecoderTag{});
|
||||
decoder.SetVerbSink(static_cast<DecliningSyncPeer*>(self));
|
||||
return MOBILEGL_OK;
|
||||
}, this);
|
||||
if (rc != MOBILEGL_OK) ::_exit(130);
|
||||
}
|
||||
};
|
||||
|
||||
TEST(RemoteClientControls, DeclinedFenceReplyAnswersSignaledInsteadOfAborting) {
|
||||
const auto child = RunInChild([] {
|
||||
StartControlSession();
|
||||
DecliningSyncPeer peer;
|
||||
peer.Install();
|
||||
auto* sync = RemoteEmitTable().GL.FenceSync();
|
||||
if (sync == nullptr) ::_exit(131);
|
||||
// glClientWaitSync on a fence nobody can wait on: signaled now, as every fallback in
|
||||
// this tree already answers (GL_Sync.cpp:103-106, PipeApplier.cpp:175-177).
|
||||
if (RemoteEmitTable().GL.ClientWaitSync(sync, 0, 0) != GL_ALREADY_SIGNALED) ::_exit(132);
|
||||
// GL_SYNC_STATUS: the same answer, in the boolean the row carries.
|
||||
if (!RemoteEmitTable().GL.GetSyncStatus(sync)) ::_exit(133);
|
||||
RemoteEmitTable().GL.DeleteSync(sync);
|
||||
ClientSessionInstance().Stop();
|
||||
});
|
||||
ExpectChildSuccess(child);
|
||||
}
|
||||
|
||||
TEST(RemoteClientControls, DeclinedQueryRepliesAnswerNoAnswerYetInsteadOfAborting) {
|
||||
const auto child = RunInChild([] {
|
||||
StartControlSession();
|
||||
DecliningSyncPeer peer;
|
||||
peer.Install();
|
||||
auto* query = RemoteEmitTable().GL.BeginTimeElapsedQuery();
|
||||
if (query == nullptr) ::_exit(134);
|
||||
// GL_QUERY_RESULT_AVAILABLE: "not available" - the answer an unfinished query gives,
|
||||
// and the one that makes a waiting application yield rather than spin.
|
||||
if (RemoteEmitTable().GL.IsQueryResultAvailable(query)) ::_exit(135);
|
||||
// GL_QUERY_RESULT: no value produced, and the caller's out-parameter untouched.
|
||||
Uint64 nanoseconds = 7;
|
||||
if (RemoteEmitTable().GL.GetQueryResult64(query, false, &nanoseconds) || nanoseconds != 7)
|
||||
::_exit(136);
|
||||
// glGetInteger64v(GL_TIMESTAMP): there is no timestamp to report.
|
||||
if (RemoteEmitTable().GL.GetGpuTimestampNs() != 0) ::_exit(137);
|
||||
RemoteEmitTable().GL.DeleteBackendQuery(query);
|
||||
ClientSessionInstance().Stop();
|
||||
});
|
||||
ExpectChildSuccess(child);
|
||||
}
|
||||
|
||||
TEST(RemoteClientControls, ErrorFenceReplyStillRefusesByName) {
|
||||
// THE TOLERANCE IS NOT BLANKET. An ERROR is a transport fault or a server-side refusal,
|
||||
// never "the server said no" (R-5/M4), and it keeps the named Fatal it always had.
|
||||
const auto child = RunInChild([] {
|
||||
StartControlSession();
|
||||
// The rows still decline (so the fence is never handed to a backend that cannot
|
||||
// create one) and EVERY answer that is posted says ERROR instead.
|
||||
DecliningSyncPeer rows;
|
||||
rows.Install();
|
||||
ErrorPeer peer;
|
||||
peer.Install();
|
||||
auto* sync = RemoteEmitTable().GL.FenceSync();
|
||||
if (sync == nullptr) ::_exit(138);
|
||||
(void)RemoteEmitTable().GL.ClientWaitSync(sync, 0, 0);
|
||||
});
|
||||
ExpectNamedAbort(child, "Fatal{ProtocolCorruption, \"Fence.reply\"}");
|
||||
}
|
||||
|
||||
TEST(RemoteClientControls, DeviceLostRespecifyDoesNotDieOfTheUncarriedBytesCheck) {
|
||||
// P12 exit gate (a): killing the server must give a CLEAN device-lost latch. With the
|
||||
// latch set every verb is DECLINED, so the resource_subdata that R-13.3 owes emits
|
||||
// nothing - which is the latch working, not bytes the client forgot to carry. The check
|
||||
// at PipeFill.cpp:1115 compared client record ordinals only, so the next glBufferData
|
||||
// with data aborted the client by name instead.
|
||||
const auto child = RunInChild([] {
|
||||
StartControlSession();
|
||||
// Taken through the production entry point, the way the barrier and the encoder take
|
||||
// it when a descriptor whose far end only the peer held reports hangup.
|
||||
ClientSessionInstance().LatchDeviceLost("RemoteClientControls: the self-check case");
|
||||
MG_Config::Features.PipePush |= kMGPipeSubsystemResources;
|
||||
MG_State::pGLContext = MakeUnique<MG_State::GLState::GLContext>();
|
||||
GLuint buffer = 0;
|
||||
MG_Impl::GLImpl::GenBuffers(1, &buffer);
|
||||
MG_Impl::GLImpl::BindBuffer(0x8892, buffer);
|
||||
Uint8 seed[64];
|
||||
std::memset(seed, 0x5a, sizeof(seed));
|
||||
MG_Impl::GLImpl::BufferData(0x8892, sizeof(seed), seed, 0x88E4);
|
||||
ClientSessionInstance().Stop();
|
||||
});
|
||||
ExpectChildSuccess(child);
|
||||
}
|
||||
|
||||
#endif
|
||||
|
||||
Reference in New Issue
Block a user