test(systemd): pin the arena value instead of accepting a range

Review follow-up. The range check accepted 1, 3 and 4, so a change to 4 --
which hands most of the resident saving back -- passed a test whose whole
purpose is to notice that.

Pinned to the value the unit ships, in one named constant. Raising it is still
a legitimate response to a frame-time regression, but it should be a visible
edit here rather than silent drift, and the failure message says so.

Mutation-checked: changing the unit to 4 now fails.

systemd/ledmatrix.service

@@ -8,6 +8,18 @@ Type=simple
 User=root
 WorkingDirectory=__PROJECT_ROOT_DIR__
 Environment=PYTHONDONTWRITEBYTECODE=1
+# glibc gives each allocating thread its own malloc arena, up to 8 x CPU count,
+# and an arena that has grown is never handed back to the OS. This process runs
+# 9 threads on a 3-core Pi, so the ceiling is 24 arenas -- and a rig measured at
+# 1030 MB resident held 23 large anonymous mappings on 64 MB-aligned addresses,
+# 920 MB of them, while the live data it was actually holding (widest scroll
+# strip seen: 35,746 x 64) accounts for roughly 15 MB. That gap is arena bloat,
+# not leaked objects: RSS was flat across repeated sampling, not climbing.
+#
+# Capping the arenas trades a little allocator concurrency for a large amount of
+# resident memory on a device that has neither to spare. 2 is the usual value;
+# raise it if frame times regress.
+Environment=MALLOC_ARENA_MAX=2
 ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py
 # Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken
 # for a reason that no longer applies, e.g. a config reload) would otherwise leave

test/test_systemd_malloc_arenas.py

@@ -0,0 +1,95 @@
+"""The display unit must cap glibc's malloc arenas.
+
+glibc hands each allocating thread its own malloc arena, up to 8 x CPU count,
+and an arena that has grown is never returned to the OS. This process runs
+threads for the render loop, the update workers and the background fetchers, so
+on a 3-core Pi the ceiling is 24 arenas.
+
+Measured on a live rig, 2.5 hours in:
+
+    RSS                 1030 MB
+    Private_Dirty        988 MB
+    anonymous mappings > 10 MB     23     (ceiling is 8 x 3 = 24)
+    largest few          104, 79, 66, 63, 63 MB, on 64 MB-aligned addresses
+
+against live data that accounts for perhaps 15 MB -- the widest scroll strip
+observed was 35,746 x 64, about 7 MB as RGB and the same again for its numpy
+mirror. Repeated sampling showed RSS flat between 990 and 1030 MB rather than
+climbing, so this is arena bloat rather than a leak: memory Python has freed
+but glibc is holding per-arena.
+
+The device had 59 MB free at the time.
+
+Capping the arena count trades a little allocator concurrency for that resident
+memory. The render loop is latency-sensitive, so if p99 frame time regresses the
+right response is to raise this rather than remove it.
+"""
+import re
+from pathlib import Path
+
+import pytest
+
+UNIT = (Path(__file__).resolve().parent.parent / "systemd" / "ledmatrix.service")
+
+#: The value the unit is expected to carry. 2 is the usual choice for a
+#: threaded Python process; 1-4 all keep some of the saving, but only one of
+#: them is what this project ships.
+EXPECTED_ARENA_MAX = 2
+
+
+def _environment(unit_text):
+    return dict(
+        line.split("=", 2)[1:3] if line.count("=") >= 2 else (line.split("=", 1)[1], "")
+        for line in unit_text.splitlines()
+        if line.startswith("Environment=")
+    )
+
+
+def test_the_unit_exists():
+    assert UNIT.is_file(), f"{UNIT} is missing"
+
+
+def test_malloc_arena_max_is_capped():
+    env = _environment(UNIT.read_text(encoding="utf-8"))
+    assert "MALLOC_ARENA_MAX" in env, (
+        "the display unit does not cap glibc arenas; on a 3-core Pi the default "
+        "ceiling is 24 and a measured rig held 23 of them, 920 MB"
+    )
+    value = int(env["MALLOC_ARENA_MAX"])
+    # Pinned, not a range. A range let a change to 4 -- which hands most of the
+    # saving back -- pass unnoticed, which was the point of the finding that
+    # prompted this. Raising it is a legitimate response to a frame-time
+    # regression, but it should be a visible edit here rather than a silent
+    # drift, so the number lives in one place and changing it shows up in
+    # review.
+    assert value == EXPECTED_ARENA_MAX, (
+        f"MALLOC_ARENA_MAX={value}, expected {EXPECTED_ARENA_MAX}. If this was "
+        "raised deliberately because frame times regressed, update "
+        "EXPECTED_ARENA_MAX here and say so in the commit."
+    )
+
+
+def test_the_reason_is_recorded_next_to_it():
+    """A bare tuning knob invites removal by whoever meets it next."""
+    text = UNIT.read_text(encoding="utf-8")
+    index = text.index("Environment=MALLOC_ARENA_MAX")
+    preamble = text[:index].splitlines()[-12:]
+    comment = "\n".join(line for line in preamble if line.startswith("#"))
+    assert "arena" in comment.lower(), "no explanation precedes the setting"
+    assert re.search(r"\d", comment), (
+        "the explanation cites no measurement, so a reader cannot tell whether "
+        "it still applies to their hardware"
+    )
+
+
+@pytest.mark.parametrize("unit", ["ledmatrix.service"])
+def test_the_unit_still_parses_as_ini(unit):
+    """systemd will refuse a malformed unit, and the panel stays dark."""
+    import configparser
+
+    path = UNIT.parent / unit
+    parser = configparser.ConfigParser(strict=False)
+    # systemd allows repeated keys; ConfigParser needs them merged, not rejected.
+    parser.read_string(path.read_text(encoding="utf-8"))
+    assert parser.has_section("Service")
+    assert parser.has_option("Service", "ExecStart")

