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

[Fix] (Remote): honor fence wait budgets and reject invalid readback status

Browse Source
This commit is contained in:
swung0x48
2026-09-16 14:12:14 -04:00
parent a021e3cc5d
commit 82683d4a83
3 changed files with 164 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Remote/Client/ClientSession.cpp
+29 -3
View File
@@ -124,6 +124,30 @@ namespace MobileGL::MG_Remote::Client {
// second-scale wait, while a lost record never completes at all. 30 s separates the two
// without turning a loaded CI machine into a red lane, and the Fatal names the seq.
constexpr Uint32 kBarrierTimeoutMs = 30000;
Uint64 AppliedWaitBudgetMs(MG_Pipe::MGPWireOp op, const void* payload) {
if (op != MG_Pipe::MGPWireOp::FenceWait) return kBarrierTimeoutMs;
const Uint64 timeoutNs = static_cast<const MG_Pipe::MGPFenceWait*>(payload)->TimeoutNs;
// ClientWaitSync may legitimately block longer than the ordinary verb barrier.
// Round up without overflowing UINT64_MAX, then allow the usual transport grace.
// FenceWaitServer queues a GPU wait; its GL_TIMEOUT_IGNORED is not a CPU budget.
return timeoutNs / 1000000 + (timeoutNs % 1000000 != 0) + kBarrierTimeoutMs;
}
Transport::SessionWait WaitForAppliedBudget(Transport::SessionProducer& producer,
Uint64 seq, Uint64 remainingMs) {
// The transport takes Uint32 milliseconds with UINT32_MAX meaning forever.
// Keep even the largest GL timeout finite, using bounded chunks and preserving
// the doorbell's immediate shutdown result. Avoid a giant chrono deadline too.
constexpr Uint32 kMaxFiniteWaitMs = Transport::kWaitForever - 1;
for (;;) {
const Uint32 chunkMs = remainingMs > kMaxFiniteWaitMs
? kMaxFiniteWaitMs : static_cast<Uint32>(remainingMs);
const auto wait = producer.WaitForApplied(seq, chunkMs);
if (wait != Transport::SessionWait::TimedOut || remainingMs <= chunkMs) return wait;
remainingMs -= chunkMs;
}
}
// How many queued control frames one pump will drain. A backlog deeper than this is a
// finding, not a steady state.
constexpr Uint32 kMaxControlFramesPerPump = 16;
@@ -776,7 +800,8 @@ namespace MobileGL::MG_Remote::Client {
}
const BarrierWaitScope waiting;
const Transport::SessionWait wait = m_producer.WaitForApplied(seq, kBarrierTimeoutMs);
const Uint64 waitBudgetMs = AppliedWaitBudgetMs(op, payload);
const Transport::SessionWait wait = WaitForAppliedBudget(m_producer, seq, waitBudgetMs);
if (wait == Transport::SessionWait::ShutDown) {
// The doorbell died: the server went away. The only thing that returns from a
// kWaitForever park, and therefore the only way a client blocked in the barrier
@@ -796,9 +821,10 @@ namespace MobileGL::MG_Remote::Client {
}
if (wait != Transport::SessionWait::Reached) {
MGLOG_F("MGPipe: Fatal{BarrierTimeout, \"%s\"} - appliedSeq did not reach %llu within "
"%u ms. A bounded wait is deliberate: a wedged CI job and a lost record look "
"%llu ms. A bounded wait is deliberate: a wedged CI job and a lost record look "
"identical from outside, and only one of them is a bug worth finding",
Wire::WireOpName(op), static_cast<unsigned long long>(seq), kBarrierTimeoutMs);
Wire::WireOpName(op), static_cast<unsigned long long>(seq),
static_cast<unsigned long long>(waitBudgetMs));
std::abort();
}
MobileGL/MG_Remote/Client/EmitTables.cpp
+4
View File
@@ -873,6 +873,10 @@ namespace MobileGL::MG_Remote::Client {
"Fatal here rather than a buffer of stale bytes");
std::abort();
}
if (status != Wire::ReplySink::kStatusOk) {
MGLOG_F("MGPipe: Fatal{ReplyStatusInvalid, \"ReadPixels\"} - unknown reply status %d", status);
std::abort();
}
if (replySize != expected) {
MGLOG_F("MGPipe: Fatal{ReadbackReplyShort, \"ReadPixels %llu < %llu\"} - the OK "
"reply carried fewer bytes than the read's own DstSize (CONTRACT-P5 row "
MobileGL/MG_Test/Wire/RemoteClientControls.inc
+131
View File
@@ -20,6 +20,54 @@ struct RepliesTag {
friend Type PeerMember(RepliesTag);
};
template struct PeerAccess<RepliesTag, &Codec::PipeWireDecoder::m_replies>;
struct ProducerBellTag {
using Type = Transport::Doorbell* Transport::SessionProducer::*;
friend Type PeerMember(ProducerBellTag);
};
template struct PeerAccess<ProducerBellTag, &Transport::SessionProducer::m_selfBell>;
// Observe the actual transport park selected by EmitAndWait. The peer waits until
// the client parks, so no GPU or wall-clock 60-second sleep is required.
struct BudgetBell : Transport::Doorbell {
Transport::Doorbell* original;
std::atomic<Uint32> firstMs{0};
std::atomic<Bool> entered{false};
Bool shutDown = false;
explicit BudgetBell(Transport::Doorbell* bell) : original(bell) {}
void Notify() override { original->Notify(); }
void Reset() override { original->Reset(); }
bool Park(std::uint32_t timeoutMs) override {
if (!entered.load()) {
firstMs.store(timeoutMs);
entered.store(true);
}
return shutDown ? false : original->Park(timeoutMs);
}
bool Dead() const override { return (shutDown && entered.load()) || original->Dead(); }
};
struct FenceBudgetPeer : Codec::WireVerbSink {
BudgetBell& bell;
std::atomic<Bool> release{false};
explicit FenceBudgetPeer(BudgetBell& value) : bell(value) {}
void AwaitPark() {
while (!bell.entered.load() || (bell.shutDown && !release.load())) std::this_thread::yield();
}
Bool OnFenceWait(const MGPFenceWait&, Uint32& result) override {
AwaitPark();
result = GL_TIMEOUT_EXPIRED;
return true;
}
Bool OnFenceWaitServer(const MGPFenceWait&) override { AwaitPark(); return true; }
void Install() {
if (Srv::ServerLoopInstance().RunOnApplyThread([](void* self) {
auto& decoder = Srv::ServerSessionInstance().Applier().*PeerMember(DecoderTag{});
decoder.SetVerbSink(static_cast<FenceBudgetPeer*>(self));
return MOBILEGL_OK;
}, this) != MOBILEGL_OK) ::_exit(92);
}
};
struct BackendTag {
using Type = UniquePtr<MG_Backend::BackendObject> Srv::ServerLoop::*;
friend Type PeerMember(BackendTag);
@@ -151,6 +199,89 @@ TEST(RemoteClientControls, ErrorReadPixelsReplyRefusesByName) {
ExpectNamedAbort(child, "Fatal{ReplyError, \"ReadPixels\"}");
}
TEST(RemoteClientControls, UnknownReadPixelsReplyStatusRefusesTightAndBounce) {
for (const Bool bounce : {false, true}) {
const auto child = RunInChild([bounce] {
StartControlSession();
if (bounce) {
MG_State::pGLContext = MakeUnique<MG_State::GLState::GLContext>();
MG_State::pGLContext->SetPixelStoreParam(PixelStoreParam::PackRowLength, 8);
}
ReadPeer peer;
peer.status = 3;
peer.Install();
Uint8 pixels[128]{};
RemoteEmitTable().GL.ReadPixels(0, 0, 4, 3, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
});
ExpectNamedAbort(child, "Fatal{ReplyStatusInvalid, \"ReadPixels\"}");
}
}
TEST(RemoteClientControls, FenceWaitBudgetHonorsGlTimeoutAndFiniteTransportChunks) {
struct WaitCase { Uint64 timeoutNs; MGPWireOp op; Uint32 expectedParkMs; };
const WaitCase cases[] = {
{0, MGPWireOp::FenceWait, 30000},
{1, MGPWireOp::FenceWait, 30001},
{60000000000ull, MGPWireOp::FenceWait, 90000},
{~Uint64(0), MGPWireOp::FenceWait, Transport::kWaitForever - 1},
{~Uint64(0), MGPWireOp::FenceWaitServer, 30000},
};
for (const auto test : cases) {
const auto child = RunInChild([test] {
StartControlSession();
auto& session = ClientSessionInstance();
auto& producer = session.Producer();
BudgetBell bell(producer.SelfDoorbell());
FenceBudgetPeer peer(bell);
peer.Install();
producer.*PeerMember(ProducerBellTag{}) = &bell;
MGPFenceWait request{};
request.TimeoutNs = test.timeoutNs;
Uint32 result = 0;
Int32 status = -1;
Uint64 size = 0;
session.EmitAndWait(test.op, &request, sizeof(request), nullptr, 0,
&result, sizeof(result), &status, &size);
producer.*PeerMember(ProducerBellTag{}) = bell.original;
const auto observed = bell.firstMs.load();
// Doorbell converts the remaining deadline to whole milliseconds. Allow
// scheduler delay, but neither a 30s clamp nor an unbounded sentinel.
if (!bell.entered.load() || observed > test.expectedParkMs ||
observed < test.expectedParkMs - 100 || observed == Transport::kWaitForever) ::_exit(93);
if (test.op == MGPWireOp::FenceWait &&
(status != Codec::ReplySink::kStatusOk || size != sizeof(result) ||
result != GL_TIMEOUT_EXPIRED)) ::_exit(94);
session.Stop();
});
ExpectChildSuccess(child);
}
}
TEST(RemoteClientControls, LongFenceWaitBudgetStillWakesOnShutdown) {
const auto child = RunInChild([] {
StartControlSession();
auto& session = ClientSessionInstance();
auto& producer = session.Producer();
BudgetBell bell(producer.SelfDoorbell());
bell.shutDown = true;
FenceBudgetPeer peer(bell);
peer.Install();
producer.*PeerMember(ProducerBellTag{}) = &bell;
MGPFenceWait request{};
request.TimeoutNs = ~Uint64(0);
Uint32 result = 0;
Int32 status = -1;
Uint64 size = 7;
session.EmitAndWait(MGPWireOp::FenceWait, &request, sizeof(request), nullptr, 0,
&result, sizeof(result), &status, &size);
producer.*PeerMember(ProducerBellTag{}) = bell.original;
peer.release.store(true);
if (status != Codec::ReplySink::kStatusDeclined || size != 0) ::_exit(95);
session.Stop();
});
ExpectChildSuccess(child);
}
TEST(RemoteClientControls, ResourceRespecifyErrorIsNotADecline) {
const auto child = RunInChild([] {
StartControlSession();