Skip to content

Launcher: persist wallpaper and overlay to internal storage to eliminate boot timing race - #3317

Open
Jesencloud wants to merge 5 commits into
Gathin23:mainfrom
Jesencloud:fix/cold-boot-theme-storage-race
Open

Jesencloud wants to merge 5 commits into
Gathin23:mainfrom
Jesencloud:fix/cold-boot-theme-storage-race

Conversation

@Jesencloud

Copy link
Copy Markdown
On cold boot (especially on TVs waking straight to Home via Direct Boot or vendor intent handlers), external storage (`/sdcard`) can take several seconds to mount after the launcher process starts.

This PR fixes three related cold-boot issues:
1. **Prevent accidental cache eviction in `readOverlay`**: When external storage is reported mounted by the framework but the file handle is not yet accessible during early boot, `readOverlay` previously
deleted the internal fallback copy (`copy.delete()`), resetting the theme back to `config.xml` defaults. It now preserves and falls back to `copy`.
2. **Internal wallpaper persistence**: Themes pushed with a `wallpaper.jpg` now mirror the image into internal app storage (`ctx.getFilesDir()`), ensuring the wallpaper is drawn immediately on the first
frame of a cold boot without waiting for `/sdcard` to mount.
3. **Resume self-healing & choice protection**: If Home starts before storage is ready, `onResume()` rechecks and displays the wallpaper once storage mounts. `syncThemeChoices()` guards against resetting
user choices when the external config file is temporarily unavailable.

**Tested on:**
Skyworth 7T861_A23, Android 10 (MediaTek MT9638 / API 29)

### If this touches the launcher
- [x] Verified on a real TV, screenshot attached
- [x] D-pad checked from every edge (Up, Down, Left, Right)
- [x] No network, no background service, no new permission
- [x] Anything TV-specific is in `config.xml` or a theme, not in code
- [x] Release APK size is 588,337 B (under 576 KB gate)
- [x] Zero lint errors

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

maintainer-bot review · this one needs a maintainer decision · Leans: merge (60%)

Thank you for tracking down the cold-boot storage race and for the syncFile tests. This reads as a tested bug fix in the same spirit as #614 and looks mergeable. One thing worth confirming: removing a theme should still clear the internal copy once /sdcard is mounted.

Why it goes to a maintainer: confidence 0.60 is below 0.75.
The bot's own suggestion would be: merge.

Re-checked at this commit: the PR's own changes are the same as at the last review.

Checks

This is a suggestion; a maintainer makes the final call. A maintainer can comment /bot review to run it again.

Reviewed by maintainer-bot · runs on 0G Private Computer

@github-actions github-actions Bot added bot-leans:rework Held for the maintainer; the review leans rework launcher Changes under launcher/: the Android TV home screen app needs-maintainer labels Nov 5, 2026
@SaviorOneZero

Copy link
Copy Markdown
Contributor

A separate cache-write failure at 07c00b47b0b6ce264f5c7cedb38534166c103873: syncFile() opens the final internal wallpaper with FileOutputStream(dst), truncating any previous cache before the copy completes. A later read/write IOException is swallowed, leaving a positive-length partial file. The fallback selection then accepts it whenever the external wallpaper is unavailable.

With the actual helper and controlled stream failures over real temporary files, a read failure after the first 8-KiB chunk replaced the previous cache with those partial bytes. A first-copy failure also published a partial cache; an injected write failure similarly overwrote the previous bytes. These are synthetic host I/O tests, not observed Android storage failures or JPEG-rendering tests.

A focused option is to write beside the destination, close successfully, then replace it; on read/write/rename failure, retain the previous cache and clean the staging file best-effort. Serialize calls if using one fixed staging filename. The existing length/timestamp skip can stay.

A one-file local prototype changes 3/8 failing cases to 8/8 passing, plus a new rename-failure control (9 passing total). The isolated helper also type-checks against Android API 34 stubs. Costs: one extra image-sized temporary file during a copy and in-process serialization; this does not move the existing synchronous Config.load() I/O off the main thread. No full launcher build, Android power-loss/fsync guarantee or physical-TV test is claimed. This is distinct from the reset/fallback-policy issue already raised above.

@Jesencloud

Copy link
Copy Markdown
Author