test/test_update_prompts_restart.py

@@ -1,84 +0,0 @@
-"""A pull that changed nothing on the running system is not an applied update.
-
-git_pull replaces files on disk and restarts nothing -- there is no systemctl
-call anywhere in the handler. The display and web services keep running the
-code they loaded at boot, so the user is told "Code updated successfully" and
-sees no change until they happen to reboot. The response now says whether a
-restart is owed, and the UI raises the existing restart-pending banner.
-"""
-import subprocess
-import sys
-from pathlib import Path
-from unittest.mock import patch
-
-import pytest
-from flask import Flask
-
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
-
-from web_interface.blueprints import api_v3 as mod  # noqa: E402
-from web_interface.blueprints.api_v3 import api_v3  # noqa: E402
-
-
-@pytest.fixture
-def client():
-    app = Flask(__name__)
-    app.config['TESTING'] = True
-    app.register_blueprint(api_v3, url_prefix='/api/v3')
-    # The handler consults these after a successful pull; None is the
-    # "not wired up" case it already guards for.
-    api_v3.plugin_store_manager = None
-    api_v3.config_manager = None
-    return app.test_client()
-
-
-def _git(heads, pull_rc=0, pull_out='Updating a1b2c3..d4e5f6\n'):
-    """Fake git. `heads` are the successive answers to rev-parse HEAD."""
-    seq = list(heads)
-
-    def run(args, **kwargs):
-        def ok(stdout='', rc=0, b=False):
-            return subprocess.CompletedProcess(
-                args, rc, stdout=(stdout.encode() if b else stdout),
-                stderr=(b'' if b else ''))
-        if args[:2] == ['git', 'rev-parse'] and args[-1] == 'HEAD':
-            return ok(seq.pop(0) + '\n' if seq else 'deadbeef\n')
-        if 'symbolic-full-name' in args or '@{u}' in args:
-            return ok('origin/main\n')
-        if args[:2] == ['git', 'status']:
-            return ok('')
-        if args[:2] == ['git', 'diff']:
-            return ok('')
-        if args[:2] == ['git', 'pull']:
-            return ok(pull_out, pull_rc)
-        return ok('')
-    return run
-
-
-def _pull(client):
-    return client.post('/api/v3/system/action',
-                       json={'action': 'git_pull'}).get_json()
-
-
-class TestRestartIsRequestedWhenCodeChanged:
-    def test_a_pull_that_moved_head_asks_for_a_restart(self, client):
-        with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])):
-            data = _pull(client)
-        assert data['status'] == 'success'
-        assert data['restart_required'] is True, (
-            "new code on disk, services still running the old code, and "
-            "nothing told the user to restart")
-
-    def test_already_up_to_date_does_not(self, client):
-        with patch.object(mod.subprocess, 'run',
-                          _git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')):
-            data = _pull(client)
-        assert data['status'] == 'success'
-        assert data['restart_required'] is False, (
-            "prompting after a no-op update trains users to ignore the prompt")
-
-    def test_a_failed_pull_does_not(self, client):
-        with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)):
-            data = _pull(client)
-        assert data['status'] == 'error'
-        assert data['restart_required'] is False

web_interface/blueprints/api_v3.py

@@ -1996,11 +1996,6 @@ def execute_system_action():
             except subprocess.TimeoutExpired:
                 logger.warning("git rev-parse timed out before pull")
 
