diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 869d922c09..cd310d3d04 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -653,13 +653,18 @@ For a responsive **Linux systemd / Proxmox LXC** install, take this small, read-only sample at a normally occurring high point. Run it **inside the Pulse container** for LXC, not on the Proxmox host. Substitute the actual service name (`pulse-backend` on older installs) in both `systemctl` calls. Use an account -authorised to read these counters; a failed read is unavailable, not zero. +already authorised to read these counters. The collector does not elevate +privileges or prompt; do not add `sudo` inside it. A failed read is unavailable, +not zero. ```bash # systemd / Proxmox LXC: Pulse resident-memory sample ( set -eu - if timeout --kill-after=1s 8s bash <<'PULSE_MEMORY_SAMPLE' + command -v timeout >/dev/null 2>&1 || { + printf 'Resident-memory sample unavailable: GNU timeout is required.\n' >&2; exit 1 + } + if sample=$(timeout --signal=KILL 8s bash <<'PULSE_MEMORY_SAMPLE' 2>/dev/null set -eu date -u +'%Y-%m-%dT%H:%M:%SZ' pid=$(systemctl show pulse --property=MainPID --value) @@ -685,23 +690,24 @@ if [ "$pid" != "$current_pid" ] || [ "$identity" != "$current_identity" ]; then fi printf 'Pulse process (PID and UTC start): %s\n%s\n' "$identity" "$counters" PULSE_MEMORY_SAMPLE - then - : + ); then + printf '%s\n' "$sample" else status=$? - printf 'Resident-memory sample unavailable; retain the error and stop sampling.\n' >&2 + printf 'Resident-memory sample unavailable (exit %s); stop sampling.\n' "$status" >&2 exit "$status" fi ) ``` The recipe requires GNU `timeout`: it gives the read-only sampling commands -eight seconds, with a one-second termination grace, rather than leaving a hung -reader running indefinitely. It does not stop or restart Pulse. A missing +eight seconds, then kills its own collection group, including readers that +ignore TERM. Waiting for the collection shell alone can leave a reader running +after it exits. This hard deadline does not stop or restart Pulse. A missing utility, timeout or reader error means the sample is unavailable; keep the failure, and do not remove the deadline or rerun it against an unresponsive -installation. No complete process/counter sample is printed before every check -succeeds; the initial UTC timestamp alone is not a memory result. +installation. No timestamp, process identity, counter or raw reader error is +printed unless every check succeeds; the failure reports only its exit status. Linux labels these values `kB`, meaning 1,024 bytes; divide by 1,024 for MiB. `VmRSS` is resident process memory, split into anonymous (`RssAnon`), diff --git a/frontend-modern/public/docs/TROUBLESHOOTING.md b/frontend-modern/public/docs/TROUBLESHOOTING.md index 869d922c09..cd310d3d04 100644 --- a/frontend-modern/public/docs/TROUBLESHOOTING.md +++ b/frontend-modern/public/docs/TROUBLESHOOTING.md @@ -653,13 +653,18 @@ For a responsive **Linux systemd / Proxmox LXC** install, take this small, read-only sample at a normally occurring high point. Run it **inside the Pulse container** for LXC, not on the Proxmox host. Substitute the actual service name (`pulse-backend` on older installs) in both `systemctl` calls. Use an account -authorised to read these counters; a failed read is unavailable, not zero. +already authorised to read these counters. The collector does not elevate +privileges or prompt; do not add `sudo` inside it. A failed read is unavailable, +not zero. ```bash # systemd / Proxmox LXC: Pulse resident-memory sample ( set -eu - if timeout --kill-after=1s 8s bash <<'PULSE_MEMORY_SAMPLE' + command -v timeout >/dev/null 2>&1 || { + printf 'Resident-memory sample unavailable: GNU timeout is required.\n' >&2; exit 1 + } + if sample=$(timeout --signal=KILL 8s bash <<'PULSE_MEMORY_SAMPLE' 2>/dev/null set -eu date -u +'%Y-%m-%dT%H:%M:%SZ' pid=$(systemctl show pulse --property=MainPID --value) @@ -685,23 +690,24 @@ if [ "$pid" != "$current_pid" ] || [ "$identity" != "$current_identity" ]; then fi printf 'Pulse process (PID and UTC start): %s\n%s\n' "$identity" "$counters" PULSE_MEMORY_SAMPLE - then - : + ); then + printf '%s\n' "$sample" else status=$? - printf 'Resident-memory sample unavailable; retain the error and stop sampling.\n' >&2 + printf 'Resident-memory sample unavailable (exit %s); stop sampling.\n' "$status" >&2 exit "$status" fi ) ``` The recipe requires GNU `timeout`: it gives the read-only sampling commands -eight seconds, with a one-second termination grace, rather than leaving a hung -reader running indefinitely. It does not stop or restart Pulse. A missing +eight seconds, then kills its own collection group, including readers that +ignore TERM. Waiting for the collection shell alone can leave a reader running +after it exits. This hard deadline does not stop or restart Pulse. A missing utility, timeout or reader error means the sample is unavailable; keep the failure, and do not remove the deadline or rerun it against an unresponsive -installation. No complete process/counter sample is printed before every check -succeeds; the initial UTC timestamp alone is not a memory result. +installation. No timestamp, process identity, counter or raw reader error is +printed unless every check succeeds; the failure reports only its exit status. Linux labels these values `kB`, meaning 1,024 bytes; divide by 1,024 for MiB. `VmRSS` is resident process memory, split into anonymous (`RssAnon`), diff --git a/scripts/tests/test_memory_troubleshooting_docs.py b/scripts/tests/test_memory_troubleshooting_docs.py index 8657db57e5..19e7bca90f 100644 --- a/scripts/tests/test_memory_troubleshooting_docs.py +++ b/scripts/tests/test_memory_troubleshooting_docs.py @@ -12,6 +12,7 @@ import json import os from pathlib import Path import re +import select import shutil import signal import subprocess @@ -41,7 +42,7 @@ def recipe() -> str: READER = '''#!/usr/bin/env python3 -import json, os, pathlib, subprocess, sys, time +import json, os, pathlib, signal, subprocess, sys, time name = pathlib.Path(sys.argv[0]).name args = sys.argv[1:] root = pathlib.Path(os.environ['RECIPE_FIXTURE']) @@ -51,7 +52,14 @@ count = sum(call['name'] == name for call in previous) with log.open('a') as stream: stream.write(json.dumps({'name': name, 'args': args}) + '\\n') if name == os.environ.get('HANG_TOOL'): - time.sleep(30) + if os.environ.get('IGNORE_TERM'): + signal.signal(signal.SIGTERM, signal.SIG_IGN) + record = root / 'hung-reader.tmp' + record.write_text(json.dumps({'pid': os.getpid()})) + record.replace(root / 'hung-reader.json') + if os.environ.get('CLOSE_PIPES'): + os.close(1); os.close(2) + time.sleep(100) if name == 'date': assert args == ['-u', '+%Y-%m-%dT%H:%M:%SZ'] print('2026-10-06T00:00:00Z') @@ -66,6 +74,10 @@ elif name == 'ps': sys.exit(int(os.environ.get(f'PROCESS_EXIT_{count}', '0'))) elif name == 'awk': assert args[-1] == '/proc/4321/status' + if os.environ.get('AWK_ERROR'): + print('VmRSS: 8192 kB') + print('private fixture reader error', file=sys.stderr) + sys.exit(3) target = root / ('missing' if os.environ.get('STATUS_MISSING') else 'status') sys.exit(subprocess.run([os.environ['REAL_AWK'], *args[:-1], str(target)]).returncode) else: @@ -74,21 +86,51 @@ else: def run_owned_recipe(copied: str, env: dict[str, str]) -> subprocess.CompletedProcess: - """Keep the existing ten-second assertion; clean up our shell on every exit.""" + """Keep the ten-second budget and observe actual owned-reader termination. + + A pidfd binds observation and emergency cleanup to the fixture process, + even if its pipes close or its PID disappears. No service is signalled. + """ with subprocess.Popen(["bash", "-c", copied], env=env, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, start_new_session=True) as process: + reader_fd = None + reader_record_missing = False try: + if env.get("HANG_TOOL"): + record = Path(env["RECIPE_FIXTURE"]) / "hung-reader.json" + deadline = time.monotonic() + 2 + while not record.exists() and process.poll() is None and time.monotonic() < deadline: + time.sleep(0.01) + if record.exists(): + reader_fd = os.pidfd_open(json.loads(record.read_text())["pid"]) + reader_exit = select.poll() + reader_exit.register(reader_fd, select.POLLIN) + else: + reader_record_missing = True stdout, stderr = process.communicate(timeout=10) + if reader_record_missing: + raise AssertionError("owned hung-reader identity unobserved") + if reader_fd is not None and not reader_exit.poll(500): + raise AssertionError("owned reader survived the collection deadline") except subprocess.TimeoutExpired: # Never target a service or another test's group. TimeoutExpired # remains an error, not a passing or empty sample. + if reader_fd is not None and not reader_exit.poll(0): + signal.pidfd_send_signal(reader_fd, signal.SIGKILL) try: os.killpg(process.pid, signal.SIGKILL) except ProcessLookupError: pass process.communicate(timeout=5) raise + finally: + if reader_fd is not None: + if not reader_exit.poll(0): + signal.pidfd_send_signal(reader_fd, signal.SIGKILL) + if not reader_exit.poll(1000): + raise AssertionError("test-owned reader cleanup unobserved") + os.close(reader_fd) return subprocess.CompletedProcess(process.args, process.returncode, stdout, stderr) @@ -111,6 +153,7 @@ class MemoryTroubleshootingDocsTest(unittest.TestCase): def assert_no_memory_claim(self, result): self.assertNotEqual(result.returncode, 0) + self.assertEqual(result.stdout, "") self.assertNotIn("Pulse process (PID and UTC start)", result.stdout) self.assertNotRegex(result.stdout, r"VmRSS:|RssAnon:|RssFile:|RssShmem:|VmSwap:") @@ -156,7 +199,7 @@ class MemoryTroubleshootingDocsTest(unittest.TestCase): def test_empty_initial_identity_is_not_a_complete_sample(self): result, calls = self.exercise(IDENTITY_0="") self.assert_no_memory_claim(result) - self.assertIn("identity unavailable", result.stderr) + self.assertIn("sample unavailable", result.stderr) self.assertNotIn("awk", [call["name"] for call in calls]) def test_missing_unreadable_or_incomplete_status_does_not_print_partial_counters(self): @@ -183,7 +226,7 @@ class MemoryTroubleshootingDocsTest(unittest.TestCase): with self.subTest(settings=settings): result, _ = self.exercise(**settings) self.assert_no_memory_claim(result) - self.assertIn("discard this sample", result.stderr) + self.assertIn("sample unavailable", result.stderr) def test_measured_zero_is_retained_without_interpreting_missing_as_zero(self): result, _ = self.exercise(status=re.sub(r"\d+ kB", "0 kB", COUNTERS)) @@ -194,7 +237,7 @@ class MemoryTroubleshootingDocsTest(unittest.TestCase): started = time.monotonic() result, calls = self.exercise(HANG_TOOL="systemctl") self.assertLess(time.monotonic() - started, 10) - self.assertEqual(result.returncode, 124, result.stderr) + self.assertEqual(result.returncode, 137, result.stderr) self.assert_no_memory_claim(result) self.assertIn("sample unavailable", result.stderr) self.assertEqual([call["name"] for call in calls], ["date", "systemctl"]) @@ -203,21 +246,42 @@ class MemoryTroubleshootingDocsTest(unittest.TestCase): started = time.monotonic() result, calls = self.exercise(HANG_TOOL="awk") self.assertLess(time.monotonic() - started, 10) - self.assertEqual(result.returncode, 124, result.stderr) + self.assertEqual(result.returncode, 137, result.stderr) self.assert_no_memory_claim(result) self.assertIn("sample unavailable", result.stderr) self.assertEqual([call["name"] for call in calls], ["date", "systemctl", "ps", "awk"]) def test_missing_deadline_utility_is_not_a_sample_or_an_unbounded_fallback(self): - copied = recipe().replace("timeout --kill-after=1s 8s bash", - "pulse-missing-memory-deadline --kill-after=1s 8s bash") + copied = recipe().replace("timeout", "pulse-missing-memory-deadline") self.assertNotEqual(copied, recipe()) result, calls = self.exercise(copied=copied) - self.assertEqual(result.returncode, 127, result.stderr) + self.assertEqual(result.returncode, 1, result.stderr) self.assert_no_memory_claim(result) self.assertIn("sample unavailable", result.stderr) self.assertEqual(calls, []) + def test_term_resistant_readers_are_stopped_even_when_their_pipes_close(self): + for tool in ("systemctl", "awk"): + with self.subTest(tool=tool): + started = time.monotonic() + result, calls = self.exercise(HANG_TOOL=tool, IGNORE_TERM="1", CLOSE_PIPES="1") + self.assertLess(time.monotonic() - started, 10) + self.assertEqual(result.returncode, 137, result.stderr) + self.assert_no_memory_claim(result) + self.assertEqual(calls[-1]["name"], tool) + + def test_shell_exit_alone_does_not_pass_the_reader_termination_oracle(self): + copied = recipe().replace("--signal=KILL 8s", "--kill-after=1s 8s") + self.assertNotEqual(copied, recipe()) + with self.assertRaisesRegex(AssertionError, "reader survived the collection deadline"): + self.exercise(copied=copied, HANG_TOOL="systemctl", IGNORE_TERM="1", CLOSE_PIPES="1") + + def test_reader_failure_does_not_expose_raw_errors_or_partial_output(self): + result, _ = self.exercise(AWK_ERROR="1") + self.assert_no_memory_claim(result) + self.assertEqual(result.returncode, 3) + self.assertEqual(result.stderr, "Resident-memory sample unavailable (exit 3); stop sampling.\n") + @unittest.skipUnless(sys.platform.startswith("linux"), "Linux /proc recipe") def test_real_linux_status_of_an_owned_disposable_process(self): with tempfile.TemporaryDirectory() as temporary: @@ -247,7 +311,8 @@ class MemoryTroubleshootingDocsTest(unittest.TestCase): "1,024 bytes", "approximate and not an atomic snapshot", "not specifically the Go heap", "different run", "do not measure Pulse RSS", "do not assume its PID 1 is Pulse", - "requires GNU `timeout`", "initial UTC timestamp alone", + "requires GNU `timeout`", "ignore TERM", "No timestamp", + "does not elevate privileges or prompt", "do not add `sudo` inside it", "do not remove the deadline", "does not stop or restart Pulse", "Do not restart Pulse, drop caches, force garbage collection", "Do not post a heap dump", "existing screenshots remain useful"):