[Fix] (MG_Pipe, MG_Test/Wire, scripts/data): aim the session-loss knob after the target is ready, not after 100 ms

- MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID=file:<path> takes the pid from a file read at every ask, so a running server can be aimed, as the Android property already can
- CompositorRecovery and ClientRecovery's pid-targeted case aim it once their target says it got there (its first frame; current): under load the 100 ms clock lost the compositor inside its first make-current
- the knob's comment no longer promises an ordering a clock cannot keep
- FatalFunnelSeam.TheSessionLossKnobTakesAPidOrAFileThatHoldsOne covers both forms
- the file-size baseline drops by the 83 lines the previous commit moved out of PipeApply.cpp

MobileGL/MG_Pipe/PipeDeviceLossKnobs.cpp

@@ -61,10 +61,28 @@ namespace MobileGL::MG_Pipe {
         return true;
     }
 
+    namespace {
+        // The knob's pid: the value itself, or, for "file:<path>", the number that file holds
+        // now (none while it is missing or empty).
+        long DebugLossTargetOf(const char* value) {
+            static constexpr char kFilePrefix[] = "file:";
+            if (std::strncmp(value, kFilePrefix, sizeof(kFilePrefix) - 1) != 0) return std::strtol(value, nullptr, 10);
+            std::FILE* file = std::fopen(value + sizeof(kFilePrefix) - 1, "r");
+            if (file == nullptr) return 0;
+            char text[32] = {};
+            const SizeT read = std::fread(text, 1, sizeof(text) - 1, file);
+            std::fclose(file);
+            text[read] = '\0';
+            return std::strtol(text, nullptr, 10);
+        }
+    } // namespace
+
     bool MGPipeDebugSessionLossDue(unsigned clientPid) {
         if (clientPid == 0) return false;
         // The first call on a thread only starts the clock, so a session is first asked 100 ms
-        // after it started rendering - never in the middle of its first make-current.
+        // after its context was bound. That is a delay, not an ordering: a client still in its
+        // first make-current 100 ms later is lost in it. A test that needs the loss after a known
+        // point aims the knob through a file once its client got there.
         thread_local auto lastAsked = std::chrono::steady_clock::time_point{};
         const auto now = std::chrono::steady_clock::now();
         if (lastAsked == std::chrono::steady_clock::time_point{}) {
@@ -75,7 +93,7 @@ namespace MobileGL::MG_Pipe {
         lastAsked = now;
         long target = 0;
         if (const char* value = std::getenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID")) {
-            target = std::strtol(value, nullptr, 10);
+            target = DebugLossTargetOf(value);
         }
 #if defined(__ANDROID__)
         else {

MobileGL/MG_Pipe/PipeSessionFail.h

@@ -154,7 +154,8 @@ namespace MobileGL::MG_Pipe {
     // server's apply thread asks after every drain. It is how one application among several is
     // made to lose its device - a browser's GPU process, which renders into dma-bufs and never
     // presents, included. Asked only for a session with a context bound; asks the environment /
-    // property at most every 100 ms per thread.
+    // property at most every 100 ms per thread. P may be "file:<path>": the pid that file holds
+    // when asked (none while it is missing or empty), so a running server is aimed by writing it.
     bool MGPipeDebugSessionLossDue(unsigned clientPid);
 #else
     // P13 W5: the device-loss knobs end a SESSION; a library without a transport has none to end.

MobileGL/MG_Test/Wire/ClientRecoveryTest.cpp

@@ -779,9 +779,9 @@ namespace {
     }
 
     // THE SESSION-TARGETED KNOB (MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID): one client of several is
-    // lost, whenever its session drains, whether or not it ever presents or reads back. Its pid
-    // has to be known before the server starts (the knob is the server's environment), so it is
-    // forked first and held until the server listens; the endpoint is handed to it then.
+    // lost, whenever its session drains, whether or not it ever presents or reads back. It is
+    // forked first and held until the server listens; the endpoint is handed to it then, and the
+    // knob is aimed at it once it is current.
     struct TargetedReport {
         Int32 initialized = 0;
         Int32 lossSeen = 0;
@@ -799,7 +799,7 @@ namespace {
 
     // Draws until its context reports the reset, recovers on a new context, then keeps drawing
     // for half a second - long enough for the knob to be asked again - and must not be lost again.
-    void RunTargetedClient(TargetedReport& r) {
+    void RunTargetedClient(TargetedReport& r, ReadyPipe& ready) {
         Egl egl;
         RecoveryReport bring{};
         if (!egl.Bring(bring)) return Note(r, bring.note);
@@ -807,6 +807,7 @@ namespace {
         if (lost == EGL_NO_CONTEXT || EGL::MakeCurrent(egl.dpy, egl.surface, egl.surface, lost) != EGL_TRUE)
             return Note(r, "the first context did not go current");
         r.initialized = 1;
+        ready.Signal(); // the test aims the knob at this pid now
         r.lossSeen = WaitFor(
                          [] {
                              (void)ClearAndRead(0.0f, 1.0f, 0.0f);
@@ -910,10 +911,19 @@ namespace {
         }
 
         Report Release(const std::string& endpoint) {
-            Report r{};
+            Go(endpoint);
+            return Wait();
+        }
+
+        void Go(const std::string& endpoint) {
             const ssize_t wrote = ::write(go, endpoint.c_str(), endpoint.size());
             (void)wrote;
             ::close(go);
+            go = -1;
+        }
+
+        Report Wait() {
+            Report r{};
             pollfd pfd{report, POLLIN, 0};
             if (::poll(&pfd, 1, 90000) > 0) {
                 if (::read(report, &r, sizeof(r)) != static_cast<ssize_t>(sizeof(r))) {
@@ -1295,13 +1305,16 @@ TEST_F(ClientRecovery, ContextsNamingALostShareContextShareOneNewGroup) {
 // of the same server draws on untouched; the target is lost although it only clears and reads
 // back - no present - recovers on a fresh session, and is not lost again by the same setting.
 TEST_F(ClientRecovery, ThePidTargetedKnobLosesOnlyThatClientAndOnlyOnce) {
-    auto targetBody = [](TargetedReport& r) { RunTargetedClient(r); };
+    ReadyPipe targetReady;
+    auto targetBody = [&targetReady](TargetedReport& r) { RunTargetedClient(r, targetReady); };
     HeldClient<TargetedReport, decltype(targetBody)> target;
     ClientLog targetLog("pid-target");
     ASSERT_TRUE(target.Fork(targetLog.base, targetBody));
+    targetReady.ParentSide();
 
+    DeviceLossKnobFile knob("pid-target");
     ServerProcess server;
-    ::setenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID", std::to_string(target.pid).c_str(), 1);
+    ::setenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID", knob.Value().c_str(), 1);
     const bool launched = LaunchUnixInProcessServer(&server, "pid");
     ::unsetenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID");
     if (!launched) {
@@ -1311,7 +1324,10 @@ TEST_F(ClientRecovery, ThePidTargetedKnobLosesOnlyThatClientAndOnlyOnce) {
 
     ClientLog bystanderLog("pid-bystander");
     const BystanderReport bystander = RunInClient<BystanderReport>(server, bystanderLog.base, &RunBystanderClient);
-    const TargetedReport r = target.Release(server.endpoint);
+    target.Go(server.endpoint);
+    // The loss comes after the target went current, however slowly the machine gets there.
+    if (targetReady.Wait(60000)) knob.Aim(target.pid);
+    const TargetedReport r = target.Wait();
     const std::string diagnostics =
         "server:\n" + server.Log() + "\ntarget:\n" + targetLog.Text() + "\nbystander:\n" + bystanderLog.Text();
 

MobileGL/MG_Test/Wire/CompositorRecoveryTest.cpp

@@ -389,7 +389,7 @@ namespace {
         if (r.note[0] == '\0') std::snprintf(r.note, sizeof(r.note), "%s", text);
     }
 
-    void RunCompositor(int sock, CompositorReport& r) {
+    void RunCompositor(int sock, CompositorReport& r, ReadyPipe& ready) {
         int fd = -1;
         SharedImageMessage image;
         r.received = ReceiveFd(sock, &fd, &image, 30000) ? 1 : 0;
@@ -442,8 +442,10 @@ namespace {
         r.firstBindError = BindImageToNewTexture(stale);
         r.firstPixel = ClearAndRead(0.0f, 1.0f, 0.0f);
         r.firstSwap = EGL::SwapBuffers(dpy, surface);
+        // The frame before the loss is done: the test aims the knob at this pid now.
+        ready.Signal();
 
-        // Compositing until the session is lost (the knob targets this pid).
+        // Compositing until the session is lost.
         const auto until = std::chrono::steady_clock::now() + std::chrono::seconds(10);
         while (std::chrono::steady_clock::now() < until) {
             (void)ClearAndRead(0.0f, 1.0f, 0.0f);
@@ -506,8 +508,7 @@ namespace {
         (void)EGL::Terminate(dpy);
     }
 
-    // A forked client that waits for the server's endpoint before it does anything (the knob names
-    // the compositor's pid, so the clients exist before the server starts).
+    // A forked client that waits for the server's endpoint before it does anything.
     template <class Report>
     struct HeldChild {
         pid_t pid = -1;
@@ -602,23 +603,28 @@ TEST(CompositorRecovery, ACompositorOnTheServerWindowRecoversOnAFreshSessionAndR
         SetClientEnvironment(endpoint, ownerLog.base, /*serverWindow=*/false);
         RunOwner(pair[0], r);
     }));
+    ReadyPipe compositorReady;
     HeldChild<CompositorReport> compositor;
     ASSERT_TRUE(compositor.Fork([&](const std::string& endpoint, CompositorReport& r) {
         ::close(pair[0]);
         SetClientEnvironment(endpoint, compositorLog.base, /*serverWindow=*/true);
-        RunCompositor(pair[1], r);
+        RunCompositor(pair[1], r, compositorReady);
     }));
+    compositorReady.ParentSide();
     ::close(pair[0]);
     ::close(pair[1]);
 
+    DeviceLossKnobFile knob("comprec");
     ServerProcess server;
-    ::setenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID", std::to_string(compositor.pid).c_str(), 1);
+    ::setenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID", knob.Value().c_str(), 1);
     const bool launched = LaunchDisplayServer(&server, "kwin");
     ::unsetenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID");
     ASSERT_TRUE(launched) << server.Log();
 
     owner.Go(server.endpoint);
     compositor.Go(server.endpoint);
+    // The loss comes after the compositor's first frame, however slowly the machine gets there.
+    if (compositorReady.Wait(60000)) knob.Aim(compositor.pid);
     const CompositorReport c = compositor.Wait();
     const OwnerReport o = owner.Wait();
     const std::string diagnostics =

MobileGL/MG_Test/Wire/FatalFamilyTest.cpp

@@ -16,11 +16,14 @@
 #include <flatbuffers/flatbuffers.h>
 #include <gtest/gtest.h>
 
+#include <chrono>
 #include <csignal>
 #include <cstdio>
 #include <cstdlib>
+#include <fstream>
 #include <set>
 #include <string>
+#include <thread>
 
 using namespace MobileGL::MG_Remote;
 using MobileGL::Wire::FatalCode;
@@ -278,3 +281,34 @@ TEST(FatalFunnelSeam, TheDeviceLossKnobFiresOncePerValueAndReArmsOnANewOne) {
         ::testing::ExitedWithCode(0), "");
 }
 #endif
+
+#if !defined(_WIN32)
+// The session-targeted knob takes a pid, or a file that holds one. It asks at most every 100 ms (a
+// thread's first ask only starts the clock) and loses the named client once per value; a "file:"
+// value is read again at every ask, so a test aims a running server by writing the file. In a
+// death-test child, so the clock and the once-per-value latch start fresh.
+TEST(FatalFunnelSeam, TheSessionLossKnobTakesAPidOrAFileThatHoldsOne) {
+    EXPECT_EXIT(
+        {
+            const auto ask = [](unsigned pid) {
+                std::this_thread::sleep_for(std::chrono::milliseconds(100));
+                return MobileGL::MG_Pipe::MGPipeDebugSessionLossDue(pid);
+            };
+            ::setenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID", "4242", 1);
+            if (MobileGL::MG_Pipe::MGPipeDebugSessionLossDue(4242)) std::_Exit(1); // starts the clock
+            if (ask(4243)) std::_Exit(2);                                          // another client
+            if (!ask(4242)) std::_Exit(3);
+            if (ask(4242)) std::_Exit(4); // once per value
+            const std::string path = "/tmp/mgl-loss-knob-unit-" + std::to_string(::getpid());
+            std::remove(path.c_str());
+            ::setenv("MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID", ("file:" + path).c_str(), 1);
+            if (ask(4242)) std::_Exit(5); // no file yet: nobody, and the latch re-arms
+            std::ofstream(path) << 4242;
+            if (!ask(4242)) std::_Exit(6); // the file names it now
+            if (ask(4242)) std::_Exit(7);  // once per value
+            std::remove(path.c_str());
+            std::_Exit(0);
+        },
+        ::testing::ExitedWithCode(0), "");
+}
+#endif

MobileGL/MG_Test/Wire/P12ServerRig.h

@@ -36,6 +36,7 @@
 #include <chrono>
 #include <csignal>
 #include <cstdint>
+#include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <filesystem>
@@ -172,6 +173,68 @@ namespace {
             }
         };
 
+        // A forked child's "I got there" to the test that forked it: the child writes one byte, the
+        // test waits for it. Made before the fork; the test drops its copy of the child's end right
+        // after it, so a child that exits without signalling ends the wait too.
+        class ReadyPipe {
+        public:
+            ReadyPipe() {
+                if (::pipe(m_fds) != 0) m_fds[0] = m_fds[1] = -1;
+            }
+            ~ReadyPipe() {
+                Close(0);
+                Close(1);
+            }
+            ReadyPipe(const ReadyPipe&) = delete;
+            ReadyPipe& operator=(const ReadyPipe&) = delete;
+
+            void Signal() {
+                const char byte = 1;
+                if (m_fds[1] >= 0 && ::write(m_fds[1], &byte, 1) != 1) std::perror("ReadyPipe::Signal");
+                Close(1);
+            }
+            void ParentSide() { Close(1); }
+            bool Wait(int timeoutMs) {
+                pollfd pfd{m_fds[0], POLLIN, 0};
+                char byte = 0;
+                return ::poll(&pfd, 1, timeoutMs) > 0 && ::read(m_fds[0], &byte, 1) == 1;
+            }
+
+        private:
+            void Close(int end) {
+                if (m_fds[end] >= 0) ::close(m_fds[end]);
+                m_fds[end] = -1;
+            }
+            int m_fds[2] = {-1, -1};
+        };
+
+        // The session-targeted device-loss knob aimed through a file (MG_Pipe/PipeSessionFail.h):
+        // a server started with Value() in MOBILEGL_DEBUG_INJECT_DEVICE_LOST_PID loses nobody until
+        // Aim writes the target's pid, so the loss comes after a point the test chose, not after a
+        // delay its target may not keep up with.
+        class DeviceLossKnobFile {
+        public:
+            explicit DeviceLossKnobFile(const std::string& label)
+                : m_path("/tmp/mgl-loss-knob-" + label + "-" + std::to_string(::getpid())) {
+                std::error_code ec;
+                std::filesystem::remove(m_path, ec);
+            }
+            ~DeviceLossKnobFile() {
+                std::error_code ec;
+                std::filesystem::remove(m_path, ec);
+            }
+            std::string Value() const { return "file:" + m_path; }
+            void Aim(pid_t target) const {
+                const std::string staging = m_path + ".new";
+                std::ofstream(staging) << target;
+                std::error_code ec;
+                std::filesystem::rename(staging, m_path, ec); // whole, never half-written
+            }
+
+        private:
+            std::string m_path;
+        };
+
         // The exec'd `--serve` supervisor (the fork-per-session shape), on a fresh loopback port.
         inline bool LaunchSupervisor(const std::string& label, ServerProcess* server) {
             server->logBase = "/tmp/mgl-p12-" + label + "-" + std::to_string(::getpid()) + ".log";

scripts/data/structure_baseline.json

@@ -1,5 +1,5 @@
 {
-  "file_excess_lines": 21854,
+  "file_excess_lines": 21771,
   "function_excess_lines": 15516,
   "layering_violations": 63
 }