[Fix] (MG_Test): the guilty-reset case waits for the held drain, not for an ack the hold delays, and the supervisor's refusal line is waited for in its log.

This commit is contained in:
2026-10-07 23:25:09 -04:00
parent 80c6aa39da
commit ddfbdab0e8
2 changed files with 25 additions and 7 deletions
@@ -191,7 +191,11 @@ TEST(UnixSupervisor, ConcurrentSharedMemoryClientsHaveSeparateWorkersAndAnExited
const PeerReport excess = RunPeer(server.endpoint, ApplyOneRecord, true, 15000);
EXPECT_EQ(excess.started, 0) << "the configured live-session limit was ignored";
EXPECT_NE(server.Log().find("Refuse{Busy} the session limit is reached"), std::string::npos) << server.Log();
// The refusal line reaches the supervisor's log asynchronously; the client can see the refusal
// first (one CI run read the log before the line landed), so the log is waited for.
EXPECT_TRUE(WaitFor([&] { return server.Log().find("Refuse{Busy} the session limit is reached") != std::string::npos; },
5000))
<< server.Log();
// The second worker must not hold the first peer's connection open after its EOF. This also
// proves CloseLocalCopy and CloseInForkedChild cover descriptors inherited by each worker.
+20 -6
View File
@@ -2026,10 +2026,15 @@ namespace {
// drain, without passing the apply loop's per-wake fault poll. That used to happen only under
// CPU load (one CI failure in this case); the between-records hook makes it happen every time:
// armed, it holds the drain after the record it follows until the GUILTY record is in the ring.
// The client does not wait for anything while the drain is held (the applied notification may
// only reach a parked client after the hook): it waits for the hold to be ENTERED, which says the
// healthy record has been applied, publishes the GUILTY record, and only then waits.
std::atomic<int> g_holdDrainForGuilty{0};
std::atomic<bool> g_holdEntered{false};
std::atomic<bool> g_guiltyPublished{false};
void HoldDrainUntilTheGuiltyRecordIsPublished() {
if (g_holdDrainForGuilty.exchange(0) != 1) return;
g_holdEntered.store(true, std::memory_order_release);
const auto until = std::chrono::steady_clock::now() + std::chrono::seconds(2);
while (!g_guiltyPublished.load(std::memory_order_acquire) && std::chrono::steady_clock::now() < until)
std::this_thread::yield();
@@ -2055,27 +2060,36 @@ namespace {
gles.glGetGraphicsResetStatus = &FakeGetGraphicsResetStatus;
MG_Backend::DirectGLES::SetGLESFuncsTable(gles);
int failed = 0;
const auto applyOne = [&](std::atomic<bool>* publishedFlag) {
const auto publish = [&] {
MG_Pipe::MGPMemoryBarrier barrier{};
barrier.Bits = 0x2000u;
const Uint64 seq = fixture.encoder.EncodeRecord(MG_Pipe::MGPWireOp::MemoryBarrier, &barrier, sizeof(barrier));
fixture.encoder.Publish();
fixture.producer.PublishAndNotify(seq);
if (publishedFlag != nullptr) publishedFlag->store(true, std::memory_order_release);
return seq;
};
const auto applied = [&](Uint64 seq) {
return fixture.producer.WaitForApplied(seq, 2000) == Transport::SessionWait::Reached;
};
// Another context hung the GPU; this one only waited behind it.
g_fakeResetStatus = GL_INNOCENT_CONTEXT_RESET;
if (!applyOne(nullptr)) failed |= kResetHealthyNotApplied;
if (!applyOne(nullptr) || MG_Remote::SessionLatched()) failed |= kResetInnocentEnded;
if (!applied(publish())) failed |= kResetHealthyNotApplied;
if (!applied(publish()) || MG_Remote::SessionLatched()) failed |= kResetInnocentEnded;
// The drain that applies this healthy record is held, after acknowledging it, until the
// GUILTY record below is published, so that drain pops the GUILTY record itself.
Server::ServerLoopInstance().SetBetweenRecordsHookForTesting(&HoldDrainUntilTheGuiltyRecordIsPublished);
g_holdDrainForGuilty = 1;
if (!applyOne(nullptr)) failed |= kResetHealthyNotApplied;
const Uint64 healthy = publish();
const auto until = std::chrono::steady_clock::now() + std::chrono::seconds(2);
while (!g_holdEntered.load(std::memory_order_acquire) && std::chrono::steady_clock::now() < until)
std::this_thread::yield();
if (!g_holdEntered.load(std::memory_order_acquire)) failed |= kResetHealthyNotApplied;
// This context's own work hung the GPU.
g_fakeResetStatus = GL_GUILTY_CONTEXT_RESET;
if (applyOne(&g_guiltyPublished)) failed |= kResetGuiltyApplied;
const Uint64 guilty = publish();
g_guiltyPublished.store(true, std::memory_order_release);
if (!applied(healthy)) failed |= kResetHealthyNotApplied;
if (applied(guilty)) failed |= kResetGuiltyApplied;
Server::ServerLoopInstance().SetBetweenRecordsHookForTesting(nullptr);
if (!MG_Remote::SessionLatched() || MG_Remote::SessionLatchedFamily() != MG_Remote::MGFatalFamily::BackendDeviceLost ||
std::strstr(MG_Remote::SessionLatchedLine(), "Espryt:apply") == nullptr)