@SaviorOneZero
Thanks for the thorough review and for catching the truncation risk on partial writes!

I have addressed both points in the latest commit:
1. **Atomic cache write with staging file**:
   - `syncFile()` now writes to a temporary staging file (`dst.getName() + ".tmp"`) with in-process synchronization (`SYNC_LOCK`).
   - Only upon a complete, successful stream flush is `tmp.renameTo(dst)` performed.
   - Any `IOException` during read/write or a failed rename leaves the pre-existing destination cache untouched and removes the staging file best-effort.
2. **Aligned with `--reset` policy**:
   - Kept the upstream contract: the internal cache is only used as a fallback when external storage is unavailable (`!mounted`).
   - When external storage is mounted and the wallpaper/config is absent (e.g. after `tools/theme.sh --reset`), the internal cache is cleanly evicted so that reset behavior remains completely intact.
3. Added `SyncFileTest` covering atomic writes, timestamp/length skips, and cache retention on invalid source.

@SaviorOneZero

Copy link
Copy Markdown
Contributor

The staged copy at 26b1637413260adb6b47688757fb5bb5525a2ce1 passes the nine previous read/write/rename controls. One close-completion edge remains in syncFile(): ok = true runs before try-with-resources closes the streams. If either close reports IOException, the catch swallows it but ok stays true, so the staging file is still renamed over the previous cache.

A small correction is to delete the staging file and return from the catch, then place timestamp/rename after the try/catch. This makes successful resource closure part of the commit boundary without changing the buffer, lock, metadata skip or fallback policy.

With the actual helper, real temporary files and controlled close failures, three added cases fail on this head: output-close failure with an existing cache, output-close failure on the first copy, and input-close failure. The correction passes 12/12 cases including the nine earlier controls; the four unchanged author helper tests also pass. The isolated corrected helper type-checks against Android API 34. These tests demonstrate publication despite a reported close failure; they do not demonstrate Android file corruption, a power-loss guarantee, a full launcher build or a physical-TV failure. No extra I/O or background work is added.

@Jesencloud

Jesencloud commented Nov 5, 2026 •

Copy link
Copy Markdown
Author

@SaviorOneZero , Thanks for the precise catch on the try-with-resources close() boundary.

Updated `syncFile()` in commit 2bd8cfaa:
- Removed `boolean ok` and handled failure by deleting `tmp` and returning early inside `catch (IOException ignored)`.
- `setLastModified()` and `renameTo(dst)` now execute strictly after both streams close cleanly without exception.

Verified with unit tests, lint (0 errors), and the release APK size remains within the gate (588,449 B < 589,824 B).

@Gathin23

Gathin23 commented Nov 6, 2026

Copy link
Copy Markdown
Owner

@Jesencloud thanks, the staged, atomic syncFile with its tests is a really careful answer to the slow /sdcard mount. One change before merge: only run the onResume recheck when Config.load saw storage unmounted, using a flag cleared once it is mounted. That way TVs without a wallpaper never reload config or risk a recreate loop. Please also trim the description to what the diff does. We'll check back in 7 days.

@github-actions github-actions Bot removed the bot-leans:rework Held for the maintainer; the review leans rework label Nov 6, 2026
@Jesencloud

Jesencloud commented Nov 6, 2026 •

Copy link
Copy Markdown
Author
Thanks for the guidance! Rebased onto latest main and updated in commit 5c3a8dd5:

1. Added `storageUnmounted` flag from `Config.load()`, tracked in `MainActivity`. The `onResume` recheck only triggers when storage was unmounted at initial load, and the flag is cleared immediately once mounted so TVs
without a wallpaper never recheck or loop.
2. Cleaned up `WallpaperPlayer` diff so only `Config` and `MainActivity` are touched.
3. Trimmed the PR description down to what the diff changes.

Verified with unit tests, lint (0 errors), and the release APK size remains within the gate (588,505 B < 589,824 B).

@Jesencloud
Jesencloud force-pushed the fix/cold-boot-theme-storage-race branch from 4a43dab to 5c3a8dd Compare November 6, 2026 03:57
@SaviorOneZero

Copy link
Copy Markdown
Contributor

