[Fix] (Client): drop the late reply of a timed-out surface op instead of reading it as the next op's

This commit is contained in:
2026-10-02 13:34:17 -04:00
parent 239fc228fc
commit d03fe4c9dd
2 changed files with 46 additions and 0 deletions
@@ -1504,6 +1504,10 @@ namespace MobileGL::MG_Remote::Client {
// One op is outstanding at a time (m_remoteControlMutex), so a report naming any
// other seq is a server that has lost track of which op it is running.
const auto* progress = envelope->msg_as_SurfaceProgress();
// An OLDER seq is an op this client already gave up on (its reply budget expired
// while the server's apply thread was blocked - a long GPU stall does it): the
// server is reporting, late, on work it did take. Dropped, like its late reply.
if (progress != nullptr && progress->seq() < frame.seq) continue;
if (progress == nullptr || progress->seq() != frame.seq) {
MGLOG_E("MG_Remote client: a SurfaceProgress names seq %llu while seq %llu is "
"the op outstanding",
@@ -1545,6 +1549,19 @@ namespace MobileGL::MG_Remote::Client {
return MOBILEGL_ERR_PROTOCOL_MISMATCH;
}
const ::MobileGL::Wire::SurfaceReply* wire = envelope->msg_as_SurfaceReply();
if (wire->seq() < frame.seq) {
// THE LATE ANSWER TO AN OP THIS CLIENT TIMED OUT ON. One op is outstanding at a
// time, so every reply older than the one owed is exactly that: the server
// answered after the budget expired. Treating it as a protocol mismatch left the
// session one reply behind for good - every later op read its predecessor's
// answer and failed, which wedged a client for the rest of its life after one
// multi-second GPU stall. Drop it and keep waiting for ours.
MGLOG_W("MG_Remote client: dropping the late SurfaceReply for seq %llu (an op that "
"timed out); still waiting for seq %llu",
static_cast<unsigned long long>(wire->seq()),
static_cast<unsigned long long>(frame.seq));
continue;
}
if (wire->seq() != frame.seq) {
MGLOG_E("MG_Remote client: SurfaceReply names seq %llu, we asked for %llu",
static_cast<unsigned long long>(wire->seq()),
+29
View File
@@ -1122,6 +1122,35 @@ TEST(ServerSpawnTest, AFrozenServerReportsNoProgressAndTheOpStillTimesOutAtTheBu
EXPECT_FALSE(Client::ClientSession::DeviceLost()) << "a frozen server is not a lost device (5.4)";
}
// THE LATE ANSWER TO AN OP THE CLIENT GAVE UP ON. A server whose apply thread is blocked for
// longer than the reply budget - a GPU hang the driver takes ~10 s to reset does it - answers the
// timed-out op when it comes back. The client used to read that answer as the NEXT op's and call
// it a protocol mismatch, and from then on every op read its predecessor's reply: one long GPU
// stall left the client unable to make a context current for the rest of its life. The late reply
// is now dropped and the next op reads its own. Red once by restoring the mismatch arm: the second
// op comes back MOBILEGL_ERR_PROTOCOL_MISMATCH.
TEST(ServerSpawnTest, AnOpAfterOneThatTimedOutReadsItsOwnReplyAndNotThatLateOne) {
const ScopedEnvironment headless("EGL_PLATFORM", HeadlessEglPlatform());
const ShortReplyBudgets budgets(1000);
Session session;
ASSERT_TRUE(Bring("latereply", &session));
ScopedSessionCleanup cleanup{session};
ASSERT_EQ(Handshake(session), MOBILEGL_OK);
ASSERT_EQ(::kill(session.server.pid, SIGSTOP), 0);
Server::SurfaceControlFrame first = PbufferCreation();
const MobileGLResult timedOut = Client::ClientSessionInstance().RunRemoteSurfaceControlFrame(first);
(void)::kill(session.server.pid, SIGCONT);
ASSERT_EQ(timedOut, MOBILEGL_ERR_TIMEOUT) << "the frozen server's op was not timed out";
Server::SurfaceControlFrame second = PbufferCreation();
second.surface = 2;
const MobileGLResult rc = Client::ClientSessionInstance().RunRemoteSurfaceControlFrame(second);
EXPECT_EQ(rc, MOBILEGL_OK) << "the op after a timed-out one came back rc=" << static_cast<int>(rc)
<< ": it read the late reply of the op before it";
EXPECT_FALSE(Client::ClientSession::DeviceLost()) << "a late reply is not a lost device";
}
TEST(ServerSpawnTest, TheHandshakeCarriesTwoDifferentProcessIdsAndNotTwoZeroes) {
// CONTRACT-P6 4.3. Hello::pid was hard-coded 0 and Welcome::serverPid ECHOED it back, so the
// handshake carried 0 and 0 and named nothing at all. Both now carry real ids, which makes