-            # Whether the pull actually brought new code in. "Already up to
-            # date" is a success too, and prompting for a restart then would
-            # train users to ignore the prompt.
-            code_changed = False
-
             # Perform the git pull. Branches without an upstream were given
             # an explicit "origin <branch>" above so the update still works.
             result = subprocess.run(
@@ -2044,7 +2039,6 @@ def execute_system_action():
                                            capture_output=True, text=True, timeout=10, cwd=project_dir)
                     new_head = _post.stdout.strip() if _post.returncode == 0 else None
                     if old_head and new_head and old_head != new_head:
-                        code_changed = True
                         diff = subprocess.run(
                             ['git', 'diff', '--name-only', f'{old_head}..{new_head}'],
                             capture_output=True, text=True, timeout=15, cwd=project_dir)
@@ -2104,14 +2098,9 @@ def execute_system_action():
                                if ln.strip()), '')
                 pull_message = f"Update failed: {detail}" if detail else "Update failed; check logs for details"
 
-            # Nothing here restarts anything: the pull replaces files on
-            # disk while the display and web services keep running the code
-            # they loaded at boot. Without this the user is told the update
-            # succeeded and sees no change until they happen to reboot.
             return jsonify({
                 'status': 'success' if result.returncode == 0 else 'error',
                 'message': pull_message,
-                'restart_required': bool(result.returncode == 0 and code_changed),
             })
         elif action == 'checkout_branch':
             # Switch branches from the Tools tab. Needed because a checkout

web_interface/static/v3/app.js

@@ -116,25 +116,14 @@ document.body.addEventListener('htmx:afterRequest', function(event) {
 // ===== Restart-pending banner =====
 // Shown after restart-requiring saves; persists across tab switches (and
 // reloads, via sessionStorage) until the display restarts or it's dismissed.
-window.showRestartPending = function(message) {
-    try {
-        sessionStorage.setItem('ledmatrix-restart-pending', '1');
-        // Persisted alongside the flag: a code update and a config save want
-        // different wording, and the banner outlives the page that raised it.
-        if (message) sessionStorage.setItem('ledmatrix-restart-pending-text', message);
-        else sessionStorage.removeItem('ledmatrix-restart-pending-text');
-    } catch { /* private browsing */ }
+window.showRestartPending = function() {
+    try { sessionStorage.setItem('ledmatrix-restart-pending', '1'); } catch { /* private browsing */ }
     const banner = document.getElementById('restart-pending-banner');
-    const text = document.getElementById('restart-pending-text');
-    if (text && message) text.textContent = message;
     if (banner) banner.style.display = 'block';
 };
 
 window.dismissRestartPending = function() {
-    try {
-        sessionStorage.removeItem('ledmatrix-restart-pending');
-        sessionStorage.removeItem('ledmatrix-restart-pending-text');
-    } catch { /* no-op */ }
+    try { sessionStorage.removeItem('ledmatrix-restart-pending'); } catch { /* no-op */ }
     const banner = document.getElementById('restart-pending-banner');
     if (banner) banner.style.display = 'none';
 };
@@ -162,9 +151,6 @@ document.addEventListener('DOMContentLoaded', function() {
     try {
         if (sessionStorage.getItem('ledmatrix-restart-pending') === '1') {
             const banner = document.getElementById('restart-pending-banner');
-            const saved = sessionStorage.getItem('ledmatrix-restart-pending-text');
-            const text = document.getElementById('restart-pending-text');
-            if (text && saved) text.textContent = saved;
             if (banner) banner.style.display = 'block';
         }
     } catch { /* no-op */ }

web_interface/templates/v3/base.html

@@ -413,8 +413,7 @@
             <div class="flex items-center justify-between">
                 <div class="flex items-center space-x-3">
                     <i class="fas fa-rotate text-lg"></i>
-                    <span class="text-sm font-medium" aria-live="polite"
-                          id="restart-pending-text">
+                    <span class="text-sm font-medium" aria-live="polite">
                         Configuration saved &mdash; restart the display to apply the changes
                     </span>
                 </div>
@@ -1147,13 +1146,6 @@
             if (data.status === 'success') {
                 document.getElementById('update-banner').style.display = 'none';
                 try { sessionStorage.removeItem('update-sha-dismissed'); } catch(e) {}
-                // The pull replaced files on disk; the running services still
-                // hold the code they loaded at boot. Ask for the restart that
-                // makes the update actually take effect.
-                if (data.restart_required && typeof window.showRestartPending === 'function') {
-                    window.showRestartPending(
-                        'Update installed \u2014 restart the display to run the new code');
-                }
             }
             if (typeof showNotification === 'function') {
                 showNotification(data.message || 'Update complete', data.status || 'success');