The one-shot gate at 5c3a8dd54d5cb69777de93ea68ae736739eafd2c avoids repeated loads, but onResume() still discards valid recovered themes when check.wallpaper == null. Config separately supports a folder-only wallpaper collection (check.wallpapers) and a parsed JSON overlay with no wallpaper, including wallpaper: false. After the unmounted → mounted transition, these load successfully but never trigger recreate(), and the flag is already cleared. The existing activity keeps its earlier configuration.

A focused condition is check.wallpaper != null || !check.wallpapers.isEmpty() || check.overlayApplied, retaining the flag clear before recreation. A TV with no theme still avoids recreation and subsequent checks.

With the verbatim mount branch in a host wrapper and controlled Config/Environment values, folder-only, JSON-only and wallpaper-disabled overlay cases fail on this head; that condition passes 7/7, including static wallpaper, no-theme, still-unmounted, initially-mounted and repeated-resume controls. The four author tests also pass with the actual extracted syncFile() and real temporary files. This is source tracing plus isolated branch validation, not a full Config/Activity Android test or a physical-TV reproduction. The change adds only field/list checks to the existing one-shot recovery path, with no polling or extra file read.

@SaviorOneZero

Copy link
Copy Markdown
Contributor

One older cache path at 5c3a8dd54d5cb69777de93ea68ae736739eafd2c still bypasses the staged writer: readOverlay() opens the final internal config.json directly. A write failure after a prefix destroys the previous good copy. The catch returns the already-parsed external JSON, so the current load can succeed, but a later unmounted load cannot parse that partial fallback. This helper is unchanged in main 3c122bc0; it is a pre-existing JSON-cache issue, separate from the corrected wallpaper syncFile() and the resume gate.

A focused option is to stage the serialized JSON beside the copy under the existing SYNC_LOCK, close successfully, then rename it over the copy. On write/close/rename failure, remove only the staging file and still return the parsed external JSON. Keep the existing json == null deletion/reset policy; this need not revive a deliberately reset overlay or add boot polling.

The actual readOverlay()/read() helpers with real temporary files, JSON parsing and controlled output failures give 6 failures among 16 cases on this head (zero-byte write, partial write and close failure, each with/without a prior cache). The correction passes those 16 plus two rename-failure controls: 18/18. Success, open failure, unmounted fallback and mounted missing/malformed-file controls are included. The uninstrumented corrected helpers type-check against Android API 34.

These are isolated host I/O tests, not an observed Android storage failure, cold-boot test or full launcher build. Cost: one extra JSON-sized temporary file (the reader caps input at 64 KiB) and a rename during the existing load; no new timer, worker or dependency. No cross-process or power-loss guarantee is claimed.

@SaviorOneZero

Copy link
Copy Markdown
Contributor

A separate choice-protection edge at 5c3a8dd54d5cb69777de93ea68ae736739eafd2c: revisionToken() turns an open/read/close exception into "absent". If the same existing marker was acknowledged previously, syncThemeChoices() sees a changed signature and removes the saved settings even though no theme push/reset occurred. The single read(bytes) also accepts an incomplete token as a new revision. These helpers are unchanged in main 3c122bc0; this is not introduced by the PR.

Small correction: return a distinct failure result (null) for I/O failure or a read that cannot complete the bounded marker, and skip only theme-choice synchronization for that load. Read in a loop until the existing ≤64-byte buffer is complete. Keep missing/empty/oversized markers as "absent", so intentional push/reset and upgrade behavior stay intact; the next ordinary config load retries naturally.

Using the actual three helpers, real files/FileInputStream and controlled input failures/chunked reads, 9 of 16 host cases fail on this head. The correction passes 16/16, covering unchanged-marker failures, retry, a real changed token, first acknowledgement, and missing/empty/oversized controls. Preferences are transactional Map stand-ins; this does not reproduce Android storage or SharedPreferences persistence failure. The uninstrumented corrected helpers type-check against API 34.

No new buffer, timer, worker or normal-path file read is added; only short reads need another bounded read. A genuinely changed theme retains the current on-TV choices until its marker can be read and acknowledged. This is distinct from #844's signature-commit/crash-history issue and from the JSON-copy and resume-gate findings above.

@Jesencloud
Jesencloud force-pushed the fix/cold-boot-theme-storage-race branch from 5c3a8dd to 83f3dbc Compare November 6, 2026 16:07
@Jesencloud

