[MG_Util, MG_Test, MG_IntegrationTest, CI] (P6): one log per role, and every reader moved with it

The spawn work had the two roles append to one MOBILEGL_LOG_FILE_PATH under
O_APPEND. It was correct - two processes at one offset would corrupt the file,
O_APPEND made each write atomic - but it put two sessions' lines in one place
and made every per-side assertion a search. This splits the sink: the client
role writes <base>.client.log and the server role writes <base>.server.log.

THE BASE NAME NAMES NO FILE ANY MORE, AND THAT IS THE POINT. Leaving the
client at the old unsuffixed path would have been the compatible choice and the
wrong one: a reader that was not updated would go on finding a file, go on
passing, and go on seeing only half the session - and a refusal census that
reports a confident zero for the half it cannot see is worse than one that
fails. With both names moved, an un-updated reader gets "file not found" and
says so. There were 34 such readers; every one is moved here.

THE ROLE IS PER-THREAD, not per-process, because under inproc both roles are
threads of one process. Log.cpp keys the sink on a thread-local role: the apply
thread declares itself server (ServerLoop.cpp), a spawned image is server for
its whole life (MOBILEGL_IPC_ROLE=server, read once), and everything else is
the client. The naming rule (RoleLogPath) and the two whole-log operations
(ReadRoleLogs / TruncateRoleLogs) are EXPORTED from the library, so no reader
reimplements the derivation - a second copy is the copy that falls behind.

Readers, by how they read:
 - unit mains set MOBILEGL_LOG_FILE_PATH and read a role file: they keep the
   base in the env and derive the client path from the library helper.
 - the death-test child runners (ReadLog) now concatenate BOTH roles: a death
   test asserts the CHILD said something, and the applier's refusals - the very
   diagnostics these check - are written under the server role.
 - the offset-based Emit readers derive the client path once after setenv.
 - PipeStatsWindow reads both, because draws/vbs increment in the backend's
   PrepareForDraw - the apply thread, the server role - so the summary line is
   in the server file even under inproc.
 - run_trace_case.cmake's split census reads both files; the client keeps the
   transport markers.
 - split_log_paths.py (and its self-test, and the control stub) derive both
   role files from the declared base.
 - the Android script pulls both role files off the device and concatenates
   them, so every downstream check is unchanged - and it judges a role file by
   CONTENT, because `run-as test -f` through `exec-out` returned 0 for a missing
   file on the device and `exec-out` folds cat's own not-found line into stdout.
   The monolith arm legitimately has no server file, and that is a silent skip
   rather than a one-line server log.

Guarded so a pull build is untouched: RoleLogPath and the role sink are wholly
inside MOBILEGL_BUILD_DISAGGREGATED; a non-disaggregated build writes the base
path exactly as before. G1 re-measured - 0 symbol diff, .text unchanged at
0xa52203.

