system/nxinit: add a per-service "console" option - #3776
Draft
JianyuWang0623 wants to merge 3 commits into
Draft
Conversation
nxstyle flags a missing blank line between the declaration of "s" and the first statement in option_reboot_on_failure(); pre-existing, unrelated to any behavioral change here. Assisted-by: Kiro:claude-sonnet-5 Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
init_service_refresh() ignored the return value of init_service_start(). If spawning a restarting service failed for any reason (e.g. posix_spawnp() itself failing), the service stayed SVC_RESTARTING forever with nothing left to re-arm its retry timer: this function's return value drives the poll timeout in the caller's event loop (main()), and a failed spawn does not fork a child, so there is no SIGCHLD either to wake it up some other way. If this service happens to be the only pending timer, the poll blocks indefinitely and the service is never attempted again. Check the return value and, on failure, feed the service restart period into the poll timeout computed by this function, the same way a successfully started/still-restarting service already does. Also move the CLOCK_MONOTONIC read that updates a service's time_started from after a successful spawn to before the spawn is even attempted, so that time_started stays current on a failed spawn too - otherwise, once woken up (by the fix above or by an unrelated event), a repeatedly failing service would look permanently overdue (elapsed time computed against a stale timestamp) and get retried immediately regardless of its restart_period. Assisted-by: Kiro:claude-sonnet-5 Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
Services started by nxinit (e.g. "sh") do not open a console device on their own, unlike nsh_main, which explicitly does so via nsh_consolemain()/nsh_waitusbready() for USB gadget consoles (CDC-ACM/PL2303). When such a board switches its top-level init from nsh_main to nxinit, no code path ever registers/connects the USB console gadget, and a service that just execs a plain "sh" inherits whatever (invalid, for a USB gadget console not yet opened at the time the idle task file descriptors are set up) stdio nxinit itself has. Add a "console [<device>]" service option: a service declared with it gets the given device (CONFIG_SYSTEM_NXINIT_CONSOLE_DEV, "/dev/console" by default, if no device is given) opened and dup'd onto its stdin, stdout and stderr via posix_spawn_file_actions before it is spawned. This does not depend on nsh being enabled at all. For a USB gadget console, the device does not exist until the gadget is actually registered; boards using one are expected to bring it up themselves before any service using "console" is started (e.g. via an "exec -- sercon" action in their init.rc, since apps/system/cdcacm already implements exactly that registration step and does not depend on nxinit or nsh either). console_file_actions() runs after the previous commit's time_started update, so a failure to build the console's file actions is covered by the same up to date timestamp - no separate clock_gettime() call is needed on this failure path. Covered by a new unit test, test_nxinit_service_console_option, that exercises the option with and without an explicit device, and confirms services without the option are left untouched. Assisted-by: Kiro:claude-sonnet-5 Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
acassis
approved these changes
Sep 8, 2026
acassis
left a comment
Contributor
There was a problem hiding this comment.
@JianyuWang0623 please update the Documentation to include info about this "console" option
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
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.
Summary
Adds a per-service
"console [<device>]"option to nxinit's init.rcservice syntax, plus two small robustness fixes uncovered while adding
it.
Services started by nxinit (e.g. plain
"sh") do not open a consoledevice on their own, unlike
nsh_main, which explicitly does so viansh_consolemain()/nsh_waitusbready()for USB gadget consoles(CDC-ACM/PL2303). When a board switches its top-level init from
nsh_mainto nxinit, no code path ever registers/connects a USBconsole gadget, and a service that just execs a plain
"sh"inheritswhatever (invalid, for a gadget console not yet opened) stdio nxinit
itself has. This was the actual root cause of a real-hardware
regression reported on esp32s3-xiao (board hangs, no USB console) —
see the companion apache/nuttx PR for the board-side fix that uses
this option.
A service declared with
console [<device>]gets the given device(
CONFIG_SYSTEM_NXINIT_CONSOLE_DEV,"/dev/console"by default, if nodevice is given) opened and dup'd onto its stdin, stdout and stderr via
posix_spawn_file_actionsbefore it is spawned. This does not dependon nsh being enabled at all — for a USB gadget console, boards are
expected to bring the gadget up themselves before any service using
"console"starts (e.g. via anexec -- serconinit.rc action, sinceapps/system/cdcacmalready implements exactly that and depends onneither nxinit nor nsh either).
Two commits unrelated to the option itself, split out for review:
system/nxinit: fix missing blank line after declaration in service.c— pre-existing nxstyle nit inoption_reboot_on_failure(),unrelated to this change but in a file this PR already touches.
system/nxinit: retry a service whose spawn failed—init_service_refresh()ignored the return value ofinit_service_start(); a service whose spawn fails stayedSVC_RESTARTINGwith nothing left to arm this function's own polltimeout, and (being a failed spawn) no
SIGCHLDeither to wake theevent loop some other way. If it were the only pending timer, the
loop would block indefinitely and the service would never be
attempted again. Also moves the
time_startedupdate ininit_service_start()from after a successful spawn to before thespawn is even attempted, so a failure doesn't leave a stale timestamp
behind (which would otherwise make a repeatedly failing service look
permanently "overdue" and get retried on every unrelated wakeup,
bypassing its own
restart_period).Impact
"console"service option in nxinit's init.rc syntax; existinginit.rc files using services without it are unaffected.
init_service_refresh()/init_service_start()behavior change is apure bug fix (previously-ignored failure case); no change to the
successful-spawn path.
Testing
Covered by a new unit test,
test_nxinit_service_console_option(
system/nxinit/test/test_nxinit_service.c), exercising the optionwith and without an explicit device, and confirming services without
the option are left untouched.
Build-verified against
esp32s3-xiao:usbnsh/combo(apache/nuttx)with the option actually wired up via the companion board-side PR:
End-to-end hardware verification on real Seeed XIAO ESP32-S3 Sense
(
usbnsh): a real physical reset (dmesgshows a clean USBdisconnect/reconnect, not a software-triggered one) now enumerates the
NuttX CDC-ACM console gadget instead of leaving USB dark:
Serial console over that device, showing
init_main(nxinit) as PID 2and the
consoleservice'sshas its child (PID 4), i.e. the"console"option actually connectedsh's stdio to the gadget:Before this option existed (plain
service console sh, noconsoleline, no
exec -- sercon), the same hardware reproduced the originalregression exactly: after a real reset the USB device disappears
entirely and never re-enumerates.
More detail on the board-side setup (
init.rc/defconfig changes, theexec -- serconstep, and a from-scratch repro of the original hang)is in the companion apache/nuttx#20088.