diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ba4aa699..34b42ac1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1169,9 +1169,12 @@ jobs: - name: The split lane really resolved the transport working-directory: build-split run: | - log=MobileGL/MG_IntegrationTest/persistent-map-arm-split-DirectGLES.log + # P6: the sink writes one file per ROLE from the base MOBILEGL_LOG_FILE_PATH; the + # transport-resolution line is ConfigLoader's, on the CLIENT thread, so it is in the + # .client half. The base name itself names no file now. + log=MobileGL/MG_IntegrationTest/persistent-map-arm-split-DirectGLES.client.log if [ ! -f "${log}" ]; then - echo "::error::${log} does not exist: the DirectGLES.Split.PersistentMapArm. entry never ran, so nothing in this job establishes that MOBILEGL_TRANSPORT ever resolved to inproc in a live process" + echo "::error::${log} does not exist: the DirectGLES.Split.PersistentMapArm. entry never ran (or the per-role split renamed it), so nothing in this job establishes that MOBILEGL_TRANSPORT ever resolved to inproc in a live process" exit 1 fi # THE DISTINCTIVE PART OF THE INFO LINE, not the bare KEY=VALUE (review finding M-5): @@ -2649,7 +2652,7 @@ jobs: working-directory: build-retrace/tools/trace_replay env: CONTROL_TMPDIR: ${{ runner.temp }} - LIBRARY_LOG: ${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.log + LIBRARY_LOG: ${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.client.log FROZEN_LIBRARY: ${{ github.workspace }}/build-linux/libMobileGL.so # The control has to run the arm it is a control FOR. Hard-coded inproc, it would go on # proving something about the other arm while spawn ran unguarded. @@ -2667,13 +2670,22 @@ jobs: if: always() working-directory: build-retrace/tools/trace_replay run: | - log="${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.log" + # BOTH ROLES, because under spawn every applier refusal is raised in the SERVER's + # process and a census reading only the client's file reports a confident zero for the + # half it cannot see. + log="${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.client.log" + server_log="${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.server.log" if [ ! -f "${log}" ]; then echo "no ${log} - the replay wrote no library log" exit 0 fi - echo "Fatal{ lines: $(grep -c 'Fatal{' "${log}" || true)" - grep -o 'Fatal{[A-Za-z]*, "[^"]*"' "${log}" | sort | uniq -c | sort -rn | head -20 || true + census_files="${log}" + if [ -f "${server_log}" ]; then census_files="${census_files} ${server_log}"; fi + echo "census over: ${census_files}" + echo "Fatal{ lines: $(cat ${census_files} | grep -c 'Fatal{' || true)" + cat ${census_files} | grep -o 'Fatal{[A-Za-z]*, "[^"]*"' | sort | uniq -c | sort -rn | head -20 || true + # The transport line is the CLIENT's: ConfigLoader resolves it there, and under spawn + # the server states its own separately. grep -m1 "MOBILEGL_TRANSPORT=" "${log}" || echo "no transport line in ${log}" - name: Upload core dumps diff --git a/MobileGL/MG_IntegrationTest/Harness/PipeStatsWindow.h b/MobileGL/MG_IntegrationTest/Harness/PipeStatsWindow.h index 02e86746..6d94c1a6 100644 --- a/MobileGL/MG_IntegrationTest/Harness/PipeStatsWindow.h +++ b/MobileGL/MG_IntegrationTest/Harness/PipeStatsWindow.h @@ -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 - `.client` and `.server` - 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 diff --git a/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py b/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py index 8969140c..acf7e277 100644 --- a/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py +++ b/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py @@ -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})") diff --git a/MobileGL/MG_IntegrationTest/Harness/test_split_log_paths.py b/MobileGL/MG_IntegrationTest/Harness/test_split_log_paths.py index 46a086f6..b416685c 100644 --- a/MobileGL/MG_IntegrationTest/Harness/test_split_log_paths.py +++ b/MobileGL/MG_IntegrationTest/Harness/test_split_log_paths.py @@ -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 .client.log / + # .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() diff --git a/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp index b13f09a7..51b2a457 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/CsoContentAddressingScenario.cpp @@ -71,6 +71,7 @@ #include #include +#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) { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp index f2aa2aaf..52761613 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp @@ -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(); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp index c7dca253..ed834b65 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp @@ -57,6 +57,7 @@ #include #include +#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() { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PointSizeDemotionScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PointSizeDemotionScenario.cpp index 3899f0b8..f1c31c98 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PointSizeDemotionScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PointSizeDemotionScenario.cpp @@ -37,6 +37,7 @@ #include #include +#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() { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp index d6b1568e..acebdcb3 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp @@ -54,6 +54,7 @@ #include #include +#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 diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PrimitivesGeneratedNoXfbScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PrimitivesGeneratedNoXfbScenario.cpp index 3b43be66..b13e2bac 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PrimitivesGeneratedNoXfbScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PrimitivesGeneratedNoXfbScenario.cpp @@ -44,6 +44,7 @@ #include #include +#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() { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/UnlocatedIoBlockScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/UnlocatedIoBlockScenario.cpp index 0da0321e..252a8a40 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/UnlocatedIoBlockScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/UnlocatedIoBlockScenario.cpp @@ -49,6 +49,7 @@ #include #include +#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 diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index a021c9ef..8f08db7d 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -343,6 +343,10 @@ namespace MobileGL::MG_Remote::Server { // declaration: it is an identity, not a handshake, and this thread is the only writer. Detail::g_applyThreadKey.store(Detail::CurrentThreadKey(), std::memory_order_relaxed); NameThisThread("mgl-srv-apply"); + // P6: THIS THREAD IS THE SERVER, and saying so is what sends its lines to the server's + // own log. Under spawn the process role already answers this; under INPROC it is the + // only thing that can, because the client role is another thread of this same process. + MG_Util::Debug::SetThreadLogRole(MG_Util::Debug::LogRole::Server); Bool recognised = true; const char* raw = AffinityStringFromConfig(); diff --git a/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp b/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp index 3fa4729e..091f324b 100644 --- a/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp +++ b/MobileGL/MG_Test/Pipe/CompositeResolverTest.cpp @@ -30,6 +30,7 @@ // pull and the push trees. #include +#include #include #include @@ -776,6 +777,13 @@ int main(int argc, char** argv) { #else setenv("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str(), 1); #endif + // P6: MOBILEGL_LOG_FILE_PATH is a BASE NAME and the library writes one file per role. These + // cases read the log by OFFSET (a single growing file), and every marker they assert is + // raised by the encoder on THIS thread - the client role. So g_logPath, which is the read + // path from here on, becomes the client-derived name; the env keeps the base. The rule is + // the library's own, not a copy. + g_logPath = MobileGL::MG_Util::Debug::RoleLogPath(g_logPath.c_str(), + MobileGL::MG_Util::Debug::LogRole::Client); #if MOBILEGL_PIPE_PUSH MobileGL::Initialize(); #endif diff --git a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp index 28d6e8a3..fd706946 100644 --- a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp +++ b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp @@ -27,6 +27,7 @@ // logs. #include +#include #include #include @@ -65,10 +66,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } long ProcessId() { diff --git a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp index 1988b6a5..94e5d6a8 100644 --- a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp @@ -37,6 +37,7 @@ // stays name-for-name identical between the pull and the push trees. #include +#include #include #include @@ -92,10 +93,10 @@ namespace { #if MOBILEGL_PIPE_PUSH std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } // A fresh applier per case, BOTH SCOPES, and it takes both because there are two: a reset diff --git a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp index 71bfa31e..b7b11bf9 100644 --- a/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ImageEmitTest.cpp @@ -24,6 +24,7 @@ // pull and the push trees. #include +#include #include #include @@ -698,6 +699,13 @@ int main(int argc, char** argv) { #else setenv("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str(), 1); #endif + // P6: MOBILEGL_LOG_FILE_PATH is a BASE NAME and the library writes one file per role. These + // cases read the log by OFFSET (a single growing file), and every marker they assert is + // raised by the encoder on THIS thread - the client role. So g_logPath, which is the read + // path from here on, becomes the client-derived name; the env keeps the base. The rule is + // the library's own, not a copy. + g_logPath = MobileGL::MG_Util::Debug::RoleLogPath(g_logPath.c_str(), + MobileGL::MG_Util::Debug::LogRole::Client); #if MOBILEGL_PIPE_PUSH MobileGL::Initialize(); #endif diff --git a/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp b/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp index 4f42aa97..6be9bc29 100644 --- a/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp @@ -18,6 +18,7 @@ // line land in this process's read. A forked child inherits its parent's path on purpose. #include +#include #include #include @@ -53,10 +54,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } long ProcessId() { diff --git a/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp b/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp index a0ff99a5..42adb464 100644 --- a/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ProgramEmitTest.cpp @@ -29,6 +29,7 @@ // pull and the push trees. #include +#include #include #include @@ -1017,6 +1018,13 @@ int main(int argc, char** argv) { #else setenv("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str(), 1); #endif + // P6: MOBILEGL_LOG_FILE_PATH is a BASE NAME and the library writes one file per role. These + // cases read the log by OFFSET (a single growing file), and every marker they assert is + // raised by the encoder on THIS thread - the client role. So g_logPath, which is the read + // path from here on, becomes the client-derived name; the env keeps the base. The rule is + // the library's own, not a copy. + g_logPath = MobileGL::MG_Util::Debug::RoleLogPath(g_logPath.c_str(), + MobileGL::MG_Util::Debug::LogRole::Client); #if MOBILEGL_PIPE_PUSH MobileGL::Initialize(); #endif diff --git a/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp b/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp index 4b810286..286c38d7 100644 --- a/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp +++ b/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp @@ -36,6 +36,7 @@ // rather than a vanishing test - and the four names are the SAME four in every build, which // is what keeps `ctest -N` name-for-name identical between the pull and the push tree. #include +#include #include #include @@ -79,10 +80,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } long ProcessId() { diff --git a/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp index 7da2d133..cff3a4f9 100644 --- a/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ResourceEmitTest.cpp @@ -34,6 +34,7 @@ // between the pull and the push trees. #include +#include #include #include @@ -95,10 +96,10 @@ namespace { } std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } // The op table is INSTALLED BY A BACKEND, at its own bring-up, and uninstalled at its diff --git a/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp b/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp index b0a03f5e..873b2092 100644 --- a/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/SamplerEmitTest.cpp @@ -33,6 +33,7 @@ // pull and the push trees. #include +#include #include #include @@ -1150,6 +1151,13 @@ int main(int argc, char** argv) { #else setenv("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str(), 1); #endif + // P6: MOBILEGL_LOG_FILE_PATH is a BASE NAME and the library writes one file per role. These + // cases read the log by OFFSET (a single growing file), and every marker they assert is + // raised by the encoder on THIS thread - the client role. So g_logPath, which is the read + // path from here on, becomes the client-derived name; the env keeps the base. The rule is + // the library's own, not a copy. + g_logPath = MobileGL::MG_Util::Debug::RoleLogPath(g_logPath.c_str(), + MobileGL::MG_Util::Debug::LogRole::Client); #if MOBILEGL_PIPE_PUSH // ONE process-wide context for the whole suite, because half these cases need a really // linked program and glslang lives behind this call. Each case uses GL names of its own. diff --git a/MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp index afa6132c..94750c37 100644 --- a/MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/ShaderBufferEmitTest.cpp @@ -29,6 +29,7 @@ // `ctest -N` stays name-for-name identical between the pull and the push trees (G2/G14). #include +#include #include #include @@ -498,6 +499,13 @@ int main(int argc, char** argv) { #else setenv("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str(), 1); #endif + // P6: MOBILEGL_LOG_FILE_PATH is a BASE NAME and the library writes one file per role. These + // cases read the log by OFFSET (a single growing file), and every marker they assert is + // raised by the encoder on THIS thread - the client role. So g_logPath, which is the read + // path from here on, becomes the client-derived name; the env keeps the base. The rule is + // the library's own, not a copy. + g_logPath = MobileGL::MG_Util::Debug::RoleLogPath(g_logPath.c_str(), + MobileGL::MG_Util::Debug::LogRole::Client); #if MOBILEGL_PIPE_PUSH MobileGL::Initialize(); #endif diff --git a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp index 46dff10e..a815c823 100644 --- a/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/TextureEmitTest.cpp @@ -37,6 +37,7 @@ // stays name-for-name identical between the pull and the push trees. #include +#include #include #include @@ -99,10 +100,10 @@ namespace { #if MOBILEGL_PIPE_PUSH std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } // A fresh applier per case, BOTH SCOPES, and it takes both because there are two: a reset diff --git a/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp b/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp index e1ac461f..509767f5 100644 --- a/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/VertexInputEmitTest.cpp @@ -33,6 +33,7 @@ // stays name-for-name identical between the pull and the push trees. #include +#include #include #include diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 441d8898..b2028965 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -2113,7 +2113,16 @@ TEST(LogSanity, UsesEnvOverrideForFilePath) { UnsetEnvVar("MOBILEGL_LOG_FILE_PATH"); { - std::ifstream logFile(logPath); + // P6: the sink writes ONE FILE PER ROLE, so the base name is not itself a file. This + // process logs on the main thread - the client role - so the line landed in the + // client-derived path. In a pull build there is no split and the base IS the file. +#if MOBILEGL_BUILD_DISAGGREGATED + const fs::path readPath = MobileGL::MG_Util::Debug::RoleLogPath( + logPath.string().c_str(), MobileGL::MG_Util::Debug::LogRole::Client); +#else + const fs::path readPath = logPath; +#endif + std::ifstream logFile(readPath); ASSERT_TRUE(logFile.good()); const std::string contents((std::istreambuf_iterator(logFile)), std::istreambuf_iterator()); @@ -2121,6 +2130,10 @@ TEST(LogSanity, UsesEnvOverrideForFilePath) { } fs::remove(logPath); +#if MOBILEGL_BUILD_DISAGGREGATED + fs::remove(MobileGL::MG_Util::Debug::RoleLogPath( + logPath.string().c_str(), MobileGL::MG_Util::Debug::LogRole::Client)); +#endif } // ---- Pure-state entry points: glHint / glPointParameter* / glPixelStoref / glGetDoublev ----------- @@ -3549,11 +3562,19 @@ namespace { ScopedLogFileRedirect& operator=(const ScopedLogFileRedirect&) = delete; // Everything written so far. Closes the log first so the last line is on disk. + // + // P6: BOTH ROLES, because "everything written" now spans two files - the sink writes one + // per role - and a caller of a general "give me the log" helper cannot know which role + // wrote the line it wants. In a pull build there is no split and m_path is the file. std::string Contents() const { MobileGL::MG_Util::Debug::Close(); +#if MOBILEGL_BUILD_DISAGGREGATED + return MobileGL::MG_Util::Debug::ReadRoleLogs(m_path.string().c_str()); +#else std::ifstream logFile(m_path); if (!logFile.good()) return {}; return std::string(std::istreambuf_iterator(logFile), std::istreambuf_iterator()); +#endif } private: diff --git a/MobileGL/MG_Test/Util/LogLevelTest.cpp b/MobileGL/MG_Test/Util/LogLevelTest.cpp index eae11ff3..dd59cdc5 100644 --- a/MobileGL/MG_Test/Util/LogLevelTest.cpp +++ b/MobileGL/MG_Test/Util/LogLevelTest.cpp @@ -198,6 +198,13 @@ int main(int argc, char** argv) { #else setenv("MOBILEGL_LOG_FILE_PATH", g_logPath.c_str(), 1); #endif + // P6: the sink writes one file per role and the env is a BASE NAME. This suite emits on the + // main thread (client role), so the read path is the client-derived one; a pull build has + // no split and keeps the base. The env keeps the base either way. +#if MOBILEGL_BUILD_DISAGGREGATED + g_logPath = MobileGL::MG_Util::Debug::RoleLogPath(g_logPath.c_str(), + MobileGL::MG_Util::Debug::LogRole::Client); +#endif ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 7bddd0eb..a14f21fe 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -22,6 +22,7 @@ // comes back through the recording ReplySink on the record's own seq (R-3/R-5). #include +#include #include #include @@ -75,10 +76,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } long ProcessId() { diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index 4710af8b..59ba97d4 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -18,6 +18,7 @@ // assert ITS OWN failure string (R-16) instead of asserting that something, somewhere, died. #include +#include #include #include @@ -78,12 +79,17 @@ namespace { std::string g_logPath; + // BOTH ROLES' LOGS. P6 gives the client and the server role a file each, and a death test + // asserts that the CHILD said something - which role's thread said it is not what these + // cases are about. The refusals raised on the apply thread (RefuseFromApplyThread and every + // guard that rides it) are written under the SERVER role by construction, so a reader that + // took only g_logPath would report "the child aborted, but said nothing" for the very + // diagnostics it exists to check. std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - if (!in) return {}; - std::ostringstream out; - out << in.rdbuf(); - return out.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } int ProcessId() { @@ -146,7 +152,7 @@ namespace { // child's output. That is how the second death case in this file came to see the first // one's slot name and assert on it: a control reading another control's message, which // is one of the three shapes R-16 was written after. - { std::ofstream truncate(g_logPath, std::ios::trunc | std::ios::binary); } + MobileGL::MG_Util::Debug::TruncateRoleLogs(g_logPath.c_str()); std::fflush(nullptr); const pid_t pid = ::fork(); if (pid < 0) return result; diff --git a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp index 8f0329f2..58b9d78c 100644 --- a/MobileGL/MG_Test/Wire/ServerLoopTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerLoopTest.cpp @@ -29,6 +29,7 @@ // exists to test: bytes that were copied survive the source being overwritten with 0xDD. #include +#include #include #include #include @@ -102,9 +103,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - if (!in) return {}; - return std::string(std::istreambuf_iterator(in), std::istreambuf_iterator()); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } unsigned ProcessId() { diff --git a/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp b/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp index 8411b564..db518ce6 100644 --- a/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp +++ b/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp @@ -23,6 +23,7 @@ // EVERY CASE BELOW CARRIES ITS RED-ONCE LINE, and each of those perturbations was run. #include +#include #include #include @@ -66,10 +67,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - std::ostringstream ss; - ss << in.rdbuf(); - return ss.str(); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } long ProcessId() { diff --git a/MobileGL/MG_Test/Wire/StagedTextureStoreTest.cpp b/MobileGL/MG_Test/Wire/StagedTextureStoreTest.cpp index 780611e4..19298add 100644 --- a/MobileGL/MG_Test/Wire/StagedTextureStoreTest.cpp +++ b/MobileGL/MG_Test/Wire/StagedTextureStoreTest.cpp @@ -22,6 +22,7 @@ // staged bytes" a checked fact rather than a design intention. #include +#include #include #include #include @@ -51,9 +52,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - if (!in) return {}; - return std::string(std::istreambuf_iterator(in), std::istreambuf_iterator()); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } unsigned ProcessId() { diff --git a/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp b/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp index 528d171c..fbe0e75b 100644 --- a/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp +++ b/MobileGL/MG_Test/Wire/SurfaceControlFrameTest.cpp @@ -24,6 +24,7 @@ // ServerLoopEglTest fixture, which now drives every forwarder through the frame channel). #include +#include #include #include #include @@ -63,9 +64,10 @@ namespace { std::string g_logPath; std::string ReadLog() { - std::ifstream in(g_logPath, std::ios::binary); - if (!in) return {}; - return std::string(std::istreambuf_iterator(in), std::istreambuf_iterator()); + // BOTH ROLES' LOGS (P6). A death test asserts that the CHILD said something; which + // role's thread said it is not what these cases are about, and refusals raised on the + // apply thread are written under the SERVER role by construction. + return MobileGL::MG_Util::Debug::ReadRoleLogs(g_logPath.c_str()); } unsigned ProcessId() { diff --git a/MobileGL/MG_Util/Debug/Log.cpp b/MobileGL/MG_Util/Debug/Log.cpp index 57ecf710..a5c59b5d 100644 --- a/MobileGL/MG_Util/Debug/Log.cpp +++ b/MobileGL/MG_Util/Debug/Log.cpp @@ -14,6 +14,88 @@ namespace MobileGL { namespace MG_Util::Debug { static FILE* s_logFile = nullptr; +#if MOBILEGL_BUILD_DISAGGREGATED + // The server role's own sink. Opened lazily, on the first line a server-role thread + // writes, so a process that never plays the role never creates the file. + static FILE* s_serverLogFile = nullptr; + + // THE PROCESS DEFAULT, read once. A spawned server image is the server on every thread - + // there is no other role in it - and MOBILEGL_IPC_ROLE is set by the launcher before + // exec, so this is answerable before any MobileGL code runs. Under inproc it is false + // and the apply thread opts in per-thread. + static bool ProcessIsSpawnedServer() { + static const bool value = [] { + const char* role = std::getenv("MOBILEGL_IPC_ROLE"); + return role != nullptr && std::strcmp(role, "server") == 0; + }(); + return value; + } + + // Per THREAD, not per process: under inproc the server role is the mgl-srv-apply thread + // and the GL thread is the client, in one address space. -1 means "not asked yet", which + // is resolved from the process default on first use. + static thread_local int t_logRole = -1; + + static bool ThreadIsServerRole() { + if (t_logRole < 0) { + t_logRole = ProcessIsSpawnedServer() ? 1 : 0; + } + return t_logRole == 1; + } + + void SetThreadLogRole(LogRole role) { t_logRole = role == LogRole::Server ? 1 : 0; } + + std::string ReadRoleLogs(const char* basePath) { + std::string all; + for (const LogRole role : {LogRole::Client, LogRole::Server}) { + const std::string path = RoleLogPath(basePath, role); + if (FILE* file = std::fopen(path.c_str(), "rb")) { + char chunk[4096]; + std::size_t got = 0; + while ((got = std::fread(chunk, 1, sizeof(chunk), file)) > 0) { + all.append(chunk, got); + } + std::fclose(file); + } + } + return all; + } + + void TruncateRoleLogs(const char* basePath) { + // BOTH, because a reader that concatenates them would otherwise fold a previous + // run's server lines into this one's - which is the stale-tail failure R-16 was + // written after, one file over. + for (const LogRole role : {LogRole::Client, LogRole::Server}) { + if (FILE* file = std::fopen(RoleLogPath(basePath, role).c_str(), "w")) { + std::fclose(file); + } + } + } + + // MOBILEGL_LOG_FILE_PATH IS A BASE NAME NOW, AND NEITHER ROLE KEEPS IT. + // + // `foo.log` becomes `foo.client.log` and `foo.server.log`; a path with no extension gets + // the suffix appended. The suffix goes BEFORE the extension so the pair sorts together + // and a `*.log` glob still finds both. + // + // THE CLIENT IS RENAMED TOO, ON PURPOSE. Leaving it at the unsuffixed path would have + // been the compatible choice and the wrong one: every 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 ENOENT and says so. + // There were fourteen such readers in this tree when the split landed. + std::string RoleLogPath(const char* basePath, LogRole role) { + const char* roleSuffix = role == LogRole::Server ? "server" : "client"; + std::string path(basePath); + 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); + } +#endif + std::mutex& LogMutex() { static auto* mutex = new std::mutex(); return *mutex; @@ -64,63 +146,63 @@ namespace MobileGL { void InitFile() { #if MOBILEGL_LOG_ENABLE_FILE - if (!s_logFile) { - // MOBILEGL_LOG_FILE_PATH must stay a raw getenv (not MG_Config::Features): - // InitFile() runs before MG_ConfigLoader::Init() in MobileGL::Initialize(). - const char* logPath = std::getenv("MOBILEGL_LOG_FILE_PATH"); - if (!logPath || !*logPath) { - logPath = MOBILEGL_LOG_FILE_PATH; - } - if (logPath && *logPath) { - // "a", NOT "w", WHENEVER A SECOND PROCESS SHARES THIS PATH. - // - // Under transport=spawn the server is a separate process - // writing the same MOBILEGL_LOG_FILE_PATH, and two "w" - // handles each carry their OWN file offset: both start at 0 - // and overwrite each other's bytes. The result is not - // interleaved, it is CORRUPT - observed as half-lines like - // `ote client: CapsMirror generation 1 adopted` while - // diagnosing the spawn retrace, and as the server's whole - // startup block vanishing under the client's later writes. - // O_APPEND makes each write atomic at the end of the file, - // which is what makes the log a usable census across both - // processes - and run_trace_case.cmake's Fatal{ count reads - // exactly this file. - // - // THE CLIENT STILL TRUNCATES ONCE, so a fresh run does not - // read as a growing pile of old ones. The server never does: - // it is always started by a client that has already opened - // this path, and a truncate from the far side would delete - // the very startup lines a failed session needs. - // - // GUARDED, BECAUSE G1 MEASURES BYTES AND NOT INTENT. Only a disaggregated - // build can ever have a second process on this path, so a pull build gains - // nothing from the append - and it would pay for it in .text, which G1 - // requires to be unchanged. Measured: unguarded, the pull library's .text - // moved 16 bytes with its symbol set identical, which is exactly the kind of - // drift that is invisible unless somebody looks. -#if MOBILEGL_BUILD_DISAGGREGATED - const char* role = std::getenv("MOBILEGL_IPC_ROLE"); - const bool isSpawnedServer = role != nullptr && std::strcmp(role, "server") == 0; - if (!isSpawnedServer) { - if (FILE* truncate = std::fopen(logPath, "w")) std::fclose(truncate); - } - s_logFile = std::fopen(logPath, "a"); -#else - s_logFile = std::fopen(logPath, "w"); -#endif - } + // MOBILEGL_LOG_FILE_PATH must stay a raw getenv (not MG_Config::Features): + // InitFile() runs before MG_ConfigLoader::Init() in MobileGL::Initialize(). + const char* logPath = std::getenv("MOBILEGL_LOG_FILE_PATH"); + if (!logPath || !*logPath) { + logPath = MOBILEGL_LOG_FILE_PATH; } + if (!logPath || !*logPath) { + return; + } +#if MOBILEGL_BUILD_DISAGGREGATED + // ONE FILE PER ROLE, OPENED LAZILY BY WHOEVER FIRST WRITES ONE. + // + // Each role truncates ITS OWN file exactly once, which is what the previous + // single-file design could not do: there the client truncated and the server was + // forbidden to, because a truncate from the far side would have deleted the very + // startup lines a failed session needs. With separate files that asymmetry is gone - + // nobody else writes the file being truncated. + // + // "w" IS SAFE AGAIN HERE, and the O_APPEND this replaces was never about ordering: + // it was about two PROCESSES holding two handles to one path at two offsets, each + // overwriting the other's bytes. Two paths, two writers, no sharing. + if (ThreadIsServerRole()) { + if (!s_serverLogFile) { + s_serverLogFile = std::fopen(RoleLogPath(logPath, LogRole::Server).c_str(), "w"); + } + return; + } + if (!s_logFile) { + s_logFile = std::fopen(RoleLogPath(logPath, LogRole::Client).c_str(), "w"); + } +#else + if (!s_logFile) { + s_logFile = std::fopen(logPath, "w"); + } +#endif #endif } void WriteToFile(const char* msg) { #if MOBILEGL_LOG_ENABLE_FILE +#if MOBILEGL_BUILD_DISAGGREGATED + // The sink is chosen by the CALLING THREAD's role, which is the only choice that is + // correct under inproc: there both roles are threads of one process, so anything + // process-scoped would file the apply thread's lines under the client. + FILE** sink = ThreadIsServerRole() ? &s_serverLogFile : &s_logFile; + if (!*sink) InitFile(); + if (*sink) { + std::fputs(msg, *sink); + std::fflush(*sink); + } +#else if (!s_logFile) InitFile(); if (s_logFile) { std::fputs(msg, s_logFile); std::fflush(s_logFile); } +#endif #endif } @@ -169,6 +251,12 @@ namespace MobileGL { void Close() { #if MOBILEGL_LOG_ENABLE_FILE +#if MOBILEGL_BUILD_DISAGGREGATED + if (s_serverLogFile) { + std::fclose(s_serverLogFile); + s_serverLogFile = nullptr; + } +#endif if (s_logFile) { std::fclose(s_logFile); s_logFile = nullptr; diff --git a/MobileGL/MG_Util/Debug/Log.h b/MobileGL/MG_Util/Debug/Log.h index 299e63a4..47bd79ab 100644 --- a/MobileGL/MG_Util/Debug/Log.h +++ b/MobileGL/MG_Util/Debug/Log.h @@ -122,6 +122,8 @@ {} #endif +#include + namespace MobileGL { namespace MG_Util { namespace Debug { @@ -131,6 +133,47 @@ namespace MobileGL { constexpr char* GetOSName(); std::string GetThreadName(); void Close(); + +#if MOBILEGL_BUILD_DISAGGREGATED + // P6: ONE LOG PER ROLE, and the role is a per-THREAD fact because under inproc both + // roles live in one process. The client keeps MOBILEGL_LOG_FILE_PATH unchanged so + // every existing reader keeps its path; the server's lines go to the same path with + // `.server` inserted before the extension. + // + // WHY NOT ONE SHARED FILE. It worked - O_APPEND made the spawn case correct - but it + // put two sessions' lines in one place and made every per-side assertion a search + // rather than a read. The `--require-spawn` gate had to accept "ANY Config: IPC line + // matches" because the server's own line is scrubbed of the client's knobs by + // construction; with the two separated it can demand that THE client's line matches. + // + // THREAD-SCOPED, NOT PROCESS-SCOPED, for the case that is easy to miss: under inproc + // the server role is the mgl-srv-apply THREAD, so a process-wide flag would file its + // lines under whichever role set it last. + enum class LogRole { Client, Server }; + + // Called by the apply thread when it starts (inproc), and once by ServerMain for a + // whole spawned server process. Without it a thread defaults to the process role, + // which MOBILEGL_IPC_ROLE=server sets for the spawned image. + void SetThreadLogRole(LogRole role); + + // THE NAMING RULE, EXPORTED, so that no reader has to reimplement it. Every consumer + // of a lane's log - the unit mains, the integration harness, the retrace runner - + // needs the same `.` derivation, and a second copy is a copy that + // can fall behind: the one that does opens a file that does not exist and reports + // "the library never logged", which reads as a product failure. + std::string RoleLogPath(const char* basePath, LogRole role); + + // THE TWO OPERATIONS EVERY READER ACTUALLY WANTS, so that no caller has to know how + // many roles there are or how they are spelled. + // + // A test asserts that THE RUN said something; which role's thread said it is not + // what the assertion is about, and guessing wrong turns a real diagnostic into "the + // child aborted and said nothing". Refusals raised on the apply thread are written + // under the SERVER role by construction, which is exactly the class of diagnostic + // these readers exist to check. + std::string ReadRoleLogs(const char* basePath); + void TruncateRoleLogs(const char* basePath); +#endif } // namespace Debug } // namespace MG_Util } // namespace MobileGL diff --git a/android-plugin/trace-replay-ci.sh b/android-plugin/trace-replay-ci.sh index 75f2480e..199097ad 100644 --- a/android-plugin/trace-replay-ci.sh +++ b/android-plugin/trace-replay-ci.sh @@ -239,7 +239,7 @@ collect_run_diagnostics() { adb_device_path shell dumpsys activity activities > "${diagnostics_dir}/activity.txt" 2>&1 || true adb_device_path shell run-as "${package_name}" ls -laR "${app_dir}" > "${diagnostics_dir}/app-files.txt" 2>&1 || true adb_device_path exec-out run-as "${package_name}" cat "${app_dir}/output/retrace.log" > "${diagnostics_dir}/retrace.log" || true - adb_device_path exec-out run-as "${package_name}" cat "${app_dir}/output/mobilegl.log" > "${diagnostics_dir}/mobilegl.log" || true + collect_role_logs "${app_dir}/output/mobilegl.log" "${diagnostics_dir}/mobilegl.log" || true } # Records why a retrace was charged to the infrastructure rather than the code @@ -305,6 +305,39 @@ copy_app_artifact() { fi } +# P6: the library writes ONE LOG PER ROLE - .client.log and .server.log - because +# under inproc both roles are threads of one process. Pull both and concatenate into the single +# every downstream reader still expects: the client-only markers (ConfigLoader's transport +# line, `spawn ARMED`, the client Config: IPC line) are present, and the Fatal census and the +# capabilities probe see BOTH roles - the applier's refusals and its GL_RENDERER line live on the +# server side. The two source files are also kept beside for a human reading the artifact. +collect_role_logs() { + base_source="$1" # e.g. /output/mobilegl.log + dest="$2" # e.g. /mobilegl.log + base_no_ext="${base_source%.log}" + dest_no_ext="${dest%.log}" +: > "${dest}" + wrote_any=0 + for role in client server; do + role_src="${base_no_ext}.${role}.log" + role_dst="${dest_no_ext}.${role}.log" + # PULL, THEN JUDGE BY CONTENT. `run-as test -f` is unreliable through `exec-out` on some + # devices (measured: it returned 0 for a file that does not exist), and `exec-out` folds the + # device-side `cat: ... No such file` error into the stdout stream - so a missing role file + # arrives as a one-line log that begins with `cat:`. The monolith arm legitimately has no + # server file, and that must be a silent skip rather than a one-line server log. So: pull, + # then drop anything empty or carrying cat's own not-found line. + adb_device_path exec-out run-as "${package_name}" cat "${role_src}" > "${role_dst}" 2>/dev/null || true + if [ -s "${role_dst}" ] && ! head -1 "${role_dst}" | grep -q '^cat: .*: No such file'; then + cat "${role_dst}" >> "${dest}" + wrote_any=1 + else + rm -f "${role_dst}" + fi + done + [ "${wrote_any}" -eq 1 ] || rm -f "${dest}" +} + copy_texture_2d_dumps() { [ -n "${texture_2d_dumps}" ] || return 0 saved_ifs="${IFS}" @@ -505,7 +538,7 @@ run_retrace() { copy_app_artifact "${app_dir}/output/${safe_case}-diff.png" "${result_dir}/${safe_case}-${backend}-diff.png" fi copy_app_artifact "${app_dir}/output/retrace.log" "${result_dir}/retrace.log" - copy_app_artifact "${app_dir}/output/mobilegl.log" "${result_dir}/mobilegl.log" + collect_role_logs "${app_dir}/output/mobilegl.log" "${result_dir}/mobilegl.log" if [ "${benchmark}" -eq 1 ]; then copy_app_artifact "${app_dir}/output/benchmark.json" "${result_dir}/benchmark.json" fi @@ -592,10 +625,12 @@ armed = re.search(r"spawn ARMED - the server role runs in pid (\d+)", text) # `any`, NOT the `all` the inproc gate uses, and the difference is load-bearing. # -# Under spawn TWO PROCESSES append to one MOBILEGL_LOG_FILE_PATH, and the launcher deliberately -# scrubs every MOBILEGL_IPC_* out of the child's environment (anti-recursion catch (b)) - so the -# server's own `Config: IPC` line reads strict=0 role-split-state=0 BY CONSTRUCTION. Demanding -# that every line match would red every spawn run for the one thing the design requires. +# This log is the CONCATENATION of the two role files (collect_role_logs), so it carries the +# SERVER's own `Config: IPC` line as well as the client's - and the server's reads strict=0 +# role-split-state=0 by construction, because the launcher scrubs every MOBILEGL_IPC_* out of +# the child's environment (anti-recursion catch (b)). Demanding that EVERY line match would red +# every spawn run for the one thing the design requires; `any` asks that the CLIENT's line is +# present, which is the one that carries the knobs. configs = re.findall(r"Config: IPC[^\r\n]*", text) fatals = re.findall(r"^.*Fatal\{.*$", text, re.MULTILINE) required = {"strict": "1", "role-split-state": "1", "run-ahead": "1"} diff --git a/scripts/ci/retrace_drop_draw_control.sh b/scripts/ci/retrace_drop_draw_control.sh index b064abc6..cb0c6eca 100644 --- a/scripts/ci/retrace_drop_draw_control.sh +++ b/scripts/ci/retrace_drop_draw_control.sh @@ -40,7 +40,7 @@ # Usage: retrace_drop_draw_control.sh # CTEST ctest binary (default: ctest) # CONTROL_TMPDIR scratch dir (default: ${RUNNER_TEMP:-/tmp}) -# LIBRARY_LOG the replay's library log (default: //output/mobilegl.log) +# LIBRARY_LOG the replay's CLIENT library log (default: //output/mobilegl.client.log) set -u CASE="${1:?usage: retrace_drop_draw_control.sh }" @@ -48,7 +48,10 @@ BACKEND="${2:?usage: retrace_drop_draw_control.sh }" CTEST="${CTEST:-ctest}" CONTROL_TMPDIR="${CONTROL_TMPDIR:-${RUNNER_TEMP:-/tmp}}" -LIBRARY_LOG="${LIBRARY_LOG:-${CASE}/${BACKEND}/output/mobilegl.log}" +# P6: BOTH ROLES HAVE THEIR OWN LOG and neither keeps the old name, so an un-updated default +# here would be a file that does not exist rather than half a session read as a whole one. This +# control's marker (`E2 control armed`) is a CLIENT-side line. +LIBRARY_LOG="${LIBRARY_LOG:-${CASE}/${BACKEND}/output/mobilegl.client.log}" mkdir -p "${CONTROL_TMPDIR}" FROZEN_LIBRARY="${FROZEN_LIBRARY:?FROZEN_LIBRARY must name the split library the replay loads}" symbols=$(nm --defined-only "${FROZEN_LIBRARY}") || exit 1 diff --git a/scripts/ci/testdata/stub_ctest.sh b/scripts/ci/testdata/stub_ctest.sh index eb6055e7..9cb80dac 100755 --- a/scripts/ci/testdata/stub_ctest.sh +++ b/scripts/ci/testdata/stub_ctest.sh @@ -183,7 +183,12 @@ write_junit() { } # Model the library file sink separately from ctest stdout (ID-53). -log="${CONTROL_TMPDIR}/entry.log" +# +# P6: the real sink writes one file per ROLE, `.client.log` / `.server.log`, and the +# validator derives those from the ENV base. So the manifest below keeps the base names (what the +# lane sets) while the on-disk writes here land in the CLIENT file - which is the role that raises +# these markers in a real run. +log="${CONTROL_TMPDIR}/entry.client.log" if [ "${json_requested:-0}" = 1 ]; then python3 -c 'import json, os; p=os.environ["CONTROL_TMPDIR"]; entries=[("ClearThenReadPixelsScenario.ClearWithNoDrawIsVisibleToDefaultFramebufferReadPixels", "entry.log"), ("PersistentCoherentMapScenario.TwoWritesThroughTheCoherentPointerEachReachTheirOwnDraw", "pmap.log")]; entries += [("TriangleScenario.SecondEntry", "second.log")] if os.environ["STUB_MODE"] == "partial-fatal" else []; print(json.dumps({"tests": [{"name": "DirectGLES.Split."+n, "properties": [{"name": "LABELS", "value": ["integration-split"]}, {"name": "ENVIRONMENT", "value": ["MOBILEGL_LOG_FILE_PATH="+p+"/"+f]}]} for n,f in entries]}))' if [ "${mode}" = stale-fatal ]; then @@ -221,7 +226,7 @@ fi # the pixel assertion arrives, the library says nothing, and the control must refuse the red. if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ] && [ "${mode}" = evidence ]; then echo 'MGPipe: persistent-map push disabled - MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 is exit gate E3(a)'"'"'s NEGATIVE CONTROL' \ - > "${CONTROL_TMPDIR}/pmap.log" + > "${CONTROL_TMPDIR}/pmap.client.log" fi case "${mode}" in e3-skipped-selection) diff --git a/tools/trace_replay/run_trace_case.cmake b/tools/trace_replay/run_trace_case.cmake index 358de6cd..6ce7bd7f 100644 --- a/tools/trace_replay/run_trace_case.cmake +++ b/tools/trace_replay/run_trace_case.cmake @@ -101,7 +101,17 @@ message(STATUS "${replay_stdout}") message(STATUS "${replay_stderr}") set(retrace_log "${TRACE_OUTPUT_DIR}/output/retrace.log") -set(mobilegl_log "${TRACE_OUTPUT_DIR}/output/mobilegl.log") +# BOTH ROLES ARE SUFFIXED, and the client's rename is deliberate. Leaving it at the old +# `mobilegl.log` 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. +# With both names moved, an un-updated reader gets "file not found" and says so. +set(mobilegl_log "${TRACE_OUTPUT_DIR}/output/mobilegl.client.log") +# P6: THE SERVER ROLE HAS ITS OWN LOG, and the census below must read BOTH or a server-side +# Fatal{ is invisible to it. Not hypothetical: under spawn the server is where every applier +# refusal and Fatal{UnmigratedSurface} is raised, which are exactly the lines this lane counts. +# The client file keeps the unsuffixed name, so every other reader here - the transport markers, +# the pipe-verify block, the artifact copy - is unchanged. +set(mobilegl_server_log "${TRACE_OUTPUT_DIR}/output/mobilegl.server.log") if(EXISTS "${retrace_log}") file(STRINGS "${retrace_log}" gl_identity_lines REGEX "MOBILEGL_TRACE_GL_") foreach(line IN LISTS gl_identity_lines) @@ -128,8 +138,12 @@ if(DEFINED TRACE_ARTIFACT_DIR AND NOT "${TRACE_ARTIFACT_DIR}" STREQUAL "") if(EXISTS "${retrace_log}") file(COPY_FILE "${retrace_log}" "${TRACE_ARTIFACT_DIR}/${TRACE_CASE_NAME}-${TRACE_BACKEND}-retrace.log") endif() + if(EXISTS "${mobilegl_server_log}") + file(COPY_FILE "${mobilegl_server_log}" + "${TRACE_ARTIFACT_DIR}/${TRACE_CASE_NAME}-${TRACE_BACKEND}-mobilegl.server.log") + endif() if(EXISTS "${mobilegl_log}") - file(COPY_FILE "${mobilegl_log}" "${TRACE_ARTIFACT_DIR}/${TRACE_CASE_NAME}-${TRACE_BACKEND}-mobilegl.log") + file(COPY_FILE "${mobilegl_log}" "${TRACE_ARTIFACT_DIR}/${TRACE_CASE_NAME}-${TRACE_BACKEND}-mobilegl.client.log") endif() endif() @@ -336,10 +350,19 @@ if(DEFINED ENV{MOBILEGL_TRANSPORT} AND NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL " endif() endif() # The refusal census. Recorded on every split run, pass or fail. + # + # BOTH FILES, and the server's is the one that matters most: under spawn every + # applier refusal is raised over there, so a census reading only the client's would + # report a confident zero for the half of the session it cannot see. file(STRINGS "${mobilegl_log}" split_fatals REGEX "Fatal\\{") + if(EXISTS "${mobilegl_server_log}") + file(STRINGS "${mobilegl_server_log}" split_server_fatals REGEX "Fatal\\{") + list(APPEND split_fatals ${split_server_fatals}) + endif() list(LENGTH split_fatals split_fatal_count) message(STATUS "MGPipe split: ${split_case} transport=$ENV{MOBILEGL_TRANSPORT}, " - "Fatal{ lines in ${mobilegl_log}: ${split_fatal_count}") + "Fatal{ lines across ${mobilegl_log} and ${mobilegl_server_log}: " + "${split_fatal_count}") if(split_fatals) foreach(line IN LISTS split_fatals) message(STATUS "${line}")