diff --git a/.github/workflows/community-reconciliation92.yml b/.github/workflows/community-reconciliation92.yml index 3e7991db91..0ca586cbc7 100644 --- a/.github/workflows/community-reconciliation92.yml +++ b/.github/workflows/community-reconciliation92.yml @@ -9,6 +9,8 @@ on: - 'scripts/run_parallel.py' - 'scripts/verify_parallel.py' - 'tests/test_parallel_verification.py' + - 'tests/test_script_exit_cleanup.py' + - 'tests/test_script_cancellation.py' - '.github/workflows/community-reconciliation92.yml' pull_request: paths: @@ -19,6 +21,8 @@ on: - 'scripts/run_parallel.py' - 'scripts/verify_parallel.py' - 'tests/test_parallel_verification.py' + - 'tests/test_script_exit_cleanup.py' + - 'tests/test_script_cancellation.py' - '.github/workflows/community-reconciliation92.yml' workflow_dispatch: permissions: @@ -35,3 +39,5 @@ jobs: run: python3 -B scripts/replay_historical_check.py reconciliation92 - name: Check worker isolation and rejection controls run: python3 -m unittest discover -s tests -p test_parallel_verification.py -v + - name: Check direct-script completion and cancellation cleanup + run: python3 -W error -B -m unittest discover -s tests -p 'test_script_*.py' -v diff --git a/docs/research/script-runner-exit-cleanup.md b/docs/research/script-runner-exit-cleanup.md new file mode 100644 index 0000000000..705eea1742 --- /dev/null +++ b/docs/research/script-runner-exit-cleanup.md @@ -0,0 +1,96 @@ +# Direct-script completion and descendant cleanup + +This is a verifier-engineering correction based on canonical main +`20ecf2891d80b5acf18d980b2164fb44ea7faa90` (priority revision 6). The portable-replay +scope is narrowed to the lifetime of ordinary POSIX descendants launched by +`scripts/run_parallel.py`. It makes no mathematical or selected-result claim. + +A script can start a solver with its output redirected to `DEVNULL` and exit +successfully while the solver continues running. Before this correction, the +real CLI printed `PASS all 1 scripts` and returned; a finite child then wrote an +observation file after the runner returned. A failed leader similarly left its +ordinary descendants running. The cancellation path already owned and stopped +active groups, but the normal-completion path discarded that ownership without +stopping the group. + +After collecting a leader's output, the worker now stops its POSIX group before +unregistering it. The existing registration lock also serializes this operation +with the controller's cancellation snapshot: whichever path acquires ownership +performs cleanup. A cleanup error retains registration for the controlling +cleanup path and prevents a success report. Existing output blocks and failure +labels are preserved. Other process groups are not signaled. + +This builds on [ywz65's direct-script cancellation correction, PR2676](https://github.com/CrocSwap/integer-mult-bounds/pull/2676), +its independent cancellation audit [PR2777](https://github.com/CrocSwap/integer-mult-bounds/pull/2777), +and the existing isolated-test runner's completed-leader cleanup. The new tests +exercise the ordinary successful and failed leader exits that the cancellation +fixtures did not cover. Substantial OpenAI Codex assistance was used in preparing +this contribution for roger-sato; Apache-2.0. + +## Reproduce and verify + +The five regressions invoke the actual CLI using disposable source fixtures. +Ordinary children and grandchildren close Unix-domain observer connections when +terminated; an unrelated process in its own group must remain alive. The tests +cover success, failure, an already-joined tree, a parent-only cleanup mutant, and +an injected group-signal error. The parent-only mutant must fail the lifetime +observation, and the signal error must produce a nonzero result with both the +original error and the cleanup-retry diagnostic. + +Run from the repository root with CPython 3.12: + +```sh +python3.12 -W error -B -m unittest discover -s tests -p 'test_script_exit_cleanup.py' -v +python3.12 -O -W error -B -m unittest discover -s tests -p 'test_script_exit_cleanup.py' -v +python3.12 -W error -B -m unittest discover -s tests -p 'test_script_*.py' -v +python3.12 -W error -B - <<'PY' +import unittest +patterns = ['test_script_cancellation.py', 'test_parallel_verification.py', + 'test_parallel_assertions.py', 'test_parallel_memory.py'] +suite = unittest.TestSuite(unittest.defaultTestLoader.discover('tests', pattern=p) + for p in patterns) +result = unittest.TextTestRunner(verbosity=2).run(suite) +raise SystemExit(not result.wasSuccessful()) +PY +``` + +Local validation used macOS arm64, CPython 3.12.14 and Git 2.54.0. Both focused +runs passed all five tests. On the original runner, the same suite fails three +tests; the joined-tree control and mutant-sensitivity control pass. The combined +script suite passes with nine existing Linux/Windows observer tests skipped on +macOS. The four existing runner modules run 30 tests: 20 pass and ten skip +(those nine observers plus the Windows memory API smoke test). The Ubuntu 3.12 +reconciliation workflow now explicitly runs the combined script suite. + +## Historical compatibility and limits + +The reconciliation92 manifest and frozen runner remain unchanged. The existing +historical adapter admits only the exact new live runner SHA-256 +`3e27934d4c5185e44bf88e0bc51cdb0af97286e033feb78494f071cd381b1e35`, while still replaying +the authenticated historical runner +`f046e98e4c296fde954b93b466b7f359be0c05c287d96943e016f0b850587387`. +An independent run of the actual default command in its complete 383-file input +closure passed all 92 pinned trees and 4,564 distinct blobs. An unknown live +runner and a corrupted frozen runner both reject; all original inputs remain +unchanged. + +```sh +python3.12 -B scripts/replay_historical_check.py reconciliation92 +python3.12 -B scripts/research_package.py --package research/isolated-test-bytecode +python3.12 -B scripts/research_package.py --package research/isolated-descendant-bytecode +python3.12 -B scripts/research_package.py --package research/historical-snapshot-one-pass +``` + +Both bytecode packages pass every registered command (their declared outcomes +remain `completed` and `partial`, respectively). The snapshot package has a +known baseline failure: its 27 tests yield one failure and one error because the +tooling-ledger adapter does not admit the current `docs/ci-verification.md` hash +`a3711fe12d560fac7726c530b9c8c622182f3d4f231a876c163616e49bd911bc`. +The identical result was reproduced through the registered CLI in an 82-file +closure containing only exact canonical-main bytes. That unrelated admission +list is unchanged by this contribution. + +Native Windows lifetime behavior was not changed or verified. Deliberately +detached process groups remain outside the owned-group contract. This change +does not add execution deadlines or require ordinary descendants to finish +their work successfully: it stops them when their owning script finishes. diff --git a/scripts/replay_historical_check.py b/scripts/replay_historical_check.py index 4bb61eae10..a3f4400ae5 100755 --- a/scripts/replay_historical_check.py +++ b/scripts/replay_historical_check.py @@ -107,6 +107,8 @@ 'sha256': 'f046e98e4c296fde954b93b466b7f359be0c05c287d96943e016f0b850587387', 'accepted_live_sha256': [ 'a49e763573343a6a068b0c48c8ff422756be5a8f55902b2385fa8d13cc951fda', + # Direct-script POSIX descendants are stopped after leader completion. + '3e27934d4c5185e44bf88e0bc51cdb0af97286e033feb78494f071cd381b1e35', ], }] diff --git a/scripts/run_parallel.py b/scripts/run_parallel.py index c37d931f50..d911a2b9d8 100644 --- a/scripts/run_parallel.py +++ b/scripts/run_parallel.py @@ -116,6 +116,8 @@ def run_all(commands, cwd=None): Controlling exceptions stop active ordinary process trees and prevent queued launches. Normal completion retains one output block per script. + POSIX groups are also stopped after normal leader exit, before releasing + ownership; closed capture streams do not imply that every solver exited. """ # Inherited optimization must not erase assertions in verification scripts # or their Python children. Deliberate -O/-OO argv controls remain explicit. @@ -153,6 +155,11 @@ def run(label, argv): # Unexpected output errors leave the process owned by the # controlling cleanup path, even after this worker returns. if finished: + # Serialize normal cleanup with cancellation's snapshot: + # exactly one owner may stop this completed process group. + # Windows exited leaders need a separate Job Object design. + if not stopping and os.name != 'nt': + stop_script(process) active.discard(process) failed = [] diff --git a/tests/test_script_exit_cleanup.py b/tests/test_script_exit_cleanup.py new file mode 100644 index 0000000000..450e224ead --- /dev/null +++ b/tests/test_script_exit_cleanup.py @@ -0,0 +1,160 @@ +"""Direct script completion must release its ordinary POSIX descendants. + +Builds on ywz65's PR2676 cancellation boundary and the isolated-test exit +controls. Prepared for roger-sato with substantial OpenAI Codex assistance. +Apache-2.0. No native Windows lifetime claim. +""" +import os +from pathlib import Path +import signal +import socket +import subprocess +import sys +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] + +SOLVER = '''import socket, subprocess, sys +from pathlib import Path +root, role = Path(sys.argv[1]), sys.argv[2] +child = None +if role == 'c': + child = subprocess.Popen([sys.executable, __file__, str(root), 'g'], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) +with socket.socket(socket.AF_UNIX) as connection: + connection.settimeout(20) + connection.connect(str(root / 'observer')) + connection.sendall(role.encode('ascii')) + connection.recv(1) +if child is not None: + child.wait(timeout=3) +''' + +LEADER = '''import os, subprocess, sys, time +from pathlib import Path +root = Path(__file__).resolve().parent +(root / 'leader.pid').write_text(str(os.getpid())) +child = subprocess.Popen([sys.executable, str(root / 'solver.py'), str(root), 'c'], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) +deadline = time.monotonic() + 15 +while not (root / 'leader-release').exists(): + if time.monotonic() >= deadline: + raise SystemExit('fixture release timeout') + time.sleep(.01) +if (root / 'join').exists(): + child.wait(timeout=3) +print('literal leader output') +raise SystemExit(7 if (root / 'fail').exists() else 0) +''' + + +@unittest.skipUnless(os.name == 'posix' and hasattr(socket, 'AF_UNIX'), + 'requires POSIX process groups and Unix-domain sockets') +class ScriptExitCleanupTests(unittest.TestCase): + def exercise(self, failed=False, joined=False, mutation=None): + connections = {} + runner = unrelated = None + quiet = False + with tempfile.TemporaryDirectory(prefix='script-exit-') as directory: + root = Path(directory) + source = (ROOT / 'scripts/run_parallel.py').read_text(encoding='utf-8') + if mutation is not None: + original = 'os.killpg(process.pid, signal.SIGKILL)' + self.assertEqual(source.count(original), 1) + source = source.replace(original, mutation) + (root / 'run_parallel.py').write_text(source, encoding='utf-8') + (root / 'solver.py').write_text(SOLVER, encoding='utf-8') + (root / 'leader.py').write_text(LEADER, encoding='utf-8') + if failed: + (root / 'fail').touch() + if joined: + (root / 'join').touch() + with socket.socket(socket.AF_UNIX) as observer: + observer.settimeout(8) + observer.bind(str(root / 'observer')) + observer.listen(3) + try: + unrelated = subprocess.Popen( + [sys.executable, str(root / 'solver.py'), str(root), 'u'], + start_new_session=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + flags = ['-O'] if sys.flags.optimize else [] + runner = subprocess.Popen( + [sys.executable, *flags, '-B', str(root / 'run_parallel.py'), 'leader.py'], + cwd=root, env=dict(os.environ, JOBS='1'), + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + for _ in range(3): + connection, _ = observer.accept() + connection.settimeout(8) + role = connection.recv(1).decode('ascii') + self.assertNotIn(role, connections) + connections[role] = connection + self.assertEqual(set(connections), {'c', 'g', 'u'}) + if joined: + for role in ('g', 'c'): + connections[role].sendall(b'x') + (root / 'leader-release').touch() + out, err = runner.communicate(timeout=8) + if mutation and 'PermissionError' in mutation: + self.assertNotEqual(runner.returncode, 0) + self.assertIn(b'injected cleanup denial', err) + self.assertIn(b'Parallel script cleanup failed', err) + self.assertNotIn(b'PASS all', out) + else: + self.assertEqual(runner.returncode, 1 if failed else 0, err.decode()) + self.assertIn(b'literal leader output\n', out) + self.assertIn(b'FAIL leader.py' if failed else b'Checked leader.py', out) + if failed: + self.assertIn(b'Failed: leader.py', err) + self.assertNotIn(b'PASS all', out) + else: + self.assertIn(b'PASS all 1 scripts', out) + for role in ('c', 'g'): + connections[role].settimeout(1) + try: + ended = connections[role].recv(1) == b'' + except TimeoutError: + ended = False + self.assertTrue(ended, 'ordinary descendant survived runner completion: ' + role) + quiet = True + self.assertIsNone(unrelated.poll(), 'cleanup killed an unrelated process') + connections['u'].sendall(b'x') + self.assertEqual(unrelated.wait(timeout=3), 0) + finally: + # A retained connection proves the fixture process has not + # finished; kill only its still-owned process group. + if not quiet and (root / 'leader.pid').exists(): + try: + os.killpg(int((root / 'leader.pid').read_text()), signal.SIGKILL) + except ProcessLookupError: + pass + for connection in connections.values(): + connection.close() + if runner is not None: + if runner.poll() is None: + runner.kill() + runner.communicate(timeout=5) + if unrelated is not None: + if unrelated.poll() is None: + unrelated.kill() + unrelated.wait(timeout=5) + + def test_successful_leader_cleans_child_and_grandchild(self): + self.exercise() + + def test_failed_leader_keeps_failure_and_cleans_descendants(self): + self.exercise(failed=True) + + def test_joined_tree_preserves_success_and_output(self): + self.exercise(joined=True) + + def test_parent_only_cleanup_mutant_is_detected(self): + with self.assertRaisesRegex(AssertionError, 'ordinary descendant survived'): + self.exercise(mutation='pass') + + def test_cleanup_denial_prevents_success_and_reports_retry_failure(self): + self.exercise(mutation="raise PermissionError('injected cleanup denial')") + + +if __name__ == '__main__': + unittest.main()