Verified on device 2f7cbe2e: monolith writes only the client log (no phantom
server log), inproc and spawn each write both and pass their transport proofs
(spawn: server role in pid 16125). Unit 2353 (one pre-existing environmental
failure), integration-split 186/186, integration-spawn 102/102, retrace 6/6,
control smoke 12/12, split-log-paths self-test 9/9.
This commit is contained in:
2026-09-21 21:26:05 -04:00
parent 15f5b54bbd
commit 5fb54f03fe
38 changed files with 524 additions and 146 deletions
@@ -37,12 +37,34 @@
namespace MGITest::PipeStatsWindow {
// The lane's private log path, or empty when the lane configured none.
inline std::string LibraryLogPath() {
const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH");
return (path != nullptr && *path != '\0') ? std::string(path) : std::string();
// P6: MOBILEGL_LOG_FILE_PATH IS A BASE NAME, NOT A FILE. The library writes one log per
// ROLE - `<stem>.client<ext>` and `<stem>.server<ext>` - because under inproc both roles are
// threads of one process and a single file made every per-side assertion a search.
//
// THE SUFFIX IS DERIVED HERE AND NOWHERE ELSE, matching MG_Util/Debug/Log.cpp's
// RoleLogPathFor. Two copies of a naming rule is one copy too many: the one that is not
// updated opens a file that does not exist and reports "the library never logged", which
// reads as a product failure rather than as a stale path.
inline std::string RoleLogPath(const char* roleSuffix) {
const char* base = std::getenv("MOBILEGL_LOG_FILE_PATH");
if (base == nullptr || *base == '\0') return {};
std::string path(base);
const std::string::size_type slash = path.find_last_of("/\\");
const std::string::size_type dot = path.find_last_of('.');
if (dot == std::string::npos || (slash != std::string::npos && dot < slash)) {
return path + "." + roleSuffix;
}
return path.substr(0, dot) + "." + roleSuffix + path.substr(dot);
}
// The lane's private CLIENT log path, or empty when the lane configured none. Every existing
// caller wants this one: they read markers the CLIENT emits - the transport resolution line,
// PipeStats summaries, the arming diagnostics.
inline std::string LibraryLogPath() { return RoleLogPath("client"); }
// The server role's half, for a caller that wants a line only the applier writes.
inline std::string ServerLibraryLogPath() { return RoleLogPath("server"); }
inline std::string ReadWholeFile(const std::string& path) {
if (path.empty()) return {};
std::ifstream file(path, std::ios::binary);
@@ -70,7 +92,15 @@ namespace MGITest::PipeStatsWindow {
return window;
}
inline Window LastFromLaneLog() { return Last(ReadWholeFile(LibraryLogPath())); }
// BOTH ROLES, and the server's is where the numbers are. The `draws` / `vbs` counters
// increment in the backend's PrepareForDraw, which runs on the APPLY THREAD - the server
// role - so under inproc split the summary line lands in the server's log. Reading only the
// client's would find the "counters ON" banner (client-side) but no window, which reads as
// "the stats channel never reached the process". Concatenated, Last() takes whichever role
// emitted the final summary.
inline Window LastFromLaneLog() {
return Last(ReadWholeFile(LibraryLogPath()) + ReadWholeFile(ServerLibraryLogPath()));
}
// One counter out of that line, by its short name ("mpr", "draws", "csom"), or -1 when the
// line does not carry it. The search includes the SEPARATOR before the name and the `=` after
@@ -9,6 +9,38 @@ import sys
import xml.etree.ElementTree as ET
# P6: THE SINK WRITES ONE FILE PER ROLE, so MOBILEGL_LOG_FILE_PATH is a BASE NAME and no file
# of that literal name exists. The derivation matches MG_Util/Debug/Log.cpp's RoleLogPath: the
# role goes before the extension. A census that read the base would find nothing and report a
# confident zero for every marker the run actually raised - which under split is most of them,
# because the applier's refusals are on the server side.
def role_paths(base):
text = str(base)
slash = max(text.rfind("/"), text.rfind("\\"))
dot = text.rfind(".")
out = []
for role in ("client", "server"):
if dot == -1 or dot < slash:
out.append(text + "." + role)
else:
out.append(text[:dot] + "." + role + text[dot:])
return out
def read_role_logs(base):
"""Concatenated text of every role file that exists for this base, or '' if none do."""
chunks = []
for path in role_paths(base):
p = Path(path)
if p.is_file():
chunks.append(p.read_text(errors="replace"))
return "".join(chunks)
def any_role_file(base):
return any(Path(p).is_file() for p in role_paths(base))
def paths(document):
owners = {}
split = {}
@@ -89,10 +121,10 @@ def classify_markers(logs):
scanned = []
for name in sorted(logs):
path = logs[name]
if not Path(path).is_file():
if not any_role_file(path):
continue
scanned.append(name)
text = Path(path).read_text(errors="replace")
text = read_role_logs(path)
for tag, field, verb, why in MARKER_RE.findall(text):
pair = f"{field}@{verb}"
if tag == "Fatal":
@@ -204,12 +236,12 @@ def expect_fatal(document, junit_path, expected_path):
if failed:
problems.append(f"{name} is RED and declares no private log")
continue # metadata/monolith-arm entries have no marker channel
if not Path(path).is_file():
problems.append(f"{name}: declared private log {path} does not exist")
if not any_role_file(path):
problems.append(f"{name}: declared private log {path} has no role file (.client/.server)")
continue
scanned += 1
entry_fatal = set()
for tag, field, verb, why in MARKER_RE.findall(Path(path).read_text(errors="replace")):
for tag, field, verb, why in MARKER_RE.findall(read_role_logs(path)):
pair = f"{field}@{verb}"
if tag == "Fatal":
entry_fatal.add(pair)
@@ -380,7 +412,7 @@ def main():
missing = []
label = sys.argv[5] if len(sys.argv) > 5 else ""
for name, path in selected.items():
if Path(path).is_file() and re.search(sys.argv[4], Path(path).read_text(errors="replace")):
if any_role_file(path) and re.search(sys.argv[4], read_role_logs(path)):
print(f"private-log evidence: {name}: {path}")
else:
missing.append(f"{name} ({path})")
@@ -31,12 +31,15 @@ class ResultAccountingTest(unittest.TestCase):
self.temp.cleanup()
def discovery(self, names):
# P6: the ENV path is a BASE NAME and the library writes <base>.client.log /
# <base>.server.log. The fixture writes the CLIENT role file - what a real run leaves on
# disk - while the XML carries the base, exactly as the lane does.
tests = []
for i, name in enumerate(names):
log = self.root / (str(i) + ".log")
log.write_text("")
base = self.root / (str(i) + ".log")
(self.root / (str(i) + ".client.log")).write_text("")
tests.append({"name": name, "properties": [{"name": "ENVIRONMENT", "value": [
"MOBILEGL_LOG_FILE_PATH=" + str(log)]}]})
"MOBILEGL_LOG_FILE_PATH=" + str(base)]}]})
return {"tests": tests}
def results(self, rows):
@@ -97,7 +100,7 @@ class ResultAccountingTest(unittest.TestCase):
def test_admitted_or_fatal_on_passed_case_is_not_hidden(self):
for marker in ['Admitted{UnmigratedPipeInput, "GetProgramObject@Clear"}',
'Fatal{UnmigratedPipeInput, "GetProgramObject@Clear"}']:
(self.root / "0.log").write_text(marker)
(self.root / "0.client.log").write_text(marker)
with self.assertRaises(ValueError):
self.census()
@@ -71,6 +71,7 @@
#include <string>
#include <vector>
#include "../Harness/PipeStatsWindow.h"
#include "../Harness/HeadlessGL.h"
#include "../Harness/ScenarioFixture.h"
@@ -124,8 +125,9 @@ void main() { oColor = vec4(0.0, 1.0, 0.0, 1.0); }
}
std::string LibraryLogPath() {
const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH");
return (path != nullptr && *path != '\0') ? std::string(path) : std::string();
// P6: the path is a BASE NAME and the library writes one log per role; PipeStatsWindow
// derives the suffix, so the rule lives in one place.
return MGITest::PipeStatsWindow::LibraryLogPath();
}
std::string ReadWholeFile(const std::string& path) {
@@ -25,6 +25,7 @@
// half of that control is TheDirectApplierResetCallOnTheGLThreadIsRoleViolation below, and
// the guard's two non-Fatal arms are pinned in PipeWireCodecTest's CtWireFatals suite.
#include "../Harness/PipeStatsWindow.h"
#include "../Harness/ScenarioFixture.h"
#include "../Harness/SplitRuntimePeek.h"
@@ -292,7 +293,10 @@ namespace {
// same); the NAME is asserted from the log below, which the child truncated and
// wrote before it died.
EXPECT_EXIT(MobileGL::MG_Pipe::MGPipeApplierReset(), ::testing::KilledBySignal(SIGABRT), ".*");
if (const char* logPath = std::getenv("MOBILEGL_LOG_FILE_PATH"); logPath != nullptr) {
// THE CLIENT'S LOG: MGPipeApplierReset is called here, on the GL thread, so the guard's
// line is written under the client role. P6 splits the lane's log per role and the
// suffix rule lives in PipeStatsWindow.
if (const std::string logPath = MGITest::PipeStatsWindow::LibraryLogPath(); !logPath.empty()) {
std::ifstream in(logPath, std::ios::binary);
std::ostringstream log;
log << in.rdbuf();
@@ -57,6 +57,7 @@
#include <iterator>
#include <string>
#include "../Harness/PipeStatsWindow.h"
#include "../Harness/HeadlessGL.h"
#include "../Harness/ScenarioFixture.h"
@@ -105,9 +106,9 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); }
protected:
// The library log this process is writing, or an empty path when none was configured.
static std::filesystem::path LibraryLogPath() {
const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH");
return (path != nullptr && *path != '\0') ? std::filesystem::path(path)
: std::filesystem::path();
// P6: the path is a BASE NAME and the library writes one log per role; PipeStatsWindow
// derives the suffix, so the rule lives in one place.
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
static std::uintmax_t LibraryLogSize() {
@@ -37,6 +37,7 @@
#include <utility>
#include <vector>
#include "../Harness/PipeStatsWindow.h"
#include "../Harness/HeadlessGL.h"
#include "../Harness/ScenarioFixture.h"
@@ -305,9 +306,9 @@ void main()
// is appended to by every process in the lane, and only bytes appended after the
// snapshot may satisfy an assertion.
static std::filesystem::path LibraryLogPath() {
const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH");
return (path != nullptr && *path != '\0') ? std::filesystem::path(path)
: std::filesystem::path();
// P6: the path is a BASE NAME and the library writes one log per role; PipeStatsWindow
// derives the suffix, so the rule lives in one place.
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
static std::uintmax_t LibraryLogSize() {
@@ -54,6 +54,7 @@
#include <string>
#include <vector>
#include "../Harness/PipeStatsWindow.h"
#include "../Harness/HeadlessGL.h"
#include "../Harness/ScenarioFixture.h"
@@ -109,9 +110,9 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); }
}
std::filesystem::path LibraryLogPath() {
const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH");
return (path != nullptr && *path != '\0') ? std::filesystem::path(path)
: std::filesystem::path();
// P6: the path is a BASE NAME and the library writes one log per role; PipeStatsWindow
// derives the suffix, so the rule lives in one place.
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
// Where the child is told to write ITS log. Empty when the lane configured no log path at
@@ -44,6 +44,7 @@
#include <utility>
#include <vector>
#include "../Harness/PipeStatsWindow.h"
#include "../Harness/HeadlessGL.h"
#include "../Harness/ScenarioFixture.h"
@@ -250,9 +251,9 @@ void main() {
const std::string& BuildLog() const { return m_buildLog; }
static std::filesystem::path LibraryLogPath() {
const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH");
return (path != nullptr && *path != '\0') ? std::filesystem::path(path)
: std::filesystem::path();
// P6: the path is a BASE NAME and the library writes one log per role; PipeStatsWindow
// derives the suffix, so the rule lives in one place.
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
static std::uintmax_t LibraryLogSize() {
@@ -49,6 +49,7 @@
#include <string>
#include <vector>
#include "../Harness/PipeStatsWindow.h"
#include "../Harness/HeadlessGL.h"
#include "../Harness/ScenarioFixture.h"
@@ -354,9 +355,9 @@ void main()
// configured. MOBILEGL_LOG_FILE_PATH is read at log-init, before anything this
// fixture can reach, so the ctest entry sets it and this only reads it back.
static std::filesystem::path LibraryLogPath() {
const char* path = std::getenv("MOBILEGL_LOG_FILE_PATH");
return (path != nullptr && *path != '\0') ? std::filesystem::path(path)
: std::filesystem::path();
// P6: the path is a BASE NAME and the library writes one log per role; PipeStatsWindow
// derives the suffix, so the rule lives in one place.
return std::filesystem::path(MGITest::PipeStatsWindow::LibraryLogPath());
}
// How many bytes the library log already holds. Everything this fixture asserts on