From 83408d5f998d29cd7788557c06c2bc5f9c1a55a2 Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Tue, 8 Sep 2026 12:56:15 +0800 Subject: [PATCH 1/3] system/nxinit: fix missing blank line after declaration in service.c 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 --- system/nxinit/service.c | 1 + 1 file changed, 1 insertion(+) diff --git a/system/nxinit/service.c b/system/nxinit/service.c index 8a92bb20baf..52719ed957a 100644 --- a/system/nxinit/service.c +++ b/system/nxinit/service.c @@ -279,6 +279,7 @@ static int option_reboot_on_failure(FAR struct service_manager_s *sm, { FAR struct service_s *s = list_last_entry(&sm->services, struct service_s, node); + s->reset_reason = atoi(argv[1]); return 0; } From c53d924abec6f800223e09b6c7a8120d43224418 Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Tue, 8 Sep 2026 13:16:52 +0800 Subject: [PATCH 2/3] system/nxinit: retry a service whose spawn failed 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 --- system/nxinit/service.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/system/nxinit/service.c b/system/nxinit/service.c index 52719ed957a..8fdb7cbe8c5 100644 --- a/system/nxinit/service.c +++ b/system/nxinit/service.c @@ -323,7 +323,11 @@ int init_service_refresh(FAR struct service_manager_s *sm) ms = TIMESPEC2MS(diff); if (ms >= service->restart_period) { - init_service_start(service); + if (init_service_start(service) < 0) + { + min = MIN(min, service->restart_period); + } + continue; } @@ -466,6 +470,8 @@ int init_service_start(FAR struct service_s *service) return -ret; } + clock_gettime(CLOCK_MONOTONIC, &service->time_started); + ret = posix_spawnp(&pid, service->argv[2], NULL, &attr, &service->argv[2], environ); posix_spawnattr_destroy(&attr); @@ -477,7 +483,6 @@ int init_service_start(FAR struct service_s *service) } service->pid = pid; - clock_gettime(CLOCK_MONOTONIC, &service->time_started); add_flags(service, SVC_RUNNING); remove_flags(service, SVC_RESTARTING); remove_flags(service, SVC_DISABLED); From 091b51ecff68fdc697289c8b5a87d1368f94c1ac Mon Sep 17 00:00:00 2001 From: wangjianyu3 Date: Tue, 8 Sep 2026 13:20:59 +0800 Subject: [PATCH 3/3] system/nxinit: add a per-service "console" option 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 []" 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 --- system/nxinit/Kconfig | 14 +++ system/nxinit/service.c | 117 ++++++++++++++++++++++- system/nxinit/service.h | 7 ++ system/nxinit/test/test_nxinit.c | 1 + system/nxinit/test/test_nxinit.h | 1 + system/nxinit/test/test_nxinit_service.c | 63 ++++++++++++ 6 files changed, 201 insertions(+), 2 deletions(-) diff --git a/system/nxinit/Kconfig b/system/nxinit/Kconfig index 251ccae9bfa..9577c1d680d 100644 --- a/system/nxinit/Kconfig +++ b/system/nxinit/Kconfig @@ -111,6 +111,20 @@ config SYSTEM_NXINIT_SERVICE_RESTART_PERIOD int "Service restart period in ms" default 5000 +config SYSTEM_NXINIT_CONSOLE_DEV + string "Default console device of a service" + default "/dev/console" + ---help--- + The device used as stdin/stdout/stderr of a service declared with the + option "console" and no explicit device, i.e. "console" rather than + "console ". + + This is needed by services which expect a controlling terminal but do + not open one themselves (e.g. plain "sh"), and is the only way to get + a console when the console device is not available at the time the + file descriptors of the idle task are set up, as is the case for a USB + gadget console. + comment "NXInit Testing" config SYSTEM_NXINIT_TEST diff --git a/system/nxinit/service.c b/system/nxinit/service.c index 8fdb7cbe8c5..71f0b76c040 100644 --- a/system/nxinit/service.c +++ b/system/nxinit/service.c @@ -29,12 +29,14 @@ #include #include +#include #include #include #include #include #include #include +#include #include "init.h" #include "parser.h" @@ -100,6 +102,8 @@ static int option_override(FAR struct service_manager_s *sm, int argc, FAR char **argv); static int option_oneshot(FAR struct service_manager_s *sm, int argc, FAR char **argv); +static int option_console(FAR struct service_manager_s *sm, + int argc, FAR char **argv); #ifdef CONFIG_BOARDCTL_RESET static int option_reboot_on_failure(FAR struct service_manager_s *sm, int argc, FAR char **argv); @@ -116,6 +120,7 @@ static const struct cmd_map_s g_option[] = {"restart_period", 2, 2, option_restart_period}, {"override", 1, 1, option_override}, {"oneshot", 1, 1, option_oneshot}, + {"console", 1, 2, option_console}, #ifdef CONFIG_BOARDCTL_RESET {"reboot_on_failure", 2, 2, option_reboot_on_failure}, #endif @@ -128,6 +133,7 @@ static const struct flag_str_s g_flag_str[] = {SVC_ONESHOT, "oneshot"}, {SVC_RUNNING, "running"}, {SVC_RESTARTING, "restarting"}, + {SVC_CONSOLE, "console"}, {SVC_GENTLE_KILL, "gentle_kill"}, {SVC_REMOVE, "remove"}, {SVC_SIGKILL, "sigkill"}, @@ -197,6 +203,7 @@ static void remove_service(FAR struct service_s *service) free(service->argv[i]); } + free(service->console); list_delete(&service->node); free(service); } @@ -273,6 +280,43 @@ static int option_oneshot(FAR struct service_manager_s *sm, return 0; } +/**************************************************************************** + * Name: option_console + * + * Description: + * Handle the service option "console []". The service is given + * 'device' (CONFIG_SYSTEM_NXINIT_CONSOLE_DEV if omitted) as its stdin, + * stdout and stderr, so that a service which does not open a console + * device on its own still gets a working console. + * + ****************************************************************************/ + +static int option_console(FAR struct service_manager_s *sm, + int argc, FAR char **argv) +{ + FAR struct service_s *s = list_last_entry(&sm->services, struct service_s, + node); + + add_flags(s, SVC_CONSOLE); + + if (argc > 1) + { + /* 'argv' points into the parser line buffer, which is reused for the + * next line, so the device name must be duplicated here. + */ + + free(s->console); + s->console = strdup(argv[1]); + if (s->console == NULL) + { + init_err("Alloc console device"); + return -ENOMEM; + } + } + + return 0; +} + #ifdef CONFIG_BOARDCTL_RESET static int option_reboot_on_failure(FAR struct service_manager_s *sm, int argc, FAR char **argv) @@ -430,8 +474,59 @@ void init_service_reap(FAR struct service_s *service, int status) } } +/**************************************************************************** + * Name: console_file_actions + * + * Description: + * Build the spawn file actions which redirect the stdio of a service + * flagged SVC_CONSOLE to its console device. The actions are performed + * in the context of the new task, so the stdio of NxInit itself is left + * untouched. + * + ****************************************************************************/ + +static int console_file_actions(FAR posix_spawn_file_actions_t *actions, + FAR struct service_s *service) +{ + FAR const char *dev = service->console ? + service->console : CONFIG_SYSTEM_NXINIT_CONSOLE_DEV; + int ret; + + ret = posix_spawn_file_actions_init(actions); + if (ret != 0) + { + init_err("posix_spawn_file_actions_init %d", ret); + return -ret; + } + + ret = posix_spawn_file_actions_addopen(actions, STDIN_FILENO, dev, + O_RDWR, 0); + if (ret == 0) + { + ret = posix_spawn_file_actions_adddup2(actions, STDIN_FILENO, + STDOUT_FILENO); + } + + if (ret == 0) + { + ret = posix_spawn_file_actions_adddup2(actions, STDIN_FILENO, + STDERR_FILENO); + } + + if (ret != 0) + { + init_err("Add console '%s' file action %d", dev, ret); + posix_spawn_file_actions_destroy(actions); + return -ret; + } + + return 0; +} + int init_service_start(FAR struct service_s *service) { + FAR posix_spawn_file_actions_t *pactions = NULL; + posix_spawn_file_actions_t actions; posix_spawnattr_t attr; sigset_t mask; int ret; @@ -472,9 +567,27 @@ int init_service_start(FAR struct service_s *service) clock_gettime(CLOCK_MONOTONIC, &service->time_started); - ret = posix_spawnp(&pid, service->argv[2], NULL, &attr, &service->argv[2], - environ); + if (check_flags(service, SVC_CONSOLE)) + { + ret = console_file_actions(&actions, service); + if (ret < 0) + { + posix_spawnattr_destroy(&attr); + init_service_reap(service, -ret); + return ret; + } + + pactions = &actions; + } + + ret = posix_spawnp(&pid, service->argv[2], pactions, &attr, + &service->argv[2], environ); posix_spawnattr_destroy(&attr); + if (pactions != NULL) + { + posix_spawn_file_actions_destroy(pactions); + } + if (ret != 0) { init_err("Starting service '%s': %d", service->argv[1], ret); diff --git a/system/nxinit/service.h b/system/nxinit/service.h index 64520064886..c6b98dd8ecf 100644 --- a/system/nxinit/service.h +++ b/system/nxinit/service.h @@ -44,6 +44,7 @@ #define SVC_ONESHOT (1 << 1) /* do not restart on exit */ #define SVC_RUNNING (1 << 2) /* currently active */ #define SVC_RESTARTING (1 << 3) /* waiting to restart */ +#define SVC_CONSOLE (1 << 4) /* requires a console as its stdio */ /* This service should be stopped with SIGTERM instead of SIGKILL. * Will still be SIGKILLed after timeout period of 200 ms. @@ -105,6 +106,12 @@ struct service_s int restart_period; pid_t pid; + /* The device given by the service option "console". NULL means that the + * default console device is used. Only meaningful with SVC_CONSOLE. + */ + + FAR char *console; + /* The "target" of service option "reboot_on_failure" */ #ifdef CONFIG_BOARDCTL_RESET diff --git a/system/nxinit/test/test_nxinit.c b/system/nxinit/test/test_nxinit.c index cfa690464ba..21fc7d3092e 100644 --- a/system/nxinit/test/test_nxinit.c +++ b/system/nxinit/test/test_nxinit.c @@ -58,6 +58,7 @@ int main(int argc, FAR char *argv[]) cmocka_unit_test(test_nxinit_service_duplicate_conflict), cmocka_unit_test(test_nxinit_service_override_replaces_duplicate), cmocka_unit_test(test_nxinit_service_args_max_boundary), + cmocka_unit_test(test_nxinit_service_console_option), }; return cmocka_run_group_tests(nxinit_tests, test_nxinit_group_setup, diff --git a/system/nxinit/test/test_nxinit.h b/system/nxinit/test/test_nxinit.h index 3921b084ac0..06c086dd50f 100644 --- a/system/nxinit/test/test_nxinit.h +++ b/system/nxinit/test/test_nxinit.h @@ -77,5 +77,6 @@ void test_nxinit_action_event_and_semantics(FAR void **state); void test_nxinit_service_duplicate_conflict(FAR void **state); void test_nxinit_service_override_replaces_duplicate(FAR void **state); void test_nxinit_service_args_max_boundary(FAR void **state); +void test_nxinit_service_console_option(FAR void **state); #endif /* __APPS_SYSTEM_NXINIT_TEST_TEST_NXINIT_H */ diff --git a/system/nxinit/test/test_nxinit_service.c b/system/nxinit/test/test_nxinit_service.c index f732d037cd0..e7d31ee506a 100644 --- a/system/nxinit/test/test_nxinit_service.c +++ b/system/nxinit/test/test_nxinit_service.c @@ -80,6 +80,7 @@ static void service_manager_free_all(FAR struct service_manager_s *sm) free(s->argv[i]); } + free(s->console); list_delete(&s->node); free(s); } @@ -249,3 +250,65 @@ void test_nxinit_service_args_max_boundary(FAR void **state) #undef NARGS_AT_LIMIT } + +/**************************************************************************** + * Name: test_nxinit_service_console_option + * + * Description: + * The "console" option flags the service with SVC_CONSOLE. Without an + * argument the default console device is used (console == NULL); with an + * argument the device name is duplicated into the service, since the + * parser reuses its line buffer for the next line. + ****************************************************************************/ + +void test_nxinit_service_console_option(FAR void **state) +{ + struct service_manager_s sm; + struct parser_s parser = + { + "service", init_service_parse, init_service_check, &sm + }; + + char decl1[] = "service console1 /bin/sh"; + char opt_default[] = " console"; + char decl2[] = "service console2 /bin/sh"; + char opt_device[] = " console /dev/ttyACM0"; + char decl3[] = "service plain /bin/sh"; + FAR struct service_s *s; + + service_manager_init(&sm); + + /* "console" without an argument: flagged, default device. */ + + assert_int_equal(init_service_parse(&parser, true, decl1), 0); + assert_int_equal(init_service_parse(&parser, false, opt_default), 0); + + s = list_last_entry(&sm.services, struct service_s, node); + assert_int_equal(s->flags & SVC_CONSOLE, SVC_CONSOLE); + assert_null(s->console); + + /* "console ": flagged, device duplicated (not aliased into the + * caller's line buffer, which is reused for the next line). + */ + + assert_int_equal(init_service_parse(&parser, true, decl2), 0); + assert_int_equal(init_service_parse(&parser, false, opt_device), 0); + + s = list_last_entry(&sm.services, struct service_s, node); + assert_int_equal(s->flags & SVC_CONSOLE, SVC_CONSOLE); + assert_non_null(s->console); + assert_string_equal(s->console, "/dev/ttyACM0"); + assert_ptr_not_equal(s->console, opt_device + 10); + + /* A service without the option keeps its stdio untouched. */ + + assert_int_equal(init_service_parse(&parser, true, decl3), 0); + + s = list_last_entry(&sm.services, struct service_s, node); + assert_int_equal(s->flags & SVC_CONSOLE, 0); + assert_null(s->console); + + assert_int_equal(init_service_check(&parser), 0); + + service_manager_free_all(&sm); +}