From 6fa6a925bdf91ba7e84d7f4ca3fde3e928086583 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:34:33 -0400 Subject: [PATCH 1/2] [Fix] (CI): make split controls mandatory and isolate runner evidence --- .github/workflows/test.yml | 40 +++++++++---------- .../Harness/split_log_paths.py | 8 ++++ MobileGL/MG_Test/Wire/c1f_redcheck.py | 19 ++++++--- scripts/ci/census_junit.py | 24 +++++++++++ scripts/ci/control_smoke_test.sh | 17 +++++--- scripts/ci/junit_tally.py | 13 ++++-- scripts/ci/redcheck_control_smoke_test.sh | 2 +- scripts/ci/retrace_drop_draw_control.sh | 10 +++++ scripts/ci/retrace_pull_library_control.sh | 19 ++++++++- scripts/ci/split_negative_controls.sh | 13 +++--- .../ci/testdata/split_private_log_smoke.sh | 3 +- scripts/ci/testdata/stub_ctest.sh | 21 +++++++++- scripts/p3a_untouched_regions.sh | 5 ++- scripts/p4a_untouched_regions.sh | 5 ++- 14 files changed, 148 insertions(+), 51 deletions(-) create mode 100644 scripts/ci/census_junit.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 521bb04b..e6371b80 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -962,6 +962,8 @@ jobs: # workflow passed that option. They had never been compiled by CI, let alone run. - name: Unit tests on the split runtime working-directory: build-split + env: + MOBILEGL_ITEST_REQUIRE_GPU: "1" run: ctest --output-on-failure -L unit --no-tests=error -j "$(nproc)" # --no-tests=error is half the gate, exactly as in integration-verify: the integration-split @@ -979,13 +981,11 @@ jobs: run: | ulimit -c unlimited sudo sysctl -w kernel.core_pattern='/tmp/core.%e.%p' - ctest --output-on-failure -L integration-split --no-tests=error -j 4 + ctest --output-on-failure -L integration-split --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/split-baseline.xml" + python3 ../scripts/ci/junit_tally.py "${RUNNER_TEMP}/split-baseline.xml" --require-split-ran - # ARCHITECTURE.md:521 asks for `ctest -L integration-gpu` to be name-for-name identical - # between the monolith and the split arm of the SAME build - the G2 shape extended to a - # third arm. The knob goes in the JOB environment rather than in a ctest property, for the - # reason the MOBILEGL_ESPRYT_DISABLE_INVALIDATE_FLUSH precedent in `integration` gives: a - # property would override it and the arm would not be an arm. + # ID-65 supersedes broad inproc status parity: class-C aborts and named wrong-answer + # debts are recorded. Monolith, integration-split and the controls remain hard gates. # # The entries that name MOBILEGL_TRANSPORT in their OWN property (the Split. lanes) keep # their value in both passes, which is correct: they are the split family in both arms and @@ -1007,14 +1007,11 @@ jobs: fi echo "integration-gpu entries in the split build: ${count}" MOBILEGL_TRANSPORT=monolith ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-monolith.xml" - MOBILEGL_TRANSPORT=inproc ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-inproc.xml" - # NAME **AND STATUS**, and it is the comparison this step claimed to make and did not - # (review finding N-3): the first version wrote a names file and never read it, and - # `--output-on-failure` treats a SKIPPED test as not-a-failure - so the very failure - # ARCHITECTURE.md:521 is about, "an inproc arm that skipped forty entries the monolith arm - # ran", was invisible here and caught only by the local gate. `ctest -N` cannot see it - # either: this is one build directory, so the two arms have identical name lists by - # construction and the difference is entirely in what each entry DID. + # ID-65: broad inproc is a recorded debt census. Reduced split + controls gate below. + inproc_rc=0 + MOBILEGL_TRANSPORT=inproc ctest --output-on-failure -L integration-gpu --no-tests=error -j 4 --output-junit "${RUNNER_TEMP}/arm-inproc.xml" || inproc_rc=$? + python3 ../scripts/ci/census_junit.py "${RUNNER_TEMP}/arm-inproc.xml" "${inproc_rc}" >> "${GITHUB_STEP_SUMMARY}" + # Retain the per-name status delta as evidence, not as the reduced-path gate. python3 - "${RUNNER_TEMP}/arm-monolith.xml" "${RUNNER_TEMP}/arm-inproc.xml" <<'PY' import sys, xml.etree.ElementTree as ET def rows(path): @@ -1031,13 +1028,10 @@ jobs: diff = sorted(set(a) ^ set(b)) + sorted(n for n in set(a) & set(b) if a[n] != b[n]) if diff: for name in diff[:40]: - print(f"::error::{name}: monolith={a.get(name, '')} inproc={b.get(name, '')}") - print(f"::error::the monolith and inproc arms of ctest -L integration-gpu differ on " - f"{len(diff)} entries. ARCHITECTURE.md:521 requires them identical name for name " - f"AND status; an entry that SKIPPED on one arm and ran on the other is the " - f"failure this compares for, and it is not a failure to --output-on-failure.") - raise SystemExit(1) - print(f"the two arms agree on all {len(a)} entries, name and status") + print(f"{name}: monolith={a.get(name, '')} inproc={b.get(name, '')}") + print(f"Recorded ID-65 census: {len(diff)} name/status differences; not a parity gate") + else: + print(f"the two arms agree on all {len(a)} entries, name and status") PY # THE RUNTIME HALF OF "THIS IS REALLY A SPLIT BUILD". The build-level nm check in @@ -1095,6 +1089,7 @@ jobs: # and FAILED as evidence the lane was live, so the controls could be measured against a # baseline that was already red. - name: Negative controls - the verb barrier and the persistent-map push must be load-bearing + if: ${{ !cancelled() }} working-directory: build-split env: MOBILEGL_ITEST_REQUIRE_GPU: "1" @@ -1109,6 +1104,8 @@ jobs: path: | build-split/MobileGL/MG_IntegrationTest/*.log* build-split/MobileGL/MG_IntegrationTest/split-logs/*.log + ${{ runner.temp }}/arm-inproc.xml + ${{ runner.temp }}/arm-monolith.xml if-no-files-found: warn - name: Upload core dumps @@ -1978,6 +1975,7 @@ jobs: env: CONTROL_TMPDIR: ${{ runner.temp }} LIBRARY_LOG: ${{ matrix.case }}/${{ matrix.backend }}/output/mobilegl.log + FROZEN_LIBRARY: ${{ github.workspace }}/build-linux/libMobileGL.so run: >- bash "${GITHUB_WORKSPACE}/scripts/ci/retrace_drop_draw_control.sh" '${{ matrix.case }}' '${{ matrix.backend }}' diff --git a/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py b/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py index 689717e0..c082f73d 100644 --- a/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py +++ b/MobileGL/MG_IntegrationTest/Harness/split_log_paths.py @@ -68,6 +68,14 @@ def main(): for n in selected for c in by_name[n]) if not_failed: raise ValueError(f"{label} control: {not_failed} selected entries did not fail") + elif mode == "assertion": + cases = ET.parse(sys.argv[4]).getroot().findall(".//testcase") + by_name = {case.get("name"): case for case in cases} + for name in selected: + case = by_name.get(name) + output = "" if case is None else " ".join(" ".join(case.itertext()).split()) + if not re.search(sys.argv[5], output): + raise ValueError(f"E3(a) FAILED: {name} red lacks its persistent-map push diagnostic") elif mode == "evidence": missing = [] label = sys.argv[5] if len(sys.argv) > 5 else "" diff --git a/MobileGL/MG_Test/Wire/c1f_redcheck.py b/MobileGL/MG_Test/Wire/c1f_redcheck.py index 602bb93b..1535277f 100644 --- a/MobileGL/MG_Test/Wire/c1f_redcheck.py +++ b/MobileGL/MG_Test/Wire/c1f_redcheck.py @@ -69,7 +69,7 @@ def cases(): [SUITE+'BoundPackBufferOffsetReadRefusesByName'], 'RemoteClientTest'), ('codex12-repeat-skip', BACKEND, replace('if (draw != EGL_NO_SURFACE && ctx != EGL_NO_CONTEXT) {', 'if (false) {'), - [SUITE+'RepeatedMakeCurrentAdoptsRepublishedCapsWithoutAPumpOrPresent'], 'RemoteClientTest'), + [SUITE+'ADifferentTupleMakeCurrentIsAdoptedWithoutAPumpOrPresent'], 'RemoteClientTest'), ('BlobMissing-optional-to-required', WIRE, replace('record.Blob = StageOptional(session, blobBytes, blobByteCount);', 'record.Blob = StageRequired(session, "SetDynamicState", blobBytes, blobByteCount);'), @@ -122,13 +122,20 @@ def main(): binary = ROOT / 'build-split/MobileGL/MG_Test' / ('Pipe' if target == 'PipeCatalogueTest' else 'Wire') / target build = ['cmake', '--build', 'build-split', '-j', '24', '--target', target] run = [str(binary), '--gtest_filter='+':'.join(names)] + # ID-67: both replacement cases must execute green before/after this mutation. + # Suppressing client adoption only reddens the different-tuple case; an identical + # tuple correctly republishes nothing and must remain green under the perturbation. + green_names = names + if label == 'codex12-repeat-skip': + green_names = names + [SUITE+'AnIdenticalRepeatedMakeCurrentRepublishesNothing'] + green_run = [str(binary), '--gtest_filter='+':'.join(green_names)] print('\n=== '+label+' ===', flush=True) try: brc, out = command(build) if brc: raise RuntimeError('baseline build failed\n'+out) - rc, out = command(run) - if rc or any('[ OK ] '+name+' (' not in out for name in names): + rc, out = command(green_run) + if rc or any('[ OK ] '+name+' (' not in out for name in green_names): raise RuntimeError('baseline not green\n'+out) path.write_text(mutate(original.decode())) brc, out = command(build) @@ -146,12 +153,12 @@ def main(): finally: path.write_bytes(original) brc, out = command(build) - rc, out = command(run) if brc == 0 else (brc, out) - if rc or any('[ OK ] '+name+' (' not in out for name in names): + rc, out = command(green_run) if brc == 0 else (brc, out) + if rc or any('[ OK ] '+name+' (' not in out for name in green_names): print('RESTORE-FAIL\n'+out, flush=True) failures.append(label+' restore') else: - print('RESTORED GREEN: '+', '.join(names), flush=True) + print('RESTORED GREEN: '+', '.join(green_names), flush=True) print('FAILED_CONTROLS='+repr(failures), flush=True) return bool(failures) diff --git a/scripts/ci/census_junit.py b/scripts/ci/census_junit.py new file mode 100644 index 00000000..2518780d --- /dev/null +++ b/scripts/ci/census_junit.py @@ -0,0 +1,24 @@ +#!/usr/bin/env python3 +"""Summarize the indebted broad lane without turning debt into a hard gate.""" +import collections +import sys +import xml.etree.ElementTree as ET + +cases = ET.parse(sys.argv[1]).getroot().findall('.//testcase') +if not cases: + sys.exit('Census FAILED: no executed test records') +counts = collections.Counter() +for case in cases: + failure = case.find('failure') + if failure is not None: + reason = failure.get('message', '') + status = 'aborted' if 'aborted' in reason.lower() else 'failed' + elif case.find('skipped') is not None: + status = 'skipped' + else: + status = 'passed' + counts[status] += 1 +print('### Broad inproc census (ID-65: recorded debt, not a gate)') +print(f'CTest exit: {sys.argv[2]}; total: {len(cases)}') +print('\n| passed | skipped | aborted | failed |\n|---:|---:|---:|---:|') +print('| ' + ' | '.join(str(counts[k]) for k in ('passed', 'skipped', 'aborted', 'failed')) + ' |') diff --git a/scripts/ci/control_smoke_test.sh b/scripts/ci/control_smoke_test.sh index 89e27b80..324ae64d 100644 --- a/scripts/ci/control_smoke_test.sh +++ b/scripts/ci/control_smoke_test.sh @@ -56,10 +56,15 @@ run_split() { # $1 = STUB_MODE run_retrace() { # $1 = STUB_MODE cd "${WORK}" || return 127 mkdir -p "${WORK}/OpenRA" + local rc=0 env -i PATH="${STUB_DIR}:/usr/bin:/bin" STUB_MODE="$1" \ CTEST=ctest CONTROL_TMPDIR="${WORK}/tmp-$1" \ PULL_LIBRARY="${WORK}/pull.so" FROZEN_LIBRARY="${WORK}/frozen.so" \ - bash "${HERE}/retrace_pull_library_control.sh" OpenRA DirectGLES + bash "${HERE}/retrace_pull_library_control.sh" OpenRA DirectGLES || rc=$? + cmp -s "${WORK}/frozen.so" "${WORK}/split.so" || { + echo 'F6 FAILED: pull control did not restore the split library'; return 1; + } + return "${rc}" } run_drop_draw() { # $1 = STUB_MODE @@ -67,7 +72,7 @@ run_drop_draw() { # $1 = STUB_MODE mkdir -p "${WORK}/OpenRA" env -i PATH="${STUB_DIR}:/usr/bin:/bin" STUB_MODE="$1" \ CTEST=ctest CONTROL_TMPDIR="${WORK}/tmp-$1" \ - LIBRARY_LOG="${WORK}/tmp-$1/mobilegl.log" \ + FROZEN_LIBRARY="${WORK}/frozen.so" LIBRARY_LOG="${WORK}/tmp-$1/mobilegl.log" \ bash "${HERE}/retrace_drop_draw_control.sh" OpenRA DirectGLES } @@ -80,8 +85,8 @@ expect PASSED "the scenarios' own diagnostic" -- run_split evidenc expect FAILED "the knob leaves the selection green" -- run_split green # The arming counter's half of the finding: a baseline that is already red cannot arm anything. expect FAILED "the baseline is already red" -- run_split red-baseline -# The disarmed lane, which is a legitimate exit 0 while c1/s1/v1 are landing. -expect PASSED "every split entry skipped (lane not armed)" -- run_split all-skipped +# P5 is complete: losing the runtime implementation must no longer disarm the gate. +expect FAILED "every split entry skipped (implementation lost)" -- run_split all-skipped echo echo "=== the retrace lane's pull-library control (scripts/ci/retrace_pull_library_control.sh)" @@ -93,7 +98,9 @@ else echo "no cc available; the retrace half of this smoke test needs one" >&2 exit 1 fi -: > "${WORK}/frozen.so" +printf '%s\n' 'int MG_Remote_stub(void) { return 1; }' > "${WORK}/split.c" +cc -shared -fPIC -o "${WORK}/frozen.so" "${WORK}/split.c" || exit 1 +cp "${WORK}/frozen.so" "${WORK}/split.so" # THE FINDING, part (b): a regex matching no tests. --no-tests=error exits non-zero and the old # control read that as "the pull library turned it red". diff --git a/scripts/ci/junit_tally.py b/scripts/ci/junit_tally.py index e1a68dbe..f36ef87f 100755 --- a/scripts/ci/junit_tally.py +++ b/scripts/ci/junit_tally.py @@ -20,9 +20,11 @@ import sys import xml.etree.ElementTree as ET -def tally(path): +def tally(path, split_only=False): passed = failed = skipped = 0 for case in ET.parse(path).getroot().iter('testcase'): + if split_only and not case.get('name', '').startswith('DirectGLES.Split.'): + continue if case.find('failure') is not None or case.find('error') is not None: failed += 1 elif case.find('skipped') is not None or case.get('status') in ('notrun', 'disabled'): @@ -33,15 +35,18 @@ def tally(path): def main(): - if len(sys.argv) != 2: - print("usage: junit_tally.py ", file=sys.stderr) + if len(sys.argv) not in (2, 3) or (len(sys.argv) == 3 and sys.argv[2] != '--require-split-ran'): + print("usage: junit_tally.py [--require-split-ran]", file=sys.stderr) return 2 try: - passed, failed, skipped = tally(sys.argv[1]) + passed, failed, skipped = tally(sys.argv[1], split_only=len(sys.argv) == 3) except Exception as exc: # a malformed file is not "zero of everything" print(f"junit_tally: cannot parse {sys.argv[1]}: {exc}", file=sys.stderr) return 1 print(f"{passed} {failed} {skipped}") + if len(sys.argv) == 3 and (passed == 0 or failed): + print('split baseline FAILED: no successful split runtime entries or an already-red selection', file=sys.stderr) + return 1 return 0 diff --git a/scripts/ci/redcheck_control_smoke_test.sh b/scripts/ci/redcheck_control_smoke_test.sh index 043ccc8a..61e71489 100755 --- a/scripts/ci/redcheck_control_smoke_test.sh +++ b/scripts/ci/redcheck_control_smoke_test.sh @@ -55,7 +55,7 @@ split, retrace, dropdraw = sys.argv[1], sys.argv[2], sys.argv[3] # other two catching the smoke test's "unrelated failure" case, and the case never flips - which # is what this script measured the first time the perturbation actually applied. rules = [ - (split, [('LINE', 'grep -qE "${evidence}"', ('if ! ', 'elif ! ')), + (split, [('LINE', '"${log_helper}" assertion', ('if ! ', 'elif ! ')), ('SUBST', '"[A-Za-z_][A-Za-z_0-9]*"\\}\' || exit 1', '"[A-Za-z_][A-Za-z_0-9]*"\\}\' || true'), ('SUBST', '"${private_evidence}" "${name}" || exit 1', '"${private_evidence}" "${name}" || true')]), diff --git a/scripts/ci/retrace_drop_draw_control.sh b/scripts/ci/retrace_drop_draw_control.sh index c7597c0c..5198b1f6 100644 --- a/scripts/ci/retrace_drop_draw_control.sh +++ b/scripts/ci/retrace_drop_draw_control.sh @@ -50,6 +50,14 @@ CTEST="${CTEST:-ctest}" CONTROL_TMPDIR="${CONTROL_TMPDIR:-${RUNNER_TEMP:-/tmp}}" LIBRARY_LOG="${LIBRARY_LOG:-${CASE}/${BACKEND}/output/mobilegl.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 +remote_count=$(printf '%s\n' "${symbols}" | grep -ic MG_Remote || true) +echo "draw-drop control library: ${FROZEN_LIBRARY}: MG_Remote=${remote_count}" +if [ "${remote_count}" -lt 1 ]; then + echo '::error::draw-drop control requires a split library: MG_Remote=0' + exit 1 +fi selector="^MobileGLTraceReplay\.${CASE}\.${BACKEND}$" @@ -66,6 +74,8 @@ restore_good_output() { echo "restored the verified run's output over the control's" fi } +trap restore_good_output EXIT +trap 'exit 130' INT TERM matched=$("${CTEST}" -N -R "${selector}" | grep -cE '^ *Test *#[0-9]+:') if [ "${matched}" -lt 1 ]; then diff --git a/scripts/ci/retrace_pull_library_control.sh b/scripts/ci/retrace_pull_library_control.sh index 4f45f47f..89ce9036 100644 --- a/scripts/ci/retrace_pull_library_control.sh +++ b/scripts/ci/retrace_pull_library_control.sh @@ -69,6 +69,18 @@ restore_good_output() { fi } +# Restore the exact split library on success, failure, and interruption. The following +# draw-drop control uses this same frozen path. +saved_library=$(mktemp "${CONTROL_TMPDIR}/split-library.XXXXXX") || exit 1 +cp -p "${FROZEN_LIBRARY}" "${saved_library}" || exit 1 +restore_control() { + cp -p "${saved_library}" "${FROZEN_LIBRARY}" + rm -f "${saved_library}" + restore_good_output +} +trap restore_control EXIT +trap 'exit 130' INT TERM + # HOLE 1: COUNT THE SELECTION FIRST. `--no-tests=error` turns an empty selection into a non-zero # exit, which is indistinguishable from a working control unless the selection is counted. matched=$("${CTEST}" -N -R "${selector}" | grep -cE '^ *Test *#[0-9]+:') @@ -82,8 +94,11 @@ fi # in. It defines no MG_Remote symbol, so ConfigLoader has no transport parser and # MOBILEGL_TRANSPORT=inproc is accepted and ignored - the exact shape of "the split lane ran # monolith". -cp "${PULL_LIBRARY}" "${FROZEN_LIBRARY}" -if nm --defined-only "${FROZEN_LIBRARY}" | grep -q -i MG_Remote; then +cp "${PULL_LIBRARY}" "${FROZEN_LIBRARY}" || exit 1 +symbols=$(nm --defined-only "${FROZEN_LIBRARY}") || exit 1 +remote_count=$(printf '%s\n' "${symbols}" | grep -ic MG_Remote || true) +echo "pull control library: ${FROZEN_LIBRARY}: MG_Remote=${remote_count}" +if [ "${remote_count}" -ne 0 ]; then restore_good_output echo "::error::the control's own library defines MG_Remote symbols, so it is not a pull build and this control would prove nothing" exit 1 diff --git a/scripts/ci/split_negative_controls.sh b/scripts/ci/split_negative_controls.sh index 95a10d3a..cb3ad70d 100644 --- a/scripts/ci/split_negative_controls.sh +++ b/scripts/ci/split_negative_controls.sh @@ -51,6 +51,7 @@ CONTROL_TMPDIR="${CONTROL_TMPDIR:-${RUNNER_TEMP:-/tmp}}" mkdir -p "${CONTROL_TMPDIR}" junit="${CONTROL_TMPDIR}/isplit.xml" +rm -f "${junit}" log_helper="$(dirname "$0")/../../MobileGL/MG_IntegrationTest/Harness/split_log_paths.py" # Check ownership even while the runtime lane is disarmed and will skip. @@ -87,14 +88,14 @@ echo "split entries - passed: ${baseline_passed}, failed: ${baseline_failed}, sk # A RED BASELINE DISARMS THE CONTROLS RATHER THAN ARMING THEM (review finding 8, second half). # `|| true` plus a "not skipped" counter used to treat a case that ran and FAILED as evidence the # lane was live. Turning an already-red entry red is not a measurement. -if [ "${baseline_failed}" -gt 0 ]; then +if [ "${baseline_failed}" -gt 0 ] || [ "${baseline_rc}" -ne 0 ]; then echo "::error::${baseline_failed} DirectGLES.Split. entries are ALREADY RED with both knobs at their defaults, so neither negative control below can attribute its red to the knob it turns. Fix the lane first; a control measured against a red baseline is not a control. (This used to be swallowed by an unconditional '|| true' and counted as 'the lane is armed'.)" exit 1 fi if [ "${baseline_passed}" -lt 1 ]; then - echo "::warning::every DirectGLES.Split. entry SKIPPED, so neither negative control can fire. The arming condition is a runtime fact - MG_Config::Transport, ClientSession::Active() and ImplementedVerbCount(), read by Harness/SplitRuntimePeek - and it becomes true on the commit that lands the last of c1/s1/v1. This step becomes a gate then, with no edit; it is not a green that asserted anything today." - exit 0 + echo "::error::split baseline FAILED: every DirectGLES.Split. entry SKIPPED; the split implementation did not execute" + exit 1 fi # ---- the controls --------------------------------------------------------------------------- @@ -145,7 +146,7 @@ run_control() { if [ "${evidence}" = "private-barrier-fatal" ]; then python3 "${log_helper}" evidence "${manifest}" "${filter}" \ 'Fatal\{BarrierViolation, "[A-Za-z_][A-Za-z_0-9]*"\}' || exit 1 - elif ! tr -s '[:space:]' ' ' < "${out}" | grep -qE "${evidence}"; then + elif ! python3 "${log_helper}" assertion "${manifest}" "${filter}" "${result}" "${evidence}"; then echo "::error::${name} FAILED: red lacks its persistent-map push diagnostic. Required: ${evidence}" exit 1 fi @@ -179,8 +180,10 @@ run_control "negative control E1 (MOBILEGL_IPC_VERB_BARRIER=0)" \ # * the LIBRARY's own line in the entry's private file, saying the push was disabled by this # knob. It did not exist until ID-65 assigned it (joint-v1.md 3), which is why this control # used to rest on the pixels alone. +# TheMapLandsInTheArmItsLaneDeclares skips by design outside PersistentMapArm. +# Select only the pixel cases; a pre-flight skip in either remains a hard failure. run_control "negative control E3(a) (MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0)" \ - 'DirectGLES\.Split\.(SmallRing\.)?PersistentCoherentMapScenario' \ + 'DirectGLES\.Split\.(SmallRing\.)?PersistentCoherentMapScenario\.(TwoWritesThroughTheCoherentPointerEachReachTheirOwnDraw|AWriteAfterAFrameBoundaryReachesTheNextFramesDraw)$' \ "the SECOND write through the same mapping, announced by nothing|frame 1's write through the SAME mapping, after a Present" \ 'MGPipe: persistent-map push disabled - MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0' \ MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 diff --git a/scripts/ci/testdata/split_private_log_smoke.sh b/scripts/ci/testdata/split_private_log_smoke.sh index f52c802b..86fa8983 100644 --- a/scripts/ci/testdata/split_private_log_smoke.sh +++ b/scripts/ci/testdata/split_private_log_smoke.sh @@ -7,7 +7,7 @@ trap 'rm -rf "${WORK}"' EXIT cp "${HERE}/testdata/stub_ctest.sh" "${WORK}/ctest" chmod +x "${WORK}/ctest" passes=0 -for mode in missing-fatal stdout-fatal stale-fatal evidence e3-unrelated e3-no-private skipped-selection notrun-selection missing-selection partial-fatal wrong-fatal; do +for mode in missing-fatal stdout-fatal stale-fatal evidence e3-unrelated e3-no-private skipped-selection e3-skipped-selection notrun-selection missing-selection partial-fatal wrong-fatal; do mkdir -p "${WORK}/${mode}" rc=0 STUB_MODE="${mode}" CTEST="${WORK}/ctest" CONTROL_TMPDIR="${WORK}/${mode}" \ @@ -25,6 +25,7 @@ for mode in missing-fatal stdout-fatal stale-fatal evidence e3-unrelated e3-no-p case "${mode}" in e3-no-private) message='no selected private log carries /MGPipe: persistent-map push disabled' ;; skipped-selection) message='SplitLogPaths FAILED: E1 control: the knob killed the pre-flight, not the entry - 1 selected entries skipped' ;; + e3-skipped-selection) message='SplitLogPaths FAILED: E3(a) control: the knob killed the pre-flight, not the entry - 1 selected entries skipped' ;; notrun-selection|missing-selection) message='SplitLogPaths FAILED: E1 control: 1 selected entries did not run' ;; esac if [[ "${mode}" = *-selection ]]; then diff --git a/scripts/ci/testdata/stub_ctest.sh b/scripts/ci/testdata/stub_ctest.sh index ae2739ed..d98efdc6 100755 --- a/scripts/ci/testdata/stub_ctest.sh +++ b/scripts/ci/testdata/stub_ctest.sh @@ -24,7 +24,7 @@ # never says the push was disabled (the half ID-65 added) # green baseline green; the control's own run PASSES (the knob is not load-bearing) # red-baseline the baseline itself has a failed entry -# all-skipped the baseline is entirely skipped (the disarmed lane, a legitimate exit 0) +# all-skipped the baseline is entirely skipped (lost implementation, a hard failure) # retrace-noselect `ctest -N` matches nothing; the run exits 8 the way --no-tests=error does # retrace-unrelated one match; the run fails without naming the transport # retrace-evidence one match; the run fails with run_trace_case.cmake's own sentence @@ -76,10 +76,20 @@ write_junit() { entry=DirectGLES.Split.ClearThenReadPixelsScenario.ClearWithNoDrawIsVisibleToDefaultFramebufferReadPixels [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" != 0 ] || entry=DirectGLES.Split.PersistentCoherentMapScenario.TwoWritesThroughTheCoherentPointerEachReachTheirOwnDraw body="" + if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ]; then + case "${mode}" in + evidence|e3-no-private) + body="the SECOND write through the same mapping, announced by nothing" ;; + esac + fi if [ "${mode}" = partial-fatal ] && [ "${MOBILEGL_IPC_VERB_BARRIER:-1}" = 0 ]; then body="${body}" fi case "${mode}" in + e3-skipped-selection) + if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ]; then + body="" + fi ;; skipped-selection) body="" ;; notrun-selection) body="" ;; missing-selection) body='' ;; @@ -130,7 +140,7 @@ fi # The control's own run. if [ "${MOBILEGL_IPC_VERB_BARRIER:-1}" = 0 ]; then case "${mode}" in - evidence|e3-unrelated|e3-no-private|skipped-selection|notrun-selection|missing-selection|partial-fatal) echo 'Fatal{BarrierViolation, "DrawVbo"}' > "${log}" ;; + evidence|e3-unrelated|e3-no-private|skipped-selection|e3-skipped-selection|notrun-selection|missing-selection|partial-fatal) echo 'Fatal{BarrierViolation, "DrawVbo"}' > "${log}" ;; wrong-fatal) echo 'Fatal{ReplyMissing, "DrawVbo"}' > "${log}" ;; missing-fatal) echo "library setup only; no fatal" > "${log}" ;; stdout-fatal) echo 'Fatal{BarrierViolation, "DrawVbo"}' ;; @@ -144,6 +154,13 @@ if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ] && [ "${mode}" = evidence ] > "${CONTROL_TMPDIR}/pmap.log" fi case "${mode}" in + e3-skipped-selection) + if [ "${MOBILEGL_IPC_PERSISTENT_BLOCK_KB:-64}" = 0 ]; then + echo 'selected E3 entry ... ***Skipped' + exit 0 + fi + exit 8 + ;; skipped-selection|notrun-selection|missing-selection) echo '1/1 Test #1: selected entry ... ***Skipped' echo '100% tests passed, 0 tests failed out of 1' diff --git a/scripts/p3a_untouched_regions.sh b/scripts/p3a_untouched_regions.sh index 1f97bed7..98289664 100644 --- a/scripts/p3a_untouched_regions.sh +++ b/scripts/p3a_untouched_regions.sh @@ -532,7 +532,8 @@ if [ "${1:-}" = "--self-test" ]; then printf '%s %s\n' \ "0000000000000000000000000000000000000000000000000000000000000000" "$target" \ >> "$WORK_DIR/pinprec.sha" - apply_pinned_shas "$WORK_DIR/pinprec.sha" || exit 2 + # Drive production extraction on a real historical ref whose body differs from the pin. + extract_baseline ff2994d9 pinprec || exit 2 got=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pinprec.sha") if [ "$got" != "$pinned" ]; then say "PIN CONTROL FAILED: a baseline that carried a DIFFERENT sha for $target came out as" @@ -552,7 +553,7 @@ if [ "${1:-}" = "--self-test" ]; then python3 "$PY" extract "$WORK_DIR/pinperturbed.cpp" "$ALL_FUNCTIONS" \ > "$WORK_DIR/pinperturbed.sha" || exit 2 cp -f "$WORK_DIR/pristine.sha" "$WORK_DIR/pinbase.sha" || exit 2 - apply_pinned_shas "$WORK_DIR/pinbase.sha" || exit 2 + extract_baseline HEAD pinbase || exit 2 if compare_lists "$WORK_DIR/pinbase.sha" "$WORK_DIR/pinperturbed.sha" \ "PIN($PINNED_BASELINE_REF)" "one-token-perturbed" 2> "$WORK_DIR/pinperturbed.err"; then say "NEGATIVE CONTROL DID NOT TRIP: one token was inserted into $target's body and the" diff --git a/scripts/p4a_untouched_regions.sh b/scripts/p4a_untouched_regions.sh index fa23391a..928047e6 100644 --- a/scripts/p4a_untouched_regions.sh +++ b/scripts/p4a_untouched_regions.sh @@ -766,7 +766,8 @@ if [ "${1:-}" = "--self-test" ]; then printf '%s %s\n' \ "0000000000000000000000000000000000000000000000000000000000000000" "$target" \ >> "$WORK_DIR/pinprec.sha" - apply_pinned_shas "$WORK_DIR/pinprec.sha" || exit 2 + # Drive production extraction on a real historical ref whose body differs from the pin. + extract_baseline 37da3c3a pinprec || exit 2 got=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pinprec.sha") if [ "$got" != "$pinned" ]; then say "PIN CONTROL FAILED: a baseline that carried a DIFFERENT sha for $target came out as" @@ -789,7 +790,7 @@ if [ "${1:-}" = "--self-test" ]; then "$WORK_DIR/pinperturbed/$(blob_name "$targetSource")" token || exit 2 python3 "$PY" extract "$WORK_DIR/pinperturbed.spec" > "$WORK_DIR/pinperturbed.sha" || exit 2 cp -f "$WORK_DIR/pristine.sha" "$WORK_DIR/pinbase.sha" || exit 2 - apply_pinned_shas "$WORK_DIR/pinbase.sha" || exit 2 + extract_baseline HEAD pinbase || exit 2 if compare_lists "$WORK_DIR/pinbase.sha" "$WORK_DIR/pinperturbed.sha" \ "PIN($PINNED_BASELINE_REF)" "one-token-perturbed" 2> "$WORK_DIR/pinperturbed.err"; then say "NEGATIVE CONTROL DID NOT TRIP: one token was inserted into $target's body and the" From d50183cb1a9855d9d03b590676cd41dc8d17a005 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 12:34:33 -0400 Subject: [PATCH 2/2] [Fix] (IntegrationTest): assert actual staging backpressure in the small ring lane --- .../Harness/SplitRuntimePeek.cpp | 11 ++++++ .../Harness/SplitRuntimePeek.h | 7 ++-- .../Harness/WireLedgerChecks.h | 3 ++ .../Scenarios/TriangleScenario.cpp | 15 +++++++ MobileGL/MG_Remote/Client/ClientSession.cpp | 1 + MobileGL/MG_Remote/Server/ServerLoop.cpp | 1 + MobileGL/MG_Remote/Server/ServerLoop.h | 6 +++ MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 39 ++++++++++++------- MobileGL/MG_Remote/Wire/PipeWireCodec.h | 17 ++++---- MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 14 +++++++ 10 files changed, 88 insertions(+), 26 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp index 7cb8c851..81a3b992 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.cpp @@ -19,10 +19,21 @@ #include #include +#include +#include +#include #define MGITEST_SPLIT_RUNTIME_PEEK_LIVE 1 #endif namespace MGITest { + void DelaySplitRetirementForTesting(bool enabled) { +#if defined(MGITEST_SPLIT_RUNTIME_PEEK_LIVE) + MobileGL::MG_Remote::Server::ServerLoopInstance().SetBeforeRetireHookForTesting( + enabled ? +[] { std::this_thread::sleep_for(std::chrono::milliseconds(30)); } : nullptr); +#else + (void)enabled; +#endif + } SplitRuntimeState PeekSplitRuntime() { SplitRuntimeState state; diff --git a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h index 9451f78e..f4e7ad88 100644 --- a/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h +++ b/MobileGL/MG_IntegrationTest/Harness/SplitRuntimePeek.h @@ -91,9 +91,8 @@ namespace MGITest { // than asserted: a uniform record stride over a power-of-two ring lands on the // boundary exactly and never straddles it. // - // stageReclaimWaits: SEG_STAGE allocations that only fitted after the encoder reclaimed - // what the server had retired - P5's one real producer wait (see PipeWireCodec.h for - // why the command ring has none while the verb barrier is armed). + // stageReclaimWaits: allocations blocked after immediate reclamation failed; + // already-retired bytes reclaimed lazily do not count as a wait. unsigned long long maxRecordBytes = 0; unsigned long long maxRecordBytesCap = 0; unsigned long long cmdWraps = 0; @@ -103,6 +102,8 @@ namespace MGITest { }; SplitRuntimeState PeekSplitRuntime(); + // Scheduling-only perturbation; never changes a watermark or counter. + void DelaySplitRetirementForTesting(bool enabled); // Empty when this process is a real split run that can be asserted about; otherwise the // reason to GTEST_SKIP() with, naming the first fact that is not true and the package that diff --git a/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h b/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h index 1c5eb512..b14d11af 100644 --- a/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h +++ b/MobileGL/MG_IntegrationTest/Harness/WireLedgerChecks.h @@ -134,6 +134,9 @@ namespace MGITest::WireLedger { "kSmallRingLaneCmdByteTarget, which is sized for the 1 MiB ring this lane " "declares (MGL_ITEST_GLES_SPLIT_SMALL_RING_ENVIRONMENT); a larger ring needs a " "larger workload and is not what this lane is for"; + EXPECT_GE(state.stageReclaimWaits, 1u) + << where << ": exit gate E3(e) - producer NEVER WAITED for staging retirement; " + "lazy reclamation of already-retired bytes is not back-pressure"; ::testing::Test::RecordProperty("ring_wraps", static_cast(state.cmdWraps)); ::testing::Test::RecordProperty("ring_wrap_pads", static_cast(state.cmdWrapPads)); ::testing::Test::RecordProperty("ring_waits", static_cast(state.stageReclaimWaits)); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp index 520615c0..490822b4 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/TriangleScenario.cpp @@ -264,6 +264,21 @@ void main() { oColor = vec4(vColor, 1.0); } // the lane is that IT is the arm with a ring the workload can fill. if (SplitLane::IsSmallRingLane()) { const unsigned long long driven = DriveUntilSmallRingOverruns(); + // Ordinary uploads, each fitting by itself, jointly exceed the lane's 1 MiB + // staging segment. Delay only scheduling between applied and retired: the + // production allocator, not this test, must observe capacity and wait. + GLuint pressureBuffer = 0; + glGenBuffers(1, &pressureBuffer); + glBindBuffer(GL_COPY_WRITE_BUFFER, pressureBuffer); + std::vector upload(768 * 1024, 0x5a); + glBufferData(GL_COPY_WRITE_BUFFER, upload.size(), nullptr, GL_DYNAMIC_DRAW); + DelaySplitRetirementForTesting(true); + glBufferSubData(GL_COPY_WRITE_BUFFER, 0, upload.size(), upload.data()); + upload[0] = 0xa5; + glBufferSubData(GL_COPY_WRITE_BUFFER, 0, upload.size(), upload.data()); + DelaySplitRetirementForTesting(false); + glBindBuffer(GL_COPY_WRITE_BUFFER, 0); + glDeleteBuffers(1, &pressureBuffer); Gl().EndFrame(); WireLedger::ExpectSmallRingWrappedAtLeastOnce( "TriangleScenario.TheSameVboAndVaoRedrawAcrossAFrameBoundary", driven); diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index bc0be7d4..72c478fb 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -516,6 +516,7 @@ namespace MobileGL::MG_Remote::Client { // a live RingProducer over a cursor triple nobody consumes would be the // half-wired shape this session exists not to have. m_encoder = Wire::PipeWireEncoder(control, &m_cmd, nullptr, &m_segments); + m_encoder.SetStageRetirementDoorbell(m_producer.SelfDoorbell()); // ---- 7. the first CapsSnapshot, if the server had a backend to publish one from. // ONE DRAIN, ONE ADOPTER (c1): PumpControlPlane below is the only thing in the client diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index 6047769f..3ac42680 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -530,6 +530,7 @@ namespace MobileGL::MG_Remote::Server { // a loop that applies and never retires ends the first MOBILEGL_IPC_STAGE_MB of // staging in Fatal{RingOverrun, "SEG_STAGE"} (w1-v1 5). Once per drain batch, not // once per record: retiring LATE is always legal, retiring EARLY never is. + if (const auto hook = m_beforeRetireHook.load(std::memory_order_acquire)) hook(); consumer.RetireThrough(consumer.AppliedSeq()); // LEAVING THE APPLIER (p1's M-5) IS *NOT* DONE HERE. It is done inside // PipeApplier::ApplyOne, before s1's SessionConsumer::ApplyOne publishes appliedSeq diff --git a/MobileGL/MG_Remote/Server/ServerLoop.h b/MobileGL/MG_Remote/Server/ServerLoop.h index 628d9ce8..4681ec93 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.h +++ b/MobileGL/MG_Remote/Server/ServerLoop.h @@ -146,6 +146,11 @@ namespace MobileGL::MG_Remote::Server { // fail for its own reason. Uint64 DrainedRecords() const; Uint64 ParkCount() const; + // Scheduling perturbation only: the hook runs after application, before retirement. + // Integration tests use it to observe real producer back-pressure from GL uploads. + void SetBeforeRetireHookForTesting(void (*hook)()) { + m_beforeRetireHook.store(hook, std::memory_order_release); + } // C7 / ID-54 diagnostics, read by ServerLoopTest's C7 and N-3 controls. NativeBindCount is // how many times ApplyMakeCurrent FORWARDED a bind to the backend (a tuple it did not @@ -230,6 +235,7 @@ namespace MobileGL::MG_Remote::Server { Uint64 m_affinityMask = 0; std::atomic m_drained{0}; std::atomic m_parks{0}; + std::atomic m_beforeRetireHook{nullptr}; // C7 / ID-54: the (dpy, draw, read, ctx) currently bound on the apply thread. Written and // read ONLY on the apply thread inside ApplyMakeCurrent, so it needs no lock; the two diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 3b940914..20974dd8 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -776,10 +776,12 @@ namespace MobileGL::MG_Remote::Wire { std::abort(); } - for (int attempt = 0; attempt < 2; ++attempt) { + bool reclaimed = false; + bool waited = false; + for (;;) { // THE WRAP SKIP MAY ONLY BE CHARGED AGAINST BYTES THAT ARE STILL IN FLIGHT. When // there are none the allocator starts over at offset zero, so a blob the segment - // can hold whole is never refused (see RebaseEmptyStage). On attempt 1 this runs + // can hold whole is never refused (see RebaseEmptyStage). On retry this runs // AFTER ReclaimStagedBytes, which is the case the finding describes: 8 MiB // allocated, then retired, then a 28 MiB request that used to abort. RebaseEmptyStage(); @@ -793,21 +795,28 @@ namespace MobileGL::MG_Remote::Wire { m_stageHead += skip + need; return m_stageBase + at; } - if (attempt == 0) { - // One try at reclaiming what the server has already retired. A second failure - // means the bytes genuinely do not fit, which R-10 says P5 does not chunk and - // must instead prove it never needs to. - // - // AND THIS IS P5'S ONE REAL BACK-PRESSURE EVENT, so it is counted here and - // published as `ringwaits=`. Reaching this line means the producer could not - // place a blob until the CONSUMER had retired earlier ones - the producer's - // progress depended on retiredSeq, which is exactly what R-9's "batching may - // only delay a watermark" is about. Exit gate E3(e)'s small-ring lane exists - // to make it happen at least once; a lane that never reaches it has a ring - // that is small only in its environment block. - ++m_stageReclaimWaits; + if (!reclaimed) { + // Lazy reclamation of already-retired bytes is NOT a producer wait. ReclaimStagedBytes(); + reclaimed = true; + continue; } + if (m_stageRetirementBell == nullptr || m_stageMarkFront == m_stageMarks.size()) break; + const Uint64 pending = m_stageMarks[m_stageMarkFront].Seq; + const auto ready = [&] { + return m_control->retiredSeq.load(std::memory_order_acquire) >= pending; + }; + if (!ready()) { + // The allocation still cannot progress after reclamation. Count this + // blocked allocation once, not each watermark poll or each reclaimed mark. + if (!waited) { ++m_stageReclaimWaits; waited = true; } + if (!m_stageRetirementBell->Wait(m_control->producerParked, ready, 0, 5000)) { + MGLOG_F("MGPipe: Fatal{RetirementWaitFailed, \"SEG_STAGE\"} producer wait " + "ended before the pending allocation retired (shutdown or timeout)"); + std::abort(); + } + } + ReclaimStagedBytes(); } MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_STAGE\"} a %llu byte blob does not fit a %llu " "byte staging segment with %llu bytes still in flight (retiredSeq=%llu); P5 " diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index c5005c7c..5cb7b3c8 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -44,6 +44,7 @@ #include #include "../Transport/Ring.h" +#include "../Transport/Doorbell.h" namespace MobileGL::MG_Remote::Wire { @@ -311,17 +312,16 @@ namespace MobileGL::MG_Remote::Wire { // this number would have been red for the arithmetic of the record catalogue rather // than for anything about the ring. // - // `StageReclaimWaits()` counts every SEG_STAGE allocation that did not fit until the - // encoder reclaimed the runs the server had already retired - i.e. every time the - // producer's progress depended on the consumer's retiredSeq. That is the honest - // back-pressure reading in P5, and the reason the command ring has none: the verb - // barrier makes EmitAndWait wait for appliedSeq after EVERY record (R-1), so at most - // one record is ever in flight on SEG_CMD and a full command ring is not a wait but a - // Fatal{RingOverrun} (ClientSession.cpp). Publishing a "command ring waits" counter - // that can only ever be zero-or-dead is the decoration this file's counters are not. + // `StageReclaimWaits()` counts allocations blocked on an outstanding retiredSeq + // after immediate reclamation still left insufficient space. Reclaiming bytes + // the consumer had already retired does not increment it. One allocation counts + // once even if it waits for several marks; this is staging, not command-ring pressure. Uint64 CmdWraps() const; Uint64 CmdWrapPads() const; Uint64 StageReclaimWaits() const; + // The live session supplies its shutdown-aware producer doorbell. Standalone codecs + // without a consumer cannot wait for retirement and retain the named refusal. + void SetStageRetirementDoorbell(Transport::Doorbell* bell) { m_stageRetirementBell = bell; } // Bytes this encoder has ever written into SEG_CMD, pad fillers included: the // producer's monotonic head cursor. It is the DENOMINATOR the wrap count only means @@ -365,6 +365,7 @@ namespace MobileGL::MG_Remote::Wire { Uint64 m_cmdWraps = 0; Uint64 m_cmdWrapPads = 0; Uint64 m_stageReclaimWaits = 0; + Transport::Doorbell* m_stageRetirementBell = nullptr; Vector m_stageMarks; SizeT m_stageMarkFront = 0; Uint8* m_stageBase = nullptr; diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 3bc60de6..c1d212ad 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1595,6 +1595,20 @@ TEST_F(PipeWireCodecTest, ABigProgramArchiveDoesNotGrowItsRecordAtAll) { EXPECT_GE(wire.Encoder().StagedBytesInFlight(), archive.size()); } +TEST_F(PipeWireCodecTest, AlreadyRetiredStagingReclamationIsNotAProducerWait) { + Wire2 wire; + std::vector payload(Wire2::kStageBytes * 3 / 4, 0x5a); + wire.Encoder().StageBytes(payload.data(), payload.size()); + MGPBindRenderState bind{}; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindRenderState, &bind, sizeof(bind)), kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + // Do not explicitly reclaim: the second real allocation must do that itself. + wire.Encoder().StageBytes(payload.data(), payload.size()); + EXPECT_EQ(wire.Encoder().StageReclaimWaits(), 0u) + << "already-retired lazy reclamation is not a producer wait"; +} + TEST_F(PipeWireCodecTest, StagedBytesAreReclaimedOnlyBehindRetiredSeq) { Wire2 wire; const std::uint8_t payload[64] = {};