Merge dev into main: Bug Fix - ScheduleManager startup self-deadlock

Bug Fix: ScheduleManager::start() held mutex_ and then called loadCache(), which re-locks the same non-recursive mutex. Whenever a schedule cache file was present (the normal state for a provisioned kiosk), this self-deadlocked and blocked SDL_AppInit before the first frame was presented, so the kiosk booted to a black screen showing only the compositor cursor. Narrow the lock scope so loadCache() acquires mutex_ itself, and add a regression test that starts with a cached payload present.
Fix ScheduleManager start() self-deadlock on cached schedule
2026-10-07 08:58:46 -05:00 · 2026-10-07 08:43:13 -05:00 · 2026-10-03 12:58:56 -05:00 · 2026-10-01 17:42:59 -05:00
3 changed files with 73 additions and 54 deletions
@@ -1,12 +1,7 @@
 name: build

 on:
-  # Manual dispatch: always builds the `dev` branch (regardless of the ref
-  # selected when dispatching) and publishes to the dev Attic cache + dev
-  # live-update manifest. Version fingerprint = short commit hash.
  workflow_dispatch:
-  # Any new tag: builds the tagged revision and publishes a release to the
-  # release Attic cache + production live-update manifest. Version = the tag.
  push:
    tags: ['*']

@@ -14,14 +9,9 @@ jobs:
  nix-build:
    runs-on: orion-lxc
    steps:
-      # actions/checkout@v4 is unreliable on host-mode runners (its files are
-      # copied into the workspace instead of the repo), so clone explicitly.
-      # A manual dispatch always clones `dev`; a tag push clones the tag. A full
-      # clone (no --depth) lets `git describe --tags` resolve the real tag.
      - name: Checkout
        run: |
          rm -rf ./* ./.[!.]* 2>/dev/null || true
-          git config --global http.extraheader "Authorization: token ${{ github.token }}"
          REF="${{ gitea.ref_name }}"
          if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
            REF=dev
@@ -40,9 +30,6 @@ jobs:
          git describe --tags --always 2>&1 || true
          test -f flake.nix && echo "flake.nix present" || echo "NO flake.nix"

-      # Dev builds fingerprint as the short commit hash; releases report the
-      # tag. Persisted via GITHUB_ENV so the impure nix eval below reads it via
-      # builtins.getEnv.
      - name: Compute version
        run: |
          if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
@@ -51,18 +38,13 @@ jobs:
            echo "APP_VERSION=$(git describe --tags --always)" >> "$GITHUB_ENV"
          fi

-      # checks is keyed per-system (checks.<system>.default); nix build's
-      # #attr shorthand can't resolve it, so resolve the current system inline.
-      # --impure lets the flake read APP_VERSION via builtins.getEnv
-      # (pure evals always get the package defaults).
-      - name: Build package
+      # ---- x86_64 (native) ----
+      - name: Build package (x86_64)
        run: |
          nix build -L ".#packages.$(nix eval --impure --raw --expr 'builtins.currentSystem').default" \
            --impure --out-link result-pkg

-      # The offline test suite runs in the check derivation's checkPhase.
-      # This is a separate build/output from the package itself.
-      - name: Build + offline test suite
+      - name: Build + offline test suite (x86_64)
        run: |
          nix build -L ".#checks.$(nix eval --impure --raw --expr 'builtins.currentSystem').default" \
            --impure --out-link result-checks
@@ -80,14 +62,25 @@ jobs:
            ./build/test_license_service
          '

-      # Manual dispatch -> dev cache; tag push -> release cache.
+      # ---- aarch64 (emulated via host binfmt_misc/qemu-user) ----
+      - name: Build package (aarch64)
+        run: |
+          nix build -L ".#packages.aarch64-linux.default" \
+            --impure --out-link result-pkg-aarch64
+
+      - name: Build + offline test suite (aarch64)
+        run: |
+          nix build -L ".#checks.aarch64-linux.default" \
+            --impure --out-link result-checks-aarch64
+
+      # ---- Publish (only reached if every build/test above passed) ----
      - name: Push to Attic dev cache
        if: github.event_name == 'workflow_dispatch'
        env:
          ATTIC_TOKEN: ${{ secrets.ATTIC_TOKEN_DEV }}
        run: |
          attic login radiantwave https://attic.radiantwavetech.com "$ATTIC_TOKEN"
-          attic push radiantwave:radiantwave-dev ./result-pkg
+          attic push radiantwave:radiantwave-dev ./result-pkg ./result-pkg-aarch64

      - name: Push to Attic release cache
        if: github.event_name != 'workflow_dispatch'
@@ -95,28 +88,22 @@ jobs:
          ATTIC_TOKEN: ${{ secrets.ATTIC_TOKEN_PROD }}
        run: |
          attic login radiantwave https://attic.radiantwavetech.com "$ATTIC_TOKEN"
-          attic push radiantwave:radiantwave ./result-pkg
+          attic push radiantwave:radiantwave ./result-pkg ./result-pkg-aarch64

-      # Publish the manifest kiosks poll for live app updates. `storePath`
-      # must match exactly the closure pushed to the matching Attic cache above
-      # (readlink resolves the result-pkg out-link symlink to its real
-      # /nix/store path), so a kiosk can `nix copy --from` it without ever
-      # building from source. Upload mirrors the ISO scp pattern to the
-      # fileserver via the build-server's `radiantwave-infra` SSH alias.
-      - name: Publish dev manifest
-        if: github.event_name == 'workflow_dispatch'
+      - name: Publish manifests
        run: |
-          STOREPATH=$(readlink -f ./result-pkg)
-          printf '{"version":"%s","storePath":"%s"}\n' "${{ env.APP_VERSION }}" "$STOREPATH" > manifest.json
-          cat manifest.json
-          scp manifest.json \
-            nclack@157.245.124.103:/mnt/infrastructure_data/files/radiantwave-dev/manifest.json
+          if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
+            DEST=radiantwave-dev
+          else
+            DEST=radiantwave
+          fi

-      - name: Publish release manifest
-        if: github.event_name != 'workflow_dispatch'
-        run: |
-          STOREPATH=$(readlink -f ./result-pkg)
-          printf '{"version":"%s","storePath":"%s"}\n' "${{ env.APP_VERSION }}" "$STOREPATH" > manifest.json
-          cat manifest.json
-          scp manifest.json \
-            nclack@157.245.124.103:/mnt/infrastructure_data/files/radiantwave/manifest.json
+          X86_PATH=$(readlink -f ./result-pkg)
+          ARM_PATH=$(readlink -f ./result-pkg-aarch64)
+
+          printf '{"version":"%s","storePath":"%s"}\n' "${{ env.APP_VERSION }}" "$X86_PATH" > manifest.json
+          printf '{"version":"%s","storePath":"%s"}\n' "${{ env.APP_VERSION }}" "$ARM_PATH" > aarch64-manifest.json
+          cat manifest.json aarch64-manifest.json
+
+          scp manifest.json aarch64-manifest.json \
+            nclack@157.245.124.103:/mnt/infrastructure_data/files/$DEST/
@@ -87,6 +87,7 @@ void ScheduleManager::refreshOnce() {

 void ScheduleManager::start(db::DatabaseManager& db, const std::string& timezone,
                            std::function<void()> onChanged) {
+    {
        std::lock_guard<std::mutex> lock(mutex_);
        if (started_) return;
        started_ = true;
@@ -95,8 +96,11 @@ void ScheduleManager::start(db::DatabaseManager& db, const std::string& timezone
        onChanged_ = std::move(onChanged);
        licenseKey_ = db.getKV(db::KV_LICENSE_KEY);
        enabled_ = true; // config supplies the toggle via setEnabled()
+    }

-    // Offline first: load any cached payload immediately.
+    // Offline first: load any cached payload immediately. Called outside the
+    // lock because loadCache() acquires mutex_ itself; holding it here would
+    // self-deadlock on the non-recursive mutex when a cache file exists.
    loadCache();

    stop_.store(false);
@@ -297,6 +297,34 @@ TEST(ScheduleManagerTest, OfflineFallsBackToCache) {
    radiantwave::sync::schedule_cache::clear();
 }

+// Regression: start() must not deadlock when a cache file is present. It used
+// to hold mutex_ and then call loadCache(), which re-locked the same
+// non-recursive mutex (self-deadlock), blocking SDL_AppInit before the first
+// frame was ever presented — the kiosk sat on a black screen.
+TEST(ScheduleManagerTest, StartWithCachedPayloadDoesNotDeadlock) {
+    const std::string dir = tempCacheDir();
+    setenv("HOME", dir.c_str(), 1);
+
+    SchedulePayload p = dailyPayload();
+    ASSERT_TRUE(radiantwave::sync::schedule_cache::write(p, "\"cached\""));
+
+    const std::string dbPath =
+        (std::filesystem::temp_directory_path() / "rw_schedule_start_test.db")
+            .string();
+    std::filesystem::remove(dbPath);
+    radiantwave::db::DatabaseManager db;
+    ASSERT_TRUE(db.initialize(dbPath));
+
+    radiantwave::schedule::ScheduleManager manager;
+    manager.start(db, "UTC");
+    EXPECT_TRUE(manager.hasSchedule());
+    manager.shutdown();
+
+    db.close();
+    std::filesystem::remove(dbPath);
+    radiantwave::sync::schedule_cache::clear();
+}
+
 // ─── Phase 5: consume real Portal-generated payloads ───────────────────
 //
 // These fixtures were captured from a running Portal (GET
Author	SHA1	Message	Date
NeilClack	476202f8e2	Merge dev into main: Bug Fix - ScheduleManager startup self-deadlock build / nix-build (push) Successful in 9m50s Details Bug Fix: ScheduleManager::start() held mutex_ and then called loadCache(), which re-locks the same non-recursive mutex. Whenever a schedule cache file was present (the normal state for a provisioned kiosk), this self-deadlocked and blocked SDL_AppInit before the first frame was presented, so the kiosk booted to a black screen showing only the compositor cursor. Narrow the lock scope so loadCache() acquires mutex_ itself, and add a regression test that starts with a cached payload present.	2026-10-07 08:58:46 -05:00
NeilClack	9b57e48969	Fix ScheduleManager start() self-deadlock on cached schedule start() held mutex_ and then called loadCache(), which re-locks the same non-recursive mutex. When a schedule cache file exists and parses (the normal case for a provisioned kiosk), this self-deadlocks and blocks SDL_AppInit before the first frame, leaving only a black screen and the compositor cursor. Narrow the lock to the member setup so loadCache() can take the lock itself. Add a regression test that starts with a cached payload present.	2026-10-07 08:43:13 -05:00
NeilClack	a5a6b5bef0	Add Raspberry Pi build steps build / nix-build (push) Successful in 8m20s Details	2026-10-03 12:58:56 -05:00
NeilClack	2793dc4586	Remove gitconfig token	2026-10-01 17:42:59 -05:00