[WIP] games: Support supervised loadable game processes.#3661
Draft
aviralgarg05 wants to merge 13 commits into
Draft
[WIP] games: Support supervised loadable game processes.#3661aviralgarg05 wants to merge 13 commits into
aviralgarg05 wants to merge 13 commits into
Conversation
Fill in most of what the initial nxpkg slice deferred: network sync and artifact acquisition over plain HTTP (verified via SHA-256), an install rewrite that supports network sources and reclaims state left by an interrupted previous install, and wiring update/remove/rollback/ available into the CLI. Harden the package path against untrusted input along the way: bounded memory-safety helpers and explicit size limits throughout, storage read/write hardened against partially-written files, path-traversal rejection in manifest name/version before they reach the filesystem, and a fix for a lost-update race on the shared installed-packages database (two concurrent installs of different packages could otherwise silently clobber each other's recorded state). Add an optional manifest icon field for the nxstore GUI frontend to consume, raise PKG_INDEX_MAX now that the in-memory index is heap-allocated, and document the repository layout and local server setup in system/nxpkg/README.txt. Also fixes a real build break: pkg_runtime_compat() unconditionally referenced CONFIG_ARCH_BOARD, a Kconfig string symbol with no default clause under ARCH_BOARD_CUSTOM, so it is left entirely undefined rather than defined-but-empty on a custom board - falls back to CONFIG_ARCH_BOARD_CUSTOM_NAME instead. Routes pkg_error()/pkg_info() through syslog rather than stdio, since neither is visible to a supervisor with no attached console. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Fix real correctness/security gaps found during review, on top of the network sync and CLI completion work: - pkg_install(): commit the installed database before writing the current/previous pointer files, and before advancing transaction state past ACTIVATED. Those are recovery bookkeeping and convenience mirrors respectively - once the installed database itself is durably committed, a failure to refresh either one must not trigger the failure-cleanup path, which could otherwise delete a payload the database now legitimately points at. Also stop treating an existing version directory as newly created when reinstalling an already-installed version, so a failed reinstall can't delete a working install. - pkg_uninstall()/pkg_rollback(): acquire the per-package install lock in addition to the installed-db lock, drop the entry from the authoritative database first, and only then remove payload files - a crash between those two steps can leave orphaned files, which are reclaimable, but never leaves the database pointing at a payload that's already gone. - pkg_sync(): stage each sync to a PID-qualified temp filename instead of a single fixed name, so concurrent sync calls can no longer race on the same staging file. Return real negative errno codes instead of EXIT_SUCCESS/EXIT_FAILURE, matching the rest of this API; pkg_main.c maps that back to a process exit code at the CLI boundary. - pkg_metadata_parse_installed_entry(): validate that name/current/ previous/every entry in versions[] are safe path components, and that current (and previous, if set) actually appear in versions[] - a corrupted or tampered installed-packages database can no longer reference a nonexistent version or smuggle a path-traversal sequence through a field this code already trusted implicitly. - pkg_store_write_all()/pkg_repo_sink(): treat a zero-byte write() as -EIO instead of looping on it silently. - Removed the malloc()-falls-back-to-kmm_malloc() logic in pkg_malloc/ pkg_zalloc/pkg_realloc/pkg_free entirely - it required tracking which allocator owned a given pointer via kmm_heapmember(), which is specific to this target's flat, single-heap memory model and not a sound general application API. These are now plain wrappers around malloc/calloc/realloc/free. - Added pkg_metadata_load_manifest_path(), so a caller (e.g. the nxstore GUI frontend, launching an installed package) can load the manifest actually recorded for a specific installed version, instead of combining a rollback-selected version with whatever the current catalog happens to describe for that package name - those can disagree after a rollback if the catalog has since moved on. - Storage root default changed from /tmp/nxpkg to /var/lib/nxpkg, following the conventional persistent application-data location instead of a path whose own name suggests non-persistent storage. A board without persistent storage mounted at /var, or that wants a different location (e.g. an SD card), still overrides this via CONFIG_SYSTEM_NXPKG_ROOT as before. - Moved system/nxpkg/README.txt into the companion NuttX documentation PR rather than shipping user-facing documentation as an in-tree README. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
- pkg_sync(): index.jsn and repo.url were updated as two independent atomic writes with nothing serializing the pair against a second, concurrent pkg_sync() call - each write stayed internally consistent, but the pair didn't, so one sync's index could end up on disk next to a different sync's source URL. Add a dedicated sync lock (pkg_repo_acquire_sync_lock(), mirroring the existing installed- db lock's blocking-retry-with-stale-reclaim pattern) around the whole read-fetch-write sequence. Also renew the lock's mtime as data actually arrives (pkg_repo_sink(), via a new renew_lock_path field threaded through pkg_acquire_source()) rather than only stamping it once at acquire time - a lock acquired once and then measured against a fixed 10-minute staleness window could otherwise be reclaimed mid-download on a large-enough file over a slow-enough link, even though the download was still genuinely in progress. pkg_reclaim_stale_lock() (renamed from pkg_install_reclaim_stale_lock, made public) is shared between both lock kinds rather than duplicated. - pkg_install_prune_oldest_version(): deleted the pruned version's on-disk payload directory before the updated installed database was even durably saved. If pkg_metadata_save_installed() subsequently failed, the payload was already gone but the last successfully-saved instpkg.jsn could still list that version as installed. The victim version is now handed back to the caller (threaded through pkg_install_add_version()/pkg_install_update_installed()) so pkg_install() can defer the actual directory removal until after the save succeeds. - pkg_metadata_version_token_cmp(): two version tokens with equal numeric prefixes (e.g. "1a" and "1b", both parsing as 1) compared as equal instead of falling back to a lexical comparison of what follows the number, contradicting this function's own documented behavior and silently treating genuinely different versions as the same one. Compare the non-numeric remainder lexically when the numeric prefixes match instead of falling through. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Shorten the installed-database lock description and centralize pkg_sync() cleanup, as requested in review. Replace repeated protocol literals with named constants and preserve errno before cleanup calls can overwrite it. Store the synchronized catalog source in the catalog itself so the catalog and its relative-artifact base are committed atomically. Continue reading the former sidecar format for upgrade compatibility, and normalize array-form catalogs before adding the private source field. Compare numeric version prefixes without strtol() overflow and retain lexical comparison of suffixes. Assisted-by: Codex:gpt-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Record a per-boot token and owner PID in every package, installed-database, and synchronization lock. A contender now keeps a lock held while its recorded owner task is alive, regardless of unreliable FAT modification timestamps. Reclaim locks from exited owners or earlier boots immediately, while retaining timestamp-based migration handling for legacy empty lock files. This prevents a just-created live lock from being mistaken for a decades-old stale file on targets whose mounted filesystem clock does not match CLOCK_REALTIME. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Add nxstore, an LVGL-based frontend for nxpkg (system/nxpkg): lists packages from the local index, installs/launches the selected one, and supervises whatever it hands the screen to. Card-based app list (populate_app_list()): each row shows name, version, and description/status, install/launch state reflected via icon glyph and color (LV_SYMBOL_DOWNLOAD/PLAY, accent/success color), and a sliding-segment progress bar during install (no real byte-level progress is available from pkg_install(), so this reads as 'actively working' without fabricating a percentage). Explicit LV_STATE_PRESSED styling on every tappable row/button, since no LVGL theme is loaded and a tap would otherwise give no visual feedback at all. Supervisor screen (build_run_screen()/nxstore_enter_running_screen()): a launched app (e.g. a game that owns /dev/fb0 directly, not just another LVGL client) gets a dedicated screen with a name label and a Close button, confined to the border region the launched app's own scaled/centered framebuffer output never draws into, so switching back to it doesn't fight over pixels with whatever the app already put in the framebuffer. Close/reap handling (close_running_app_event_cb()/ nxstore_poll_running_app()): sends SIGTERM and polls waitpid(WNOHANG) for the launched pid, but explicitly also treats waitpid() returning ECHILD as 'already gone' rather than 'still running' - both the close-button handler and the passive per-loop poll independently race to reap the same child, so whichever one loses that race must not spin forever waiting for a wait() that can now never succeed. Requires the target app to install its own SIGTERM handler to exit cleanly (this is why there is no generic force-kill fallback here: an earlier version of this code called task_delete() when SIGTERM wasn't reaped quickly enough, which was found on real hardware to hang the entire board - not just the one task - when it landed mid framebuffer/heap access on this flat-memory build). Toast notifications (nxstore_toast()) provide a transient, unmissable confirmation for install/uninstall/launch outcomes and app-closed events, additive to the durable per-row subtitle text rather than a replacement for it. nxstore's own boot-time catalog sync waits (bounded, 15s) on g_wifi_dhcp_ret before attempting a network fetch, since Wi-Fi association completing doesn't imply DHCP has - an HTTP fetch attempted in that window fails with -ENETUNREACH even though the link itself is already up, indistinguishable from being genuinely offline without this wait. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Review/hardening findings from the companion nxpkg PRs: - Own framebuffer/input device paths instead of borrowing CONFIG_EXAMPLES_LVGLDEMO_FBDEVPATH/INPUT_DEVPATH from an unrelated example app: new CONFIG_SYSTEM_NXSTORE_FBDEVPATH/INPUT_DEVPATH Kconfig string options (defaulting to /dev/fb0 and /dev/input0). - g_index moves from a static struct to a heap-allocated FAR struct pkg_index_s * (pkg_zalloc()), with a "Not enough memory to load the catalog." UI fallback if the allocation fails. - nxstore_launch() now loads the specific installed version's manifest via pkg_metadata_load_manifest_path() instead of combining a rollback-selected version with whatever the current catalog entry happens to describe for that name - those can disagree after a rollback if the catalog has since moved on. Also passes through the installed manifest's launch_args/launch_argc (previously always argv[1] = NULL). - title_text buffer sized PKG_NAME_MAX + PKG_VERSION_MAX + 5 instead of a fixed 96, fixing a real compiler truncation warning. - README.txt moved to the companion NuttX documentation PR rather than shipping user-facing documentation as an in-tree README. Bugs found bringing this up on real hardware: - LV_EVENT_LONG_PRESSED could fire for a touch that was actually driving a scroll of the app list: if a drag starts slowly enough that the long-press timer (400ms) elapses before the finger crosses the scroll-lock distance, LVGL hasn't committed the gesture to scrolling yet and still delivers the long-press event, silently uninstalling whatever card the touch happened to land on. Ignore the long-press if this input device is currently attributed to scrolling any object (lv_indev_get_scroll_obj()). - Tapping an installed-app card to launch it, while a different install was in progress elsewhere in the list (or another app was already running), was allowed through unconditionally - install_worker() auto-launches its own package once done, and letting a second launch through independently meant whichever one finished last silently overwrote g_running, leaving the other process alive, unsupervised, and drawing into the same shared framebuffer with no way to close it from this UI again. Both the direct-launch and fresh-install paths in install_btn_event_cb() now refuse (with a toast) if g_running.active or g_active.manifest indicate another app is already running or about to be. - nxstore_is_installed() only checked the package *name*, not which version was actually on disk - an older installed version showed as plain "Installed" identically to a current one, and tapping it silently launched the stale payload with no update indication or action. Add nxstore_is_up_to_date() (compares the installed version against the catalog's latest manifest) and a third status_bar color (in addition to not-installed/up-to-date) plus a "Update available - tap to update" subtitle for the mismatch case; tapping such a card now goes through the install path (which fetches and auto-launches the newly installed version) instead of the direct-launch path. - lv_indev_set_scroll_limit() was set to 255 (copied from examples/ lvgldemo/lvgldemo.c, whose own touch-drift mitigation this file reused), requiring a nearly-full-screen drag before a touch was even recognized as a scroll gesture. Unlike that demo, this screen's app list is scrolled constantly, and a threshold that large read as broken/laggy scrolling rather than drift protection. Lowered to 20, enough to reject typical touch-driver jitter while still recognizing a real scroll almost immediately; momentum stays off (scroll_throw 0), which is what the dual-launch/scroll-lock fixes above actually depend on, not the gesture-start threshold. Also adds nxstore_load_icon(): best-effort loads and caches a package's optional icon (manifest->icon) as a raw RGB565 image LVGL can render with no decoder (this board has no PNG/JPEG decode capability), falling back to the existing colored-circle-plus-glyph rendering on any failure (no icon set, download failed, corrupt/ oversized file) so a bad icon never blocks a package from being listed or installed. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Remove the dependency on an ESP-specific DHCP global. Retry catalog synchronization for a bounded period only while the network stack reports readiness-related errors, allowing the same frontend to work with Wi-Fi, Ethernet, and other boards. Keep the LVGL timer serviced between attempts so the interface remains responsive while the network comes up. Assisted-by: Codex:gpt-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Use nxstore-owned framebuffer and input configuration symbols instead of relying on an unrelated LVGL demo configuration. Load the manifest for the installed version before launching it, validate that it matches the installed database entry, and pass its recorded arguments to posix_spawn(). Check spawn-attribute setup errors as well. Add the shared supervisor-bar height header to the nxstore change itself so the branch builds independently and framebuffer applications can follow the required reserved-strip contract. Assisted-by: Codex:gpt-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Keep the list behavior documentation consistent with the 20-pixel threshold used by the tested implementation. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Read cached icons completely, validate the RGB565 format and exact payload size, and discard corrupt cache entries so a later launch can retry acquisition. Key the local cache by package name and version so a catalog update cannot silently reuse an older icon. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Add a calculator example that can run as either a built-in application or loadable module. It uses configurable framebuffer and touchscreen paths, reserves the nxstore supervisor bar, and exits cooperatively on SIGTERM. Validate display, touchscreen, and signal-handler setup before entering the LVGL loop. Reset static state on every invocation so built-in relaunches behave like fresh module loads. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Allow cgol, match4, and snake to be selected as loadable modules while preserving built-in builds in both Make and CMake. For cgol, use a safer 4 KiB default stack, fix the FBIO_UPDATE area object, add an optional top-row offset for launcher controls, and handle SIGTERM cooperatively. Validate physical and virtual framebuffer dimensions and release mapped or allocated buffers on every exit path. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: Please adhere to Contributing Guidelines.
Summary
Allow CGOL, Match4, and Snake to participate in the supervised loadable-app
flow.
CGOL gains configurable framebuffer/input paths, framebuffer validation,
reserved-strip placement, clean setup failure handling, and cooperative
SIGTERM. Match4 and Snake become tristate so they can be selected asloadable modules without removing their builtin option.
This PR is stacked on the nxstore series.
For focused review, the supervised-games change is the final commit
d7b08fd41.The earlier draft commits are dependencies already under review in #3642,
#3643, and #3660; they will disappear as the stack merges.
Impact
device paths.
Testing
git diff --checktools/checkpatch.shSIGTERMcorrupting state
Testing logs before change:
Testing logs after change:
PR verification Self-Check