diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index b7826fc7..152b9fc1 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2704,6 +2704,8 @@ if (MOBILEGL_BUILD_DISAGGREGATED) foreach(mglD2Arm Split Spawn Tcp) string(TOLOWER "${mglD2Arm}" mglD2ArmLower) set(mglD2Log "${CMAKE_CURRENT_BINARY_DIR}/texture-upload-shape-${mglD2ArmLower}-DirectGLES.log") + get_filename_component(mglD2LogName "${mglD2Log}" NAME) + set(mglD2Locks "${mglD2LogName}") set(mglD2Extra) if(mglD2Arm STREQUAL "Split") set(mglD2Transport "MOBILEGL_TRANSPORT=inproc" @@ -2718,6 +2720,13 @@ if (MOBILEGL_BUILD_DISAGGREGATED) "MOBILEGL_IPC_LOG_FORWARD=1" "MOBILEGL_IPC_TOKEN=" "MGITEST_TCP_LANE=1" "MOBILEGL_ITEST_REQUIRE_GPU=1") set(mglD2Extra FIXTURES_REQUIRED mobilegl-tcp) + # The loopback supervisor serves ONE session (ServerMain.cpp answers a second Hello + # Refuse{Busy}), and every other Tcp entry holds this lock. Without it `ctest -L + # integration-gpu -j N` ran this case beside another Tcp case and whichever client + # connected second died Refuse{Busy} -> Fatal{CapsBeforeFirstSnapshot}. + # scripts/ci/spawn_lane_parity.py and junit_tally.py --discovery now refuse any entry + # on the fixture that does not hold it. + set(mglD2Locks "${mglD2LogName}\;mobilegl-tcp") endif() mgl_itest_join_environment(MGL_D2_TUS_ENV_${mglD2Arm} "MOBILEGL_BACKEND_TYPE=DirectGLES" ${mglD2Transport} "MGITEST_SPLIT_LANE=1" @@ -2726,7 +2735,6 @@ if (MOBILEGL_BUILD_DISAGGREGATED) "MOBILEGL_ESPRYT_ENABLE_TEXTURE_VIEW=1" "MOBILEGL_LOG_FILE_PATH=${mglD2Log}" ${MGL_ITEST_CAPABILITY_ENV} ${MGL_ITEST_COMMON_ENV}) - get_filename_component(mglD2LogName "${mglD2Log}" NAME) gtest_discover_tests(MobileGLIntegrationTest TEST_PREFIX "DirectGLES.${mglD2Arm}.TextureUploadShape." TEST_FILTER "${MGL_D2_TUS_FILTER}" @@ -2734,7 +2742,7 @@ if (MOBILEGL_BUILD_DISAGGREGATED) PROPERTIES LABELS "integration-gpu\;integration-${mglD2ArmLower}" TIMEOUT ${MGL_ITEST_TIMEOUT} - RESOURCE_LOCK "${mglD2LogName}" + RESOURCE_LOCK "${mglD2Locks}" ${mglD2Extra} ENVIRONMENT "${MGL_D2_TUS_ENV_${mglD2Arm}}" ) diff --git a/scripts/ci/junit_tally.py b/scripts/ci/junit_tally.py old mode 100755 new mode 100644 index d3932f14..4ec8ff8c --- a/scripts/ci/junit_tally.py +++ b/scripts/ci/junit_tally.py @@ -36,6 +36,52 @@ ARM_PREFIXES = { TCP_ARM = re.compile(r'control=tcp data=stream server=\S+ pid=[1-9][0-9]*\b') +# THE LOOPBACK SUPERVISOR SERVES ONE SESSION AT A TIME (ServerMain.cpp answers a second Hello +# Refuse{Busy}; CONTRACT-P65.md), so every entry that reaches it has to hold the lane's lock. +# P7's D2 TextureUploadShape Tcp entry required the fixture but locked only its private log name: +# under `ctest -L integration-gpu -j 4` it ran beside another Tcp case and whichever client +# connected second died Refuse{Busy} -> Fatal{CapsBeforeFirstSnapshot}. `-L '^integration-tcp$'` +# happened to schedule it next to SupervisorProtocolControls (which runs its own supervisor), so +# the lane that exists to exercise the fixture could not see it. Asserted on the registration, not +# on a lucky schedule. +TCP_FIXTURE = 'mobilegl-tcp' +TCP_LANE_LABEL = 'integration-tcp' + + +def _list_property(test, name): + """A json-v1 list property as a list of strings (a ';'-joined string counts as a list).""" + for prop in test.get('properties', []): + if prop.get('name') != name: + continue + value = prop.get('value') + if value is None: + return [] + if isinstance(value, list): + return [str(item) for item in value] + return [item for item in str(value).split(';') if item] + return [] + + +def tcp_lock_violations(document): + """Entries of a ctest --show-only=json-v1 document that reach the loopback TCP supervisor - + FIXTURES_REQUIRED names mobilegl-tcp, or the entry is labelled exactly integration-tcp - and do + not hold RESOURCE_LOCK mobilegl-tcp. Sorted names; empty when the registration is sound.""" + bad = [] + for test in document.get('tests', []): + reaches = (TCP_FIXTURE in _list_property(test, 'FIXTURES_REQUIRED') + or TCP_LANE_LABEL in _list_property(test, 'LABELS')) + if reaches and TCP_FIXTURE not in _list_property(test, 'RESOURCE_LOCK'): + bad.append(test.get('name', '')) + return sorted(bad) + + +def require_tcp_locks(document): + bad = tcp_lock_violations(document) + if bad: + raise ValueError(f'{len(bad)} entrie(s) reach the one-session TCP supervisor without ' + f'RESOURCE_LOCK {TCP_FIXTURE}, so `ctest -j` can run them beside another ' + f'Tcp case and the second client dies Refuse{{Busy}}: ' + ', '.join(bad)) + def require_tcp_proof(path, prefix, discovery, require_run_ahead=False): helper_path = Path(__file__).resolve().parents[2] / 'MobileGL/MG_IntegrationTest/Harness/split_log_paths.py' @@ -109,7 +155,9 @@ def main(): arms = parser.add_mutually_exclusive_group() for flag, prefix in ARM_PREFIXES.items(): arms.add_argument(flag, dest='prefix', action='store_const', const=prefix) - parser.add_argument('--discovery', help='CTest --show-only=json-v1 output (required for TCP proof)') + parser.add_argument('--discovery', help='CTest --show-only=json-v1 output (required for TCP proof); ' + 'every entry in it that reaches the TCP supervisor must ' + 'hold RESOURCE_LOCK mobilegl-tcp') parser.add_argument('--require-run-ahead', action='store_true', help='Additionally prove actual run-ahead, without fallback or demotion, on each TCP case') parser.add_argument('--require-entry-passed', action='append', default=[], metavar='NAME', @@ -121,6 +169,8 @@ def main(): parser.error('--require-run-ahead requires a TCP arm selection') try: prefix = args.prefix + if args.discovery: + require_tcp_locks(json.loads(Path(args.discovery).read_text())) passed, failed, skipped = tally(args.junit, prefix=prefix) if prefix in ('DirectGLES.Tcp.', 'DirectGLES.TcpDevice.'): require_tcp_proof(args.junit, prefix, args.discovery, args.require_run_ahead) diff --git a/scripts/ci/spawn_lane_parity.py b/scripts/ci/spawn_lane_parity.py index b2a73649..7d00d8e7 100644 --- a/scripts/ci/spawn_lane_parity.py +++ b/scripts/ci/spawn_lane_parity.py @@ -26,6 +26,9 @@ that had silently lost one. So there the arm - and only the arm, the SECOND dott what comes off. """ import argparse +import importlib.util +import json +from pathlib import Path import re import subprocess import sys @@ -108,6 +111,36 @@ def lane_cases(build_dir, label): return {m.group(1) for m in CASE.finditer(out)} +def whole_build_discovery(build_dir): + """`ctest --show-only=json-v1` over EVERY entry: a lock is a property of the registration, and + an entry that reaches the supervisor under some other label is just as able to collide.""" + out = subprocess.run(["ctest", "--show-only=json-v1"], cwd=build_dir, + capture_output=True, text=True, check=True).stdout + return json.loads(out) + + +def tcp_lock_check(build_dir): + """Every entry on the loopback TCP supervisor holds RESOURCE_LOCK mobilegl-tcp (the rule and + its history live beside junit_tally.tcp_lock_violations). Returns True on failure.""" + spec = importlib.util.spec_from_file_location("junit_tally", Path(__file__).with_name("junit_tally.py")) + tally = importlib.util.module_from_spec(spec) + spec.loader.exec_module(tally) + document = whole_build_discovery(build_dir) + reaching = [t for t in document.get("tests", []) + if tally.TCP_FIXTURE in tally._list_property(t, "FIXTURES_REQUIRED") + or tally.TCP_LANE_LABEL in tally._list_property(t, "LABELS")] + bad = tally.tcp_lock_violations(document) + print(f"tcp supervisor lock: {len(reaching)} entrie(s) reach the loopback supervisor, " + f"{len(bad)} without RESOURCE_LOCK {tally.TCP_FIXTURE}") + if bad: + print(f"::error::entries that reach the one-session TCP supervisor (FIXTURES_REQUIRED " + f"{tally.TCP_FIXTURE}, or label {tally.TCP_LANE_LABEL}) must hold RESOURCE_LOCK " + f"{tally.TCP_FIXTURE}; without it `ctest -j` runs them beside another Tcp case and " + f"the second client dies Refuse{{Busy}}: " + ", ".join(bad), file=sys.stderr) + return True + return False + + def compare_arms(build_dir, tier, labels, inproc_only=(), no_tcp=()): """The three arms of one tier must name the same set after the arm segment comes off. @@ -246,6 +279,9 @@ def main(): {"split": "integration-magma-full-split", "spawn": "integration-magma-full-spawn", "tcp": "integration-magma-full-tcp"}) + # The arms above can only be compared if they can all RUN: every tcp entry shares one + # single-session supervisor, so the lock is part of what registering a tcp arm means. + failed |= tcp_lock_check(args.build_dir) return 1 if failed else 0 diff --git a/scripts/ci/tcp_lane_tools_test.py b/scripts/ci/tcp_lane_tools_test.py index 2f1a5adb..e5bced60 100644 --- a/scripts/ci/tcp_lane_tools_test.py +++ b/scripts/ci/tcp_lane_tools_test.py @@ -124,13 +124,76 @@ class Accounting(unittest.TestCase): # assertion, instead of running a real ctest in the placeholder build dir. with patch.object(parity, 'lane_cases', side_effect=[{a}, {a}, {b}]), \ patch.object(parity, 'lane_names', return_value=set()), \ + patch.object(parity, 'whole_build_discovery', return_value={'tests': []}), \ patch.object(sys, 'argv', ['spawn_lane_parity.py', 'unused']): self.assertEqual(parity.main(), 1) with patch.object(parity, 'lane_cases', return_value={a}), \ patch.object(parity, 'lane_names', return_value=set()), \ + patch.object(parity, 'whole_build_discovery', return_value={'tests': []}), \ patch.object(sys, 'argv', ['spawn_lane_parity.py', 'unused']): self.assertEqual(parity.main(), 0) + @staticmethod + def discovered(name, **properties): + return {'name': name, 'properties': [{'name': key, 'value': value} + for key, value in properties.items()]} + + def test_every_entry_on_the_one_session_supervisor_holds_its_lock(self): + """P7 CI: DirectGLES.Tcp.TextureUploadShape required mobilegl-tcp and locked only its log. + + The loopback supervisor answers a second concurrent Hello Refuse{Busy}, so under + `ctest -L integration-gpu -j 4` that entry and another Tcp case killed each other. + The stub mirrors the real json-v1 shapes: the fixture's own setup entry, a sound Tcp + case, the violator as it was registered, a Magma tcp entry that reaches the fixture + under another label, a labelled control with no fixture, and the device lane, whose + lock is its own.""" + tally = module('junit_tally') + parity = module('spawn_lane_parity') + entry = self.discovered + tus = 'DirectGLES.Tcp.TextureUploadShape.TextureUploadShapeScenario.TheEmittedUploadShapeIsRecordedAndTheTwoSidesAgree' + sound = [ + entry('TcpServer.Start', FIXTURES_SETUP=['mobilegl-tcp']), + entry('DirectGLES.Tcp.TriangleScenario.Draw', LABELS=['integration-gpu', 'integration-tcp'], + FIXTURES_REQUIRED=['mobilegl-tcp'], RESOURCE_LOCK=['mobilegl-tcp']), + entry('TcpLane.SupervisorProtocolControls', LABELS=['integration-tcp'], RESOURCE_LOCK=['mobilegl-tcp']), + entry('DirectGLES.TcpDevice.TriangleScenario.Draw', LABELS=['integration-tcp-device'], + RESOURCE_LOCK=['mobilegl-tcp-device']), + entry('DirectGLES.Split.TriangleScenario.Draw', LABELS=['integration-split'], RESOURCE_LOCK=['x.log']), + # A lock list that reached json-v1 as one ';'-joined string still counts. + entry(tus, LABELS='integration-gpu;integration-tcp', FIXTURES_REQUIRED='mobilegl-tcp', + RESOURCE_LOCK='texture-upload-shape-tcp-DirectGLES.log;mobilegl-tcp'), + ] + self.assertEqual(tally.tcp_lock_violations({'tests': sound}), []) + broken = sound[:-1] + [ + entry(tus, LABELS=['integration-gpu', 'integration-tcp'], FIXTURES_REQUIRED=['mobilegl-tcp'], + RESOURCE_LOCK=['texture-upload-shape-tcp-DirectGLES.log']), + entry('DirectVulkan.Tcp.Fm.TriangleScenario.Draw', LABELS=['integration-magma-tcp'], + FIXTURES_REQUIRED=['mobilegl-tcp']), + entry('TcpLane.Unlocked', LABELS=['integration-tcp']), + ] + expected = sorted([tus, 'DirectVulkan.Tcp.Fm.TriangleScenario.Draw', 'TcpLane.Unlocked']) + self.assertEqual(tally.tcp_lock_violations({'tests': broken}), expected) + with self.assertRaises(ValueError) as raised: + tally.require_tcp_locks({'tests': broken}) + self.assertIn(tus, str(raised.exception)) + # Both places CI reads the registration: junit_tally's --discovery handling (the TCP lane + # step) and the parity gate's whole-build discovery (every label). + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + junit, discovery = root / 'junit.xml', root / 'lane.json' + junit.write_text('') + for tests, rc in ((broken, 1), (sound, 0)): + discovery.write_text(json.dumps({'tests': tests})) + with patch.object(sys, 'argv', ['junit_tally.py', str(junit), '--discovery', str(discovery), + '--require-entry-passed', 'TcpLane.SupervisorProtocolControls']): + self.assertEqual(tally.main(), rc) + for tests, rc in ((broken, 1), (sound, 0)): + with patch.object(parity, 'lane_cases', return_value={'TriangleScenario.Draw'}), \ + patch.object(parity, 'lane_names', return_value=set()), \ + patch.object(parity, 'whole_build_discovery', return_value={'tests': tests}), \ + patch.object(sys, 'argv', ['spawn_lane_parity.py', 'unused']): + self.assertEqual(parity.main(), rc) + class DeviceIdleExemption(unittest.TestCase): def test_stop_restores_only_the_setting_this_task_changed(self):