diff --git a/plugin-repos/starlark-apps/manager.py b/plugin-repos/starlark-apps/manager.py index 3da8c24d..86ad36b6 100644 --- a/plugin-repos/starlark-apps/manager.py +++ b/plugin-repos/starlark-apps/manager.py @@ -681,12 +681,18 @@ class StarlarkAppsPlugin(BasePlugin): if app.is_enabled() and app.should_render(current_time): self._render_app(app, force=False) - def display(self, force_clear: bool = False) -> None: + def display(self, force_clear: bool = False) -> bool: """ Display current Starlark app. This method is called during the display rotation. Displays frames from the currently active app. + + Returns False when there is no app to show -- which is the state of + every install without Pixlet, and of a fresh one before any app is + added. The display controller only skips a mode on a boolean False + (it checks isinstance(result, bool)), so returning None held a black + panel for the full display_duration instead of rotating on. """ try: if force_clear: @@ -699,20 +705,22 @@ class StarlarkAppsPlugin(BasePlugin): if not self.current_app: # No apps available self.logger.debug("No Starlark apps to display") - return + return False # Render app if needed if not self.current_app.frames: success = self._render_app(self.current_app, force=True) if not success: self.logger.error(f"Failed to render app: {self.current_app.app_id}") - return + return False # Display current frame self._display_frame() + return True except Exception as e: self.logger.error(f"Error displaying Starlark app: {e}") + return False def _select_next_app(self) -> None: """Select the next enabled app for display.""" diff --git a/src/display_controller.py b/src/display_controller.py index bb650698..72100568 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -1271,8 +1271,16 @@ class DisplayController: """Poll cache for new on-demand requests from external controllers.""" try: # Use a long max_age (1 hour) to ensure requests aren't expired before processing - # The request_id check prevents duplicate processing - request = self.cache_manager.get('display_on_demand_request', max_age=3600) + # The request_id check prevents duplicate processing. + # + # memory_ttl=0 is required, not optional: this key is a mailbox the + # web process writes and this process reads. get() defaults the + # in-memory TTL to max_age, so without it the first request read was + # pinned in memory for the full hour and every later poll returned + # that stale copy -- meaning no second on-demand request was honoured + # for an hour, while the API still reported success. + request = self.cache_manager.get('display_on_demand_request', + max_age=3600, memory_ttl=0) except (OSError, RuntimeError, ValueError, TypeError) as err: logger.error("Failed to read on-demand request: %s", err, exc_info=True) return @@ -1318,6 +1326,15 @@ class DisplayController: # Mark as processed BEFORE processing (to prevent duplicate processing) self.cache_manager.set('display_on_demand_processed_id', request_id, ttl=3600) self.on_demand_request_id = request_id + # Consume the mailbox entry. Leaving it on disk meant a restart replayed + # the previous request: the fresh controller read it, activated it and + # cached it, so the request the caller had just made was ignored and the + # panel silently showed the earlier plugin. processed_id still guards + # against double-processing if this delete fails. + try: + self.cache_manager.delete('display_on_demand_request') + except Exception as err: # pragma: no cover - best-effort cleanup + logger.debug("Could not clear the on-demand request mailbox: %s", err) if action == 'start': logger.info("Processing on-demand start request for plugin: %s", request.get('plugin_id')) diff --git a/src/display_manager.py b/src/display_manager.py index e578ad7c..076e9220 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -1453,12 +1453,30 @@ class DisplayManager: if parent_dir and str(parent_dir) != '/tmp': # nosec B108 - guard to skip /tmp for permission ops ensure_directory_permissions(parent_dir, get_assets_dir_mode()) self._snapshot_dir_prepared = True - # Write atomically: temp then replace - tmp_path = f"{self._snapshot_path}.tmp" - self.image.save(tmp_path, format='PNG') + # Write atomically: temp then replace. The temp name must be + # unique, not ".tmp": /tmp is world-writable and sticky, + # and this file is written by whichever user the display service + # runs as while tests and tooling run as someone else. A leftover + # fixed-name temp owned by another user is then unopenable even by + # root (fs.protected_regular refuses O_CREAT on a foreign file in a + # sticky dir), which froze the preview and the health check's + # liveness proxy until somebody deleted it by hand. Same pattern as + # the hardware-status write above. + _fd, tmp_path = tempfile.mkstemp( + dir=str(snapshot_path_obj.parent), + prefix=f".{snapshot_path_obj.name}.", suffix=".tmp") try: + with os.fdopen(_fd, "wb") as _f: + self.image.save(_f, format='PNG') + os.chmod(tmp_path, 0o644) os.replace(tmp_path, self._snapshot_path) except Exception: + # Never leave the temp behind -- that is what made the failure + # permanent rather than transient. + try: + os.unlink(tmp_path) + except OSError: + pass # Fallback to direct save if replace not supported self.image.save(self._snapshot_path, format='PNG') # Set proper file permissions after saving