Copy link
Copy Markdown
Author

Thanks @SaviorOneZero for the thorough review and the three precise findings. All three are addressed in 83f3dbc7:

1. Resume gate condition — the onResume() recovery now also triggers recreate() for folder-only wallpaper collections (check.wallpapers) and JSON-only overlays (check.overlayApplied), not only a single static wallpaper. A TV with no theme still avoids recreation.

2. Atomic overlay cache — readOverlay() now writes the internal config.json copy through a .tmp staging file under the existing SYNC_LOCK, matching the wallpaper syncFile() pattern. A partial write no longer destroys the previous good fallback.

3. revisionToken I/O guard — revisionToken() returns null on read exception or short read (instead of "absent"), and syncThemeChoices() skips that load rather than clearing saved user choices. Reads use a loop to handle partial reads. Missing, empty and oversized markers still return "absent" as before, preserving intentional push/reset and upgrade behavior.

All existing SyncFileTest tests pass, launcher unit tests pass, lint zero errors, APK size 589,037 B (under the 576 KB gate). Rebased onto current origin/main (3c122bc0).

@Gathin23

Gathin23 commented Nov 7, 2026

Copy link
Copy Markdown
Owner

@Jesencloud thank you, the one-shot recheck behind storageUnmounted and the staged writes are exactly what we asked for. Since this changes what the launcher reads and copies at cold boot, it gets one real-TV cold-boot run (wallpaper and theme on the card, then a reboot) before it merges. No change is needed from you.

@littlesteve01

Copy link
Copy Markdown

Reproduced on another set, in case it helps with testing: Xiaomi MiTV-MOEU0 (ladybird), Google TV 14 (SDK 34), firmware UKN1.240806.017.V816.0.26.10.12.1.ULBMIXM, launcher built from main at 3f17512, with the ambient-glass theme pushed by tools/theme.sh.

After each cold reboot (two tries) Home comes up with the theme's layout (5-column dock, icon-only buttons), so the overlay came from the internal copy, but with no wallpaper: plain black. Pressing Home again does not change it. tools/theme_restart.py brings the photos back at once, and they stay until the next cold boot. The files are on the TV the whole time (/sdcard/Android/data/com.gathin.tvhome/files/wallpapers/).

Reading Config.readOverlay and MainActivity.onCreate on main, this matches the race described here: the config is loaded once in onCreate, before external storage is mounted, and nothing looks for the wallpapers again later.

@Jesencloud

Jesencloud commented Nov 7, 2026 •

Copy link
Copy Markdown
Author

Thanks @littlesteve01 for reproducing and confirming this on Xiaomi / Google TV 14! That exact cold-boot black screen behavior (layout loads from cache, but wallpaper fails because /sdcard isn't ready during early onCreate) is what prompted this fix.

This PR addresses that by:

  1. Mirroring wallpapers pushed with a theme into internal app storage so they render on the very first frame of a cold boot.
  2. Adding a guarded one-shot recheck in onResume() when storage wasn't mounted on startup, so dynamic wallpaper folders (like ambient-glass) reload automatically once storage finishes mounting.

If you have a chance to build and test this branch (fix/cold-boot-theme-storage-race, commit 935b8d25), it would be awesome to confirm whether it resolves the cold-boot black screen on your MiTV as well!

jiantai added 5 commits November 8, 2026 00:37
…t I/O edges

Address three review findings from SaviorOneZero:

1. onResume recovery gate: also recreate for folder-only wallpaper
   collections (check.wallpapers) and JSON-only overlays
   (check.overlayApplied), not only a single static wallpaper.

2. readOverlay atomic cache: write the internal config.json copy
   through a .tmp staging file under SYNC_LOCK, matching the
   wallpaper syncFile pattern. A partial write no longer destroys
   the previous good fallback.

3. revisionToken I/O guard: return null on read exception or short
   read instead of "absent", so syncThemeChoices skips that load
   rather than clearing saved user choices. Read in a loop to
   handle partial reads. Missing, empty and oversized markers
   still return "absent" as before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot-leans:merge Held for the maintainer; the review leans merge launcher Changes under launcher/: the Android TV home screen app needs-maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants