[MG_IntegrationTest, scripts] (P7): the Tcp TextureUploadShape entry holds the mobilegl-tcp lock, and the lane tools refuse any supervisor entry that does not

2026-11-08 10:33:24 +09:00 · 2026-09-23 13:23:09 -04:00
parent d1549cba57
commit f1ce655b42
4 changed files with 160 additions and 3 deletions
@@ -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}}"
        )
@@ -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', '<unnamed>'))
+    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)
@@ -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


@@ -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('<testsuite><testcase name="TcpLane.SupervisorProtocolControls" /></testsuite>')
+            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):