From 2d6415907a1e298df8fa632b38f3d3272c31f00f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 12:55:53 +0000 Subject: [PATCH] libsystemd: graceful service stop with stop handlers and stop_timeout_ms libsystemd_stop_service() used to go straight to dmosi_process_kill(), which deletes the service's threads wherever they are - a driver I/O lock taken inside an ioctl or a half-written filesystem structure stays that way forever. A service can now register a stop request handler for its own process (libsystemd_set_stop_handler()). A stop then: 1. unregisters restart supervision (unchanged), 2. calls the handler in the stopper's context - it only sets a flag or posts a semaphore so main() leaves on its own, 3. waits up to the unit's new stop_timeout_ms key (default 3000, 0 = kill at once) for the process to terminate, 4. kills it as before if it has not, with a warning. Processes without a handler, units with stop_timeout_ms=0 and a unit stopping itself are killed at once, exactly as before. Registrations are keyed by process (so they follow libsystemd_notify_main_pid()), live in a mutex-guarded registry (src/stophandler.c) and are dropped by a process exit callback when the process terminates. Tests spawn a real helper service (tests/stoppable) in four variants: cooperative (exits with its own status well within the timeout), stubborn (killed after stop_timeout_ms), no handler and stop_timeout_ms=0 (killed at once); plus registration/clearing from the calling process. 60/60 pass. Refs choco-technologies/dmod-ecosystem#19 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TgkwHQKqf6c4DzypSKzZT7 --- README.md | 1 + app/libsystemd/CMakeLists.txt | 1 + app/libsystemd/docs/api-reference.md | 28 +- app/libsystemd/docs/configuration.md | 60 ++++ app/libsystemd/include/libsystemd.h | 1 + app/libsystemd/include/libsystemd_types.h | 13 + app/libsystemd/src/serviceapi.c | 139 +++++++- app/libsystemd/src/stophandler.c | 306 ++++++++++++++++++ app/libsystemd/src/stophandler.h | 47 +++ app/libsystemd/tests/CMakeLists.txt | 2 + .../tests/fixtures/stop/cooperative.ini | 4 + .../tests/fixtures/stop/no-grace.ini | 4 + app/libsystemd/tests/fixtures/stop/plain.ini | 3 + .../tests/fixtures/stop/stubborn.ini | 4 + app/libsystemd/tests/libsystemd_test.c | 132 ++++++++ app/libsystemd/tests/stoppable/CMakeLists.txt | 19 ++ app/libsystemd/tests/stoppable/stoppable.c | 63 ++++ 17 files changed, 821 insertions(+), 6 deletions(-) create mode 100644 app/libsystemd/src/stophandler.c create mode 100644 app/libsystemd/src/stophandler.h create mode 100644 app/libsystemd/tests/fixtures/stop/cooperative.ini create mode 100644 app/libsystemd/tests/fixtures/stop/no-grace.ini create mode 100644 app/libsystemd/tests/fixtures/stop/plain.ini create mode 100644 app/libsystemd/tests/fixtures/stop/stubborn.ini create mode 100644 app/libsystemd/tests/stoppable/CMakeLists.txt create mode 100644 app/libsystemd/tests/stoppable/stoppable.c diff --git a/README.md b/README.md index 51c0411..4c00055 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,7 @@ Recognized keys: | `description` | unit name | Implemented | Free-form text, surfaced via `libsystemd_list()`'s `libsystemd_service_info_t.description` and printed by `service list`/`service status` - not otherwise interpreted | | `type` | `simple` | Implemented | `simple` (default, expected to keep running), `oneshot` (expected to run to completion - a clean exit is logged as informational, not a warning), or `library` (`exec` names a **Library** module instead of an Application - start loads+enables it, stop disables+unloads it; no process is spawned) - see [app/libsystemd/docs/configuration.md](app/libsystemd/docs/configuration.md#service-type) | | `restart` | `no` | Implemented | `no` (default), `always`, or `on-failure` - restarts the unit when its process exits **on its own** (not on `service stop`), via `dmosi`'s process exit-callback API. See [app/libsystemd/docs/configuration.md](app/libsystemd/docs/configuration.md#restart-supervision) | +| `stop_timeout_ms` | `3000` | Implemented | How long a stop waits for a service that registered a stop handler (`libsystemd_set_stop_handler()`) to leave `main()` on its own before killing it; `0` kills at once. Services without a handler are killed at once, as before. See [app/libsystemd/docs/configuration.md](app/libsystemd/docs/configuration.md#graceful-stop) | An unset stream key leaves that stream at whatever default the spawned module would otherwise get; `stdout`/`stderr`/`stdlog` may point at the same path (as diff --git a/app/libsystemd/CMakeLists.txt b/app/libsystemd/CMakeLists.txt index 903e789..f81699d 100644 --- a/app/libsystemd/CMakeLists.txt +++ b/app/libsystemd/CMakeLists.txt @@ -31,6 +31,7 @@ set(DMOD_MAL_IMPLS ) dmod_add_library(${DMOD_MODULE_NAME} ${DMOD_MODULE_VERSION} # List of source files - can include C and C++ files src/serviceapi.c + src/stophandler.c ) target_include_directories(${DMOD_MODULE_NAME}_if diff --git a/app/libsystemd/docs/api-reference.md b/app/libsystemd/docs/api-reference.md index b112048..01ade2c 100644 --- a/app/libsystemd/docs/api-reference.md +++ b/app/libsystemd/docs/api-reference.md @@ -18,6 +18,7 @@ a quick-reference summary, not a replacement for those. | `libsystemd_service_type_t` | enum | `LIBSYSTEMD_SERVICE_TYPE_SIMPLE` (default) / `LIBSYSTEMD_SERVICE_TYPE_ONESHOT` / `LIBSYSTEMD_SERVICE_TYPE_LIBRARY` - from the unit's `type` key, see [configuration.md](configuration.md#keys). | | `libsystemd_restart_policy_t` | enum | `LIBSYSTEMD_RESTART_NO` (default) / `LIBSYSTEMD_RESTART_ALWAYS` / `LIBSYSTEMD_RESTART_ON_FAILURE` - from the unit's `restart` key, see [configuration.md](configuration.md#keys). | | `libsystemd_service_info_t` | struct | `{ const char* unit_name; const char* description; libsystemd_service_type_t type; libsystemd_restart_policy_t restart_policy; libsystemd_service_status_t status; }` - passed to `libsystemd_list()`'s visitor. | +| `libsystemd_stop_handler_t` | function pointer | `void (*)(void* arg)` - stop request handler registered with `libsystemd_set_stop_handler()`. Runs in the stopper's context; must only make the service leave and return at once. | | `libsystemd_visitor_t` | function pointer | `bool (*)(const libsystemd_service_info_t* info, void* user_ptr)` - return `false` to stop `libsystemd_list()` early. | ## Functions @@ -114,10 +115,14 @@ For a `type=library` unit: ### `libsystemd_stop_service(const char* unit_name)` -Looks `unit_name` up and stops it. For a `simple`/`oneshot` unit, kills its -tracked process (`dmosi_process_kill`); if a restart-supervision callback is -registered for the unit, unregisters it first, so a deliberate stop is never -mistaken for a crash that needs restarting. For a `type=library` unit, instead +Looks `unit_name` up and stops it. For a `simple`/`oneshot` unit, stops its +tracked process: if a restart-supervision callback is registered for the +unit, unregisters it first, so a deliberate stop is never mistaken for a +crash that needs restarting. If the process registered a stop handler +(`libsystemd_set_stop_handler()`), it is called and the process gets up to +the unit's `stop_timeout_ms` to exit on its own - the call blocks meanwhile. +Otherwise, or if it does not exit in time, it is killed +(`dmosi_process_kill`). See [Graceful stop](configuration.md#graceful-stop). For a `type=library` unit, instead disables then unloads `exec` (`Dmod_DisableModule` + `Dmod_UnloadModule`) - disable before unload, since the dmod core refuses to unload a module that is still enabled. @@ -154,6 +159,21 @@ exits right after adopting would kill what it just handed the unit to. See - `-ESRCH` - `pid` does not resolve to a live process. - `-ENOSYS` - process lookup is not connected on this build/platform. +### `libsystemd_set_stop_handler(libsystemd_stop_handler_t handler, void* arg)` + +Registers `handler` as the calling process's stop request handler, replacing +a previous one; `NULL` clears it. `libsystemd_stop_service()` calls it (in the +stopper's context) before waiting up to `stop_timeout_ms` for the process to +exit on its own, and kills the process only if it does not. The handler must +only make the service leave - set a flag, post a semaphore - and return at +once; it must not block or call into libsystemd. `arg` must stay valid until +the handler is cleared or the process has terminated; clearing waits for an +invocation still in progress. The registration is dropped automatically when +the process terminates. See [Graceful stop](configuration.md#graceful-stop). + +- `-ESRCH` - the caller is not a dmosi process. +- `-ENOMEM` - allocation failed, or the module is not initialized. + ### `libsystemd_status(const char* unit_name, libsystemd_service_status_t* out_status)` Fills `*out_status` with the unit's current process state and PID. Reports diff --git a/app/libsystemd/docs/configuration.md b/app/libsystemd/docs/configuration.md index 25259aa..64ef4d0 100644 --- a/app/libsystemd/docs/configuration.md +++ b/app/libsystemd/docs/configuration.md @@ -36,6 +36,7 @@ else (non-`.ini` files) is silently skipped. | `description` | no | Free-form text, surfaced (not parsed/interpreted) via `libsystemd_service_info_t.description` in `libsystemd_list()`, and printed by `service list`/`service status`. | | `type` | no | `simple` (default), `oneshot` or `library` - see [Service type](#service-type) below. | | `restart` | no | `no` (default), `always` or `on-failure` - see [Restart supervision](#restart-supervision) below. | +| `stop_timeout_ms` | no | How long a stop waits for a service that registered a stop handler to exit on its own before killing it. Default `3000`; `0` kills at once. See [Graceful stop](#graceful-stop) below. | A unit file with no `exec` key fails to parse (`libsystemd_parse_file()` returns `-EINVAL`); `libsystemd_parse_dir()` logs that failure and skips the @@ -145,6 +146,65 @@ Like the rest of this module, no internal locking is done - an application enabling `restart` from a multi-threaded environment is responsible for serializing its own calls into this module's API. +### Graceful stop + +By default `libsystemd_stop_service()` (`service stop`, a device removal +rule, a rescan, module unload) ends a unit's process with +`dmosi_process_kill()`: its threads are deleted wherever they happen to be. +Whatever the service held at that moment stays held - a driver's I/O lock +taken inside an ioctl, a half-written filesystem structure, a mutex shared +with another module. + +A service avoids that by registering a stop handler for its own process: + +```c +static void on_stop(void* arg) /* runs in the stopper's context */ +{ + my_service_t* s = arg; + s->stop = true; /* only make main() leave ... */ + dmosi_semaphore_post(s->wakeup, 1); /* ... and wake it if it is waiting */ +} + +int main(int argc, char* argv[]) +{ + ... + libsystemd_set_stop_handler(on_stop, &s); + while (!s.stop) + { + dmosi_semaphore_wait(s.wakeup, 1, -1); + do_work(&s); /* never interrupted halfway */ + } + libsystemd_set_stop_handler(NULL, NULL); /* before freeing &s */ + cleanup(&s); + return 0; +} +``` + +A stop then runs: + +1. The restart-supervision callback is unregistered (a stop never triggers a + restart). +2. The process's stop handler is called, in the caller's context. It must + return at once - no blocking, no calls into libsystemd. +3. libsystemd waits up to `stop_timeout_ms` for the process to terminate on + its own (return from `main()`, so its cleanup runs). +4. If it has not, it is killed with `dmosi_process_kill()` as before, and a + warning is logged. + +A process without a handler, a unit with `stop_timeout_ms=0`, and a unit +stopping itself (the caller is the unit's own process) skip steps 2-3 and +are killed at once - exactly the behavior before stop handlers existed. + +The stop call blocks for up to `stop_timeout_ms` while it waits. Keep the +timeout above the longest piece of work the service does between two checks +of its stop flag. + +The registration belongs to the calling *process*, not to a unit name, so it +follows the process a unit tracks - including one adopted with +`libsystemd_notify_main_pid()`. It is dropped automatically when the process +terminates. Clear it (`NULL` handler) before freeing the handler's argument: +that call waits for a handler invocation still in progress. + ## Templates `libsystemd` supports `systemd`-style template units, identified by a literal diff --git a/app/libsystemd/include/libsystemd.h b/app/libsystemd/include/libsystemd.h index 2dbf35e..e58c4b0 100644 --- a/app/libsystemd/include/libsystemd.h +++ b/app/libsystemd/include/libsystemd.h @@ -8,6 +8,7 @@ dmod_libsystemd_api(1.0, int, _start_service, ( const char* unit_name, const char* user_value )); dmod_libsystemd_api(1.0, int, _stop_service, ( const char* unit_name )); dmod_libsystemd_api(1.0, int, _notify_main_pid, ( const char* unit_name, Dmod_Pid_t pid )); +dmod_libsystemd_api(1.0, int, _set_stop_handler, ( libsystemd_stop_handler_t handler, void* arg )); dmod_libsystemd_api(1.0, int, _status, ( const char* unit_name, libsystemd_service_status_t* out_status )); dmod_libsystemd_api(1.0, int, _list, (libsystemd_visitor_t visitor, void* user_ptr)); diff --git a/app/libsystemd/include/libsystemd_types.h b/app/libsystemd/include/libsystemd_types.h index 4d4da23..9318dd9 100644 --- a/app/libsystemd/include/libsystemd_types.h +++ b/app/libsystemd/include/libsystemd_types.h @@ -48,6 +48,19 @@ typedef struct libsystemd_service_status_t status; } libsystemd_service_info_t; +/** + * @brief Stop request handler registered by a service with libsystemd_set_stop_handler() + * + * Called by libsystemd_stop_service() in the *stopper's* context (whichever + * thread asked for the stop), before it waits for the service's process to + * exit. It must only make the service leave on its own - set a flag, post a + * semaphore the service is waiting on - and return at once: it must not + * block, and must not call back into libsystemd. + * + * @param arg The pointer passed to libsystemd_set_stop_handler(). + */ +typedef void (*libsystemd_stop_handler_t)(void* arg); + /** * @brief pointer to a visitor function */ diff --git a/app/libsystemd/src/serviceapi.c b/app/libsystemd/src/serviceapi.c index 1b00ee4..c134db4 100644 --- a/app/libsystemd/src/serviceapi.c +++ b/app/libsystemd/src/serviceapi.c @@ -8,6 +8,21 @@ // its surface (see the libsystemd_services_t doc in libsystemd_types.h), so the // registry's own list handling includes it here instead. #include "dmlist.h" +#include "stophandler.h" + +/** + * @brief Default for a unit's "stop_timeout_ms" key + * + * How long libsystemd_stop_service() waits for a service that registered a + * stop handler (libsystemd_set_stop_handler()) to exit on its own before it + * falls back to dmosi_process_kill(). + */ +#define LIBSYSTEMD_DEFAULT_STOP_TIMEOUT_MS 3000u + +/** + * @brief Poll period while waiting for a stopping service to exit + */ +#define LIBSYSTEMD_STOP_POLL_MS 10u /** * @brief Full definition of the opaque @ref libsystemd_service_t handle @@ -29,6 +44,7 @@ struct libsystemd_service char* description; //!< From the optional "description" key, or NULL if unset. Owned copy. libsystemd_service_type_t type; //!< From the optional "type" key ("simple"/"oneshot"). Defaults to ::LIBSYSTEMD_SERVICE_TYPE_SIMPLE. libsystemd_restart_policy_t restart_policy; //!< From the optional "restart" key ("no"/"always"/"on-failure"). Defaults to ::LIBSYSTEMD_RESTART_NO. + uint32_t stop_timeout_ms; //!< From the optional "stop_timeout_ms" key: how long a stop waits for a service with a stop handler to exit on its own before killing it. 0 = kill at once. Defaults to ::LIBSYSTEMD_DEFAULT_STOP_TIMEOUT_MS. char* exec; //!< Module name (or file path) to spawn, from the "exec" key. Owned copy. int argc; //!< Number of entries in argv (always >= 1, argv[0] == exec). char** argv; //!< NULL-terminated argument vector (argc+1 entries, each an owned copy). @@ -1145,6 +1161,73 @@ static bool libsystemd_start_service_visitor(void* data, void* user_data) return true; } +/** + * @brief Wait until a process has terminated (or can no longer be found) + * + * The process is looked up by PID on every poll rather than held as a + * `dmosi_process_t`, so its structure going away while waiting is harmless. + * + * @param pid Process to wait for. + * @param timeout_ms Upper bound of the wait. + * + * @retval true The process is gone. + * @retval false It is still running after @p timeout_ms. + */ +static bool libsystemd_wait_for_exit(dmosi_process_id_t pid, uint32_t timeout_ms) +{ + uint32_t waited = 0; + for (;;) + { + dmosi_process_t process = dmosi_process_find_by_id(pid); + if (process == NULL || dmosi_process_get_state(process) == DMOSI_PROCESS_STATE_TERMINATED) + { + return true; + } + if (waited >= timeout_ms) + { + return false; + } + dmosi_thread_sleep(LIBSYSTEMD_STOP_POLL_MS); + waited += LIBSYSTEMD_STOP_POLL_MS; + } +} + +/** + * @brief Ask a service's process to exit on its own, through its stop handler + * + * Skipped (returns false at once) when the unit's `stop_timeout_ms` is 0, when + * the process registered no handler with libsystemd_set_stop_handler(), and + * when the process is the caller itself - waiting for our own exit could only + * ever time out. + * + * @param service Service whose tracked `pid` is being stopped (must not be NULL). + * + * @retval true The process exited on its own within `stop_timeout_ms`. + * @retval false It has to be killed (no handler, disabled, or it did not exit in time). + */ +static bool libsystemd_request_graceful_stop(libsystemd_service_t service) +{ + dmosi_process_id_t pid = (dmosi_process_id_t)service->pid; + dmosi_process_t self = dmosi_process_current(); + + if (service->stop_timeout_ms == 0 || (self != NULL && dmosi_process_get_id(self) == pid)) + { + return false; + } + if (!libsystemd_stophandler_request(pid)) + { + return false; + } + if (libsystemd_wait_for_exit(pid, service->stop_timeout_ms)) + { + return true; + } + + DMOD_LOG_WARN("Service '%s' did not stop within %u ms, killing it\n", + service->unit_name, (unsigned)service->stop_timeout_ms); + return false; +} + /** * @brief Actually stop a service's process, without looking it up by name first * @@ -1152,7 +1235,11 @@ static bool libsystemd_start_service_visitor(void* data, void* user_data) * first) and libsystemd_stop_all_services_visitor() (which already has a direct * pointer while walking a registry that is about to be replaced/torn down). * - * Resolves `service->pid` to a live `dmosi_process_t` and kills it via + * Resolves `service->pid` to a live `dmosi_process_t`. If that process + * registered a stop handler (libsystemd_set_stop_handler()), it is asked to + * exit on its own first and given up to the unit's `stop_timeout_ms` + * (libsystemd_request_graceful_stop()) - this blocks the caller for at most + * that long. Otherwise, or if it does not exit in time, it is killed via * `dmosi_process_kill()`. Always clears `service->pid` back to the "not * running" sentinel (-1) once the process is confirmed gone or killed, so a * subsequent libsystemd_start_service() call is never blocked by stale state. @@ -1165,7 +1252,7 @@ static bool libsystemd_start_service_visitor(void* data, void* user_data) * * @param service Service to stop (must not be NULL). * - * @retval 0 The service's process was found and killed successfully. + * @retval 0 The service's process was found and exited on request or was killed successfully. * @retval -ESRCH The service had no running process to stop (`pid` was already * <= 0, or the process it pointed to could no longer be found - * e.g. it already exited on its own). @@ -1203,6 +1290,20 @@ static int libsystemd_stop_service_internal(libsystemd_service_t service) DMOD_LOG_INFO("Stopping service '%s'\n", service->unit_name); + if (libsystemd_request_graceful_stop(service)) + { + service->pid = -1; + return 0; + } + + // Looked up again: after a graceful attempt the old handle may be stale. + process = dmosi_process_find_by_id((dmosi_process_id_t)service->pid); + if (process == NULL) + { + service->pid = -1; + return 0; + } + int result = dmosi_process_kill(process, 0); if (result != 0) { @@ -2177,6 +2278,28 @@ static libsystemd_restart_policy_t libsystemd_parse_restart_policy(dmini_context return LIBSYSTEMD_RESTART_NO; } +/** + * @brief Parse a unit's optional "stop_timeout_ms" key + * + * @param ctx Unit's ini context (must not be NULL). + * @param exec The unit's `exec` value, used only in the warning message. + * + * @return The configured timeout, or ::LIBSYSTEMD_DEFAULT_STOP_TIMEOUT_MS if + * the key is absent or negative (the latter logged as a warning). + */ +static uint32_t libsystemd_parse_stop_timeout(dmini_context_t ctx, const char* exec) +{ + int value = dmini_get_int(ctx, NULL, "stop_timeout_ms", (int)LIBSYSTEMD_DEFAULT_STOP_TIMEOUT_MS); + if (value < 0) + { + DMOD_LOG_WARN("Unit with exec '%s' has negative stop_timeout_ms %d, using %u\n", + exec, value, (unsigned)LIBSYSTEMD_DEFAULT_STOP_TIMEOUT_MS); + return LIBSYSTEMD_DEFAULT_STOP_TIMEOUT_MS; + } + + return (uint32_t)value; +} + /** * @brief Build a fresh `libsystemd_service_t` from an already-parsed ini context * @@ -2215,6 +2338,7 @@ static int libsystemd_build_service_from_ctx(dmini_context_t ctx, libsystemd_ser new_service->description = NULL; new_service->type = LIBSYSTEMD_SERVICE_TYPE_SIMPLE; new_service->restart_policy = LIBSYSTEMD_RESTART_NO; + new_service->stop_timeout_ms = LIBSYSTEMD_DEFAULT_STOP_TIMEOUT_MS; new_service->exec = NULL; new_service->argc = 0; new_service->argv = NULL; @@ -2232,6 +2356,7 @@ static int libsystemd_build_service_from_ctx(dmini_context_t ctx, libsystemd_ser new_service->type = libsystemd_parse_service_type(ctx, exec); new_service->restart_policy = libsystemd_parse_restart_policy(ctx, exec); + new_service->stop_timeout_ms = libsystemd_parse_stop_timeout(ctx, exec); bool ok = (new_service->exec = Dmod_StrDup(exec)) != NULL; ok = ok && libsystemd_build_argv(new_service, exec, args); @@ -2906,6 +3031,13 @@ static int libsystemd_serviceapi_init(void) return -ENOMEM; } + int result = libsystemd_stophandler_init(); + if (result != 0) + { + libsystemd_destroy_services(services); + return result; + } + g_services = services; return 0; @@ -2945,6 +3077,9 @@ static void libsystemd_serviceapi_deinit(void) libsystemd_destroy_devices(g_devices); g_devices = NULL; + + // Last: stopping services above may still ask their stop handlers. + libsystemd_stophandler_deinit(); } /** diff --git a/app/libsystemd/src/stophandler.c b/app/libsystemd/src/stophandler.c new file mode 100644 index 0000000..5509a14 --- /dev/null +++ b/app/libsystemd/src/stophandler.c @@ -0,0 +1,306 @@ +#include "stophandler.h" +#include +#include "dmosi.h" +#include "dmlist.h" + +/** + * @brief Magic value of a live registry entry ('STOP') + */ +#define LIBSYSTEMD_STOPHANDLER_MAGIC 0x53544F50u + +/** + * @brief One process's stop handler registration + * + * Created by libsystemd_set_stop_handler() for the calling process and + * removed when that process unregisters (NULL handler), when it terminates + * (process exit callback, see libsystemd_stophandler_process_exited()), or + * when the module is unloaded. + */ +typedef struct +{ + uint32_t magic; //!< ::LIBSYSTEMD_STOPHANDLER_MAGIC while registered. + dmosi_process_id_t pid; //!< Process that registered the handler. + libsystemd_stop_handler_t handler; //!< Handler to call on a stop request. + void* arg; //!< Argument passed to @ref handler. + dmosi_process_exit_callback_handle_t exit_handle; //!< Exit callback that drops this entry when the process terminates. +} libsystemd_stophandler_t; + +/** + * @brief Registered stop handlers (list data pointers are libsystemd_stophandler_t) + * + * Unlike the unit registry in serviceapi.c, this list *is* touched from + * several threads at once - services register from their own process while + * the stopper (systemd, `service`, a device removal) looks handlers up - so + * every access goes through @ref g_stophandlers_lock. + */ +static dmlist_context_t* g_stophandlers = NULL; + +/** + * @brief Guards @ref g_stophandlers and every entry in it + */ +static dmosi_mutex_t g_stophandlers_lock = NULL; + +/** + * @brief dmlist compare function matching an entry by process ID + * + * @param data1 Entry from the list (`libsystemd_stophandler_t*`). + * @param data2 `const dmosi_process_id_t*` to look for. + * + * @retval 0 The entry belongs to that process. + * @retval !0 It does not. + */ +static int libsystemd_stophandler_compare_pid(const void* data1, const void* data2) +{ + const libsystemd_stophandler_t* entry = (const libsystemd_stophandler_t*)data1; + const dmosi_process_id_t* pid = (const dmosi_process_id_t*)data2; + + return (entry->pid == *pid) ? 0 : 1; +} + +/** + * @brief Lock held: remove and return the entry of @p pid, or NULL + */ +static libsystemd_stophandler_t* libsystemd_stophandler_take(dmosi_process_id_t pid) +{ + libsystemd_stophandler_t* entry = dmlist_find(g_stophandlers, &pid, libsystemd_stophandler_compare_pid); + if (entry != NULL) + { + dmlist_remove(g_stophandlers, &pid, libsystemd_stophandler_compare_pid); + } + + return entry; +} + +/** + * @brief Free an entry that is no longer in the list + */ +static void libsystemd_stophandler_free(libsystemd_stophandler_t* entry) +{ + entry->magic = 0; + Dmod_Free(entry); +} + +/** + * @brief dmosi process exit callback: forget the handler of a terminated process + * + * The exit callback list of @p process is already detached by dmosi when this + * runs, so the entry's @ref libsystemd_stophandler_t::exit_handle must not be + * unregistered here - only the entry itself is dropped. + * + * @param process Terminated process (unused - the entry carries its PID). + * @param exit_status Unused. + * @param arg The `libsystemd_stophandler_t` registered for it. + */ +static void libsystemd_stophandler_process_exited(dmosi_process_t process, int exit_status, void* arg) +{ + (void)process; + (void)exit_status; + + libsystemd_stophandler_t* registered = (libsystemd_stophandler_t*)arg; + if (g_stophandlers_lock == NULL || registered == NULL || registered->magic != LIBSYSTEMD_STOPHANDLER_MAGIC) + { + return; + } + + dmosi_mutex_lock(g_stophandlers_lock); + libsystemd_stophandler_t* entry = libsystemd_stophandler_take(registered->pid); + dmosi_mutex_unlock(g_stophandlers_lock); + + if (entry != NULL) + { + libsystemd_stophandler_free(entry); + } +} + +/** + * @brief Lock held: register a new entry for @p process + * + * @retval 0 Registered. + * @retval -ENOMEM Allocation or exit callback registration failed; nothing is left behind. + */ +static int libsystemd_stophandler_add(dmosi_process_t process, libsystemd_stop_handler_t handler, void* arg) +{ + libsystemd_stophandler_t* entry = Dmod_Malloc(sizeof(libsystemd_stophandler_t)); + if (entry == NULL) + { + return -ENOMEM; + } + + entry->magic = LIBSYSTEMD_STOPHANDLER_MAGIC; + entry->pid = dmosi_process_get_id(process); + entry->handler = handler; + entry->arg = arg; + entry->exit_handle = dmosi_process_register_exit_callback(process, libsystemd_stophandler_process_exited, entry); + + if (entry->exit_handle == NULL || !dmlist_push_back(g_stophandlers, entry)) + { + if (entry->exit_handle != NULL) + { + dmosi_process_unregister_exit_callback(process, entry->exit_handle); + } + libsystemd_stophandler_free(entry); + return -ENOMEM; + } + + return 0; +} + +/** + * @brief Lock held: drop the entry of @p process, if it has one + */ +static void libsystemd_stophandler_remove(dmosi_process_t process) +{ + libsystemd_stophandler_t* entry = libsystemd_stophandler_take(dmosi_process_get_id(process)); + if (entry == NULL) + { + return; + } + + dmosi_process_unregister_exit_callback(process, entry->exit_handle); + libsystemd_stophandler_free(entry); +} + +int libsystemd_stophandler_init(void) +{ + if (g_stophandlers != NULL) + { + return 0; + } + + g_stophandlers = dmlist_create(); + g_stophandlers_lock = dmosi_mutex_create(false); + if (g_stophandlers == NULL || g_stophandlers_lock == NULL) + { + libsystemd_stophandler_deinit(); + return -ENOMEM; + } + + return 0; +} + +void libsystemd_stophandler_deinit(void) +{ + if (g_stophandlers != NULL) + { + libsystemd_stophandler_t* entry = dmlist_pop_front(g_stophandlers); + while (entry != NULL) + { + dmosi_process_t process = dmosi_process_find_by_id(entry->pid); + if (process != NULL) + { + dmosi_process_unregister_exit_callback(process, entry->exit_handle); + } + libsystemd_stophandler_free(entry); + entry = dmlist_pop_front(g_stophandlers); + } + dmlist_destroy(g_stophandlers); + g_stophandlers = NULL; + } + + if (g_stophandlers_lock != NULL) + { + dmosi_mutex_destroy(g_stophandlers_lock); + g_stophandlers_lock = NULL; + } +} + +bool libsystemd_stophandler_request(dmosi_process_id_t pid) +{ + if (g_stophandlers_lock == NULL) + { + return false; + } + + dmosi_mutex_lock(g_stophandlers_lock); + libsystemd_stophandler_t* entry = dmlist_find(g_stophandlers, &pid, libsystemd_stophandler_compare_pid); + bool requested = (entry != NULL); + if (requested) + { + entry->handler(entry->arg); + } + dmosi_mutex_unlock(g_stophandlers_lock); + + return requested; +} + +/** + * @brief Register (or clear) the calling process's stop request handler + * + * By default libsystemd_stop_service() ends a unit with dmosi_process_kill(): + * the process's threads are deleted wherever they happen to be, so a lock the + * service holds at that moment (a driver's I/O lock, a filesystem's + * metadata) stays held forever. A service that registers a handler is asked + * first instead: libsystemd calls @p handler, then waits up to the unit's + * `stop_timeout_ms` for the process to exit on its own (return from main()), + * and only kills it if it has not. + * + * The registration belongs to the calling process - whatever unit tracks it + * as its main PID (including one adopted via libsystemd_notify_main_pid()). + * It is dropped automatically when the process terminates. + * + * @param handler Called on a stop request, in the stopper's context. It must + * only make the service leave (set a flag, post a semaphore + * the service waits on) and return at once - no blocking, no + * calls into libsystemd. NULL clears the registration. + * @param arg Passed to @p handler. It must stay valid until the handler + * is cleared or the process has terminated: clear it (NULL + * handler) before freeing @p arg - that call waits for a + * handler invocation in progress to return. + * + * @retval 0 Registered, replaced, or cleared (clearing an absent registration is not an error). + * @retval -ESRCH The caller is not a dmosi process. + * @retval -ENOMEM Allocation failed, or the libsystemd module is not initialized. + * + * @par Example + * @code + * static void on_stop(void* arg) + * { + * service_t* s = arg; + * s->stop = true; + * dmosi_semaphore_post(s->wakeup, 1); + * } + * + * libsystemd_set_stop_handler(on_stop, &s); + * while (!s.stop) { + * dmosi_semaphore_wait(s.wakeup, 1, -1); + * ... + * } + * libsystemd_set_stop_handler(NULL, NULL); + * cleanup(&s); + * return 0; + * @endcode + */ +dmod_libsystemd_api_declaration(1.0, int, _set_stop_handler, ( libsystemd_stop_handler_t handler, void* arg )) +{ + dmosi_process_t self = dmosi_process_current(); + if (self == NULL) + { + return -ESRCH; + } + if (g_stophandlers_lock == NULL) + { + return -ENOMEM; + } + + dmosi_process_id_t pid = dmosi_process_get_id(self); + int result = 0; + + dmosi_mutex_lock(g_stophandlers_lock); + libsystemd_stophandler_t* entry = dmlist_find(g_stophandlers, &pid, libsystemd_stophandler_compare_pid); + if (handler == NULL) + { + libsystemd_stophandler_remove(self); + } + else if (entry != NULL) + { + entry->handler = handler; + entry->arg = arg; + } + else + { + result = libsystemd_stophandler_add(self, handler, arg); + } + dmosi_mutex_unlock(g_stophandlers_lock); + + return result; +} diff --git a/app/libsystemd/src/stophandler.h b/app/libsystemd/src/stophandler.h new file mode 100644 index 0000000..da85aeb --- /dev/null +++ b/app/libsystemd/src/stophandler.h @@ -0,0 +1,47 @@ +#ifndef LIBSYSTEMD_STOPHANDLER_H +#define LIBSYSTEMD_STOPHANDLER_H + +#include +#include "libsystemd.h" + +/** + * @file stophandler.h + * @brief Private interface of the stop handler registry (see stophandler.c) + * + * Maps a process ID to the libsystemd_stop_handler_t that process registered + * for itself with libsystemd_set_stop_handler(). Used by + * libsystemd_stop_service_internal() (serviceapi.c) to ask a service to leave + * on its own before falling back to dmosi_process_kill(). + */ + +/** + * @brief Allocate the registry (called from libsystemd_serviceapi_init()) + * + * @retval 0 Ready (or already initialized). + * @retval -ENOMEM Allocation failed. + */ +int libsystemd_stophandler_init(void); + +/** + * @brief Release the registry (called from libsystemd_serviceapi_deinit()) + * + * Unregisters the process exit callbacks still attached to live processes, so + * no callback into this module outlives it. + */ +void libsystemd_stophandler_deinit(void); + +/** + * @brief Ask a process to stop through the handler it registered, if any + * + * The handler runs in the caller's context with the registry locked, so a + * concurrent libsystemd_set_stop_handler() (e.g. the service unregistering + * before it frees the handler's argument) waits until the call returns. + * + * @param pid Process to ask. + * + * @retval true A handler was registered for @p pid and has been called. + * @retval false No handler is registered for @p pid (nothing was called). + */ +bool libsystemd_stophandler_request(dmosi_process_id_t pid); + +#endif // LIBSYSTEMD_STOPHANDLER_H diff --git a/app/libsystemd/tests/CMakeLists.txt b/app/libsystemd/tests/CMakeLists.txt index fa1d3a4..9e91831 100644 --- a/app/libsystemd/tests/CMakeLists.txt +++ b/app/libsystemd/tests/CMakeLists.txt @@ -4,6 +4,8 @@ set(DMOD_AUTHOR_NAME "Patryk Kubiak") set(DMOD_STACK_SIZE 1024) set(DMOD_PRIORITY 0) +add_subdirectory(stoppable) + dmod_add_test(${DMOD_MODULE_NAME} ${DMOD_MODULE_VERSION} libsystemd_test.c ) diff --git a/app/libsystemd/tests/fixtures/stop/cooperative.ini b/app/libsystemd/tests/fixtures/stop/cooperative.ini new file mode 100644 index 0000000..27ed7be --- /dev/null +++ b/app/libsystemd/tests/fixtures/stop/cooperative.ini @@ -0,0 +1,4 @@ +# Registers a stop handler and leaves main() when asked (see stoppable/stoppable.c) +exec=test_stoppable +args=cooperative +stop_timeout_ms=2000 diff --git a/app/libsystemd/tests/fixtures/stop/no-grace.ini b/app/libsystemd/tests/fixtures/stop/no-grace.ini new file mode 100644 index 0000000..1eed49b --- /dev/null +++ b/app/libsystemd/tests/fixtures/stop/no-grace.ini @@ -0,0 +1,4 @@ +# Cooperative, but stop_timeout_ms=0 disables the graceful path +exec=test_stoppable +args=cooperative +stop_timeout_ms=0 diff --git a/app/libsystemd/tests/fixtures/stop/plain.ini b/app/libsystemd/tests/fixtures/stop/plain.ini new file mode 100644 index 0000000..6f85a68 --- /dev/null +++ b/app/libsystemd/tests/fixtures/stop/plain.ini @@ -0,0 +1,3 @@ +# No stop handler - killed at once, as before stop handlers existed +exec=test_stoppable +args=plain diff --git a/app/libsystemd/tests/fixtures/stop/stubborn.ini b/app/libsystemd/tests/fixtures/stop/stubborn.ini new file mode 100644 index 0000000..3673961 --- /dev/null +++ b/app/libsystemd/tests/fixtures/stop/stubborn.ini @@ -0,0 +1,4 @@ +# Registers a stop handler but ignores it - must be killed after stop_timeout_ms +exec=test_stoppable +args=stubborn +stop_timeout_ms=300 diff --git a/app/libsystemd/tests/libsystemd_test.c b/app/libsystemd/tests/libsystemd_test.c index fb9689b..e92e8af 100644 --- a/app/libsystemd/tests/libsystemd_test.c +++ b/app/libsystemd/tests/libsystemd_test.c @@ -776,3 +776,135 @@ DMOD_TEST_STEP(notify_device_added_substitutes_user_value_specifier) DMOD_TEST_EXPECT_NOT_NULL(state.info.description); DMOD_TEST_EXPECT_EQ(strcmp(state.info.description, "/dev/ttyPATH1"), 0); } + +/* + * Graceful stop (libsystemd_set_stop_handler() + "stop_timeout_ms"). + * + * tests/fixtures/stop/ holds four units running the helper Application + * tests/stoppable (test_stoppable), which libsystemd_scan() starts right away: + * "cooperative" leaves main() when asked (exit status + * LIBSYSTEMD_TEST_STOPPABLE_EXIT), "stubborn" registers a handler but ignores + * it, "plain" registers none, and "no-grace" is cooperative but has + * stop_timeout_ms=0. + */ +#define LIBSYSTEMD_STOP_FIXTURES_DIR LIBSYSTEMD_TEST_FIXTURES_DIR "/stop" +#define LIBSYSTEMD_TEST_STOPPABLE_EXIT 42 +#define LIBSYSTEMD_TEST_STUBBORN_MS 300 + +/** + * @brief Wait until @p unit_name's process is running (the helper got as far as main()) + */ +static bool wait_until_running(const char* unit_name) +{ + for (int i = 0; i < 100; i++) + { + libsystemd_service_status_t status; + if (libsystemd_status(unit_name, &status) == 0 && status.state == DMOSI_PROCESS_STATE_RUNNING) + { + dmosi_thread_sleep(100); /* let it reach its stop handler registration */ + return true; + } + dmosi_thread_sleep(20); + } + return false; +} + +/** + * @brief Stop @p unit_name and report how long libsystemd_stop_service() took + */ +static uint64_t stop_and_measure(const char* unit_name, int* result) +{ + uint64_t start = (uint64_t)Dmod_GetUptime(); + *result = libsystemd_stop_service(unit_name); + return (uint64_t)Dmod_GetUptime() - start; +} + +/** + * @brief Exit status of the unit's last process, or -1 if it is no longer known to dmosi + */ +static int last_exit_status(const char* unit_name) +{ + libsystemd_service_status_t status; + if (libsystemd_status(unit_name, &status) != 0) + { + return -1; + } + dmosi_process_t process = dmosi_process_find_by_id(status.pid); + return (process != NULL) ? dmosi_process_get_exit_status(process) : -1; +} + +DMOD_TEST_STEP(stop_service_lets_cooperative_service_exit_on_its_own) +{ + DMOD_TEST_EXPECT_EQ(libsystemd_scan(LIBSYSTEMD_STOP_FIXTURES_DIR), 0); + DMOD_TEST_EXPECT_TRUE(wait_until_running("cooperative")); + + int result = -1; + uint64_t elapsed = stop_and_measure("cooperative", &result); + DMOD_TEST_EXPECT_EQ(result, 0); + DMOD_TEST_EXPECT_TRUE(elapsed < 2000); + + libsystemd_service_status_t status; + DMOD_TEST_EXPECT_EQ(libsystemd_status("cooperative", &status), 0); + DMOD_TEST_EXPECT_EQ(status.state, DMOSI_PROCESS_STATE_TERMINATED); + /* main() returned - it was not killed */ + DMOD_TEST_EXPECT_EQ(last_exit_status("cooperative"), LIBSYSTEMD_TEST_STOPPABLE_EXIT); +} + +DMOD_TEST_STEP(stop_service_kills_service_ignoring_request_after_timeout) +{ + DMOD_TEST_EXPECT_EQ(libsystemd_scan(LIBSYSTEMD_STOP_FIXTURES_DIR), 0); + DMOD_TEST_EXPECT_TRUE(wait_until_running("stubborn")); + + int result = -1; + uint64_t elapsed = stop_and_measure("stubborn", &result); + DMOD_TEST_EXPECT_EQ(result, 0); + DMOD_TEST_EXPECT_TRUE(elapsed >= LIBSYSTEMD_TEST_STUBBORN_MS); + + libsystemd_service_status_t status; + DMOD_TEST_EXPECT_EQ(libsystemd_status("stubborn", &status), 0); + DMOD_TEST_EXPECT_EQ(status.state, DMOSI_PROCESS_STATE_TERMINATED); + DMOD_TEST_EXPECT_TRUE(last_exit_status("stubborn") != LIBSYSTEMD_TEST_STOPPABLE_EXIT); +} + +DMOD_TEST_STEP(stop_service_kills_service_without_handler_at_once) +{ + DMOD_TEST_EXPECT_EQ(libsystemd_scan(LIBSYSTEMD_STOP_FIXTURES_DIR), 0); + DMOD_TEST_EXPECT_TRUE(wait_until_running("plain")); + + int result = -1; + uint64_t elapsed = stop_and_measure("plain", &result); + DMOD_TEST_EXPECT_EQ(result, 0); + DMOD_TEST_EXPECT_TRUE(elapsed < LIBSYSTEMD_TEST_STUBBORN_MS); + DMOD_TEST_EXPECT_TRUE(last_exit_status("plain") != LIBSYSTEMD_TEST_STOPPABLE_EXIT); +} + +DMOD_TEST_STEP(stop_service_skips_handler_when_stop_timeout_is_zero) +{ + DMOD_TEST_EXPECT_EQ(libsystemd_scan(LIBSYSTEMD_STOP_FIXTURES_DIR), 0); + DMOD_TEST_EXPECT_TRUE(wait_until_running("no-grace")); + + int result = -1; + stop_and_measure("no-grace", &result); + DMOD_TEST_EXPECT_EQ(result, 0); + DMOD_TEST_EXPECT_TRUE(last_exit_status("no-grace") != LIBSYSTEMD_TEST_STOPPABLE_EXIT); +} + +static void count_stop_requests(void* arg) +{ + (*(int*)arg)++; +} + +DMOD_TEST_STEP(set_stop_handler_registers_and_clears_for_calling_process) +{ + if (own_pid() <= 0) + { + return; /* no resolvable current process on this platform */ + } + + int requests = 0; + DMOD_TEST_EXPECT_EQ(libsystemd_set_stop_handler(NULL, NULL), 0); /* nothing to clear */ + DMOD_TEST_EXPECT_EQ(libsystemd_set_stop_handler(count_stop_requests, &requests), 0); + DMOD_TEST_EXPECT_EQ(libsystemd_set_stop_handler(count_stop_requests, &requests), 0); /* replace */ + DMOD_TEST_EXPECT_EQ(libsystemd_set_stop_handler(NULL, NULL), 0); + DMOD_TEST_EXPECT_EQ(requests, 0); /* registering never calls it */ +} diff --git a/app/libsystemd/tests/stoppable/CMakeLists.txt b/app/libsystemd/tests/stoppable/CMakeLists.txt new file mode 100644 index 0000000..eae8230 --- /dev/null +++ b/app/libsystemd/tests/stoppable/CMakeLists.txt @@ -0,0 +1,19 @@ +# test_stoppable - helper Application spawned by test_libsystemd's stop steps +# (fixtures/stop/*.ini). Never released. +set(DMOD_MODULE_NAME test_stoppable) +set(DMOD_AUTHOR_NAME "Patryk Kubiak") +set(DMOD_STACK_SIZE 1024) +set(DMOD_PRIORITY 0) + +dmod_add_executable(${DMOD_MODULE_NAME} ${DMOD_MODULE_VERSION} + stoppable.c +) + +dmod_link_modules(${DMOD_MODULE_NAME} + dmosi + dmlist +) + +target_link_libraries(${DMOD_MODULE_NAME} + libsystemd_if + ) diff --git a/app/libsystemd/tests/stoppable/stoppable.c b/app/libsystemd/tests/stoppable/stoppable.c new file mode 100644 index 0000000..8e917ad --- /dev/null +++ b/app/libsystemd/tests/stoppable/stoppable.c @@ -0,0 +1,63 @@ +#include "dmod.h" +#include "dmosi.h" +#include "libsystemd.h" +#include +#include + +/* + * Stand-in service for the graceful stop steps in libsystemd_test.c. + * + * test_stoppable cooperative registers a stop handler that wakes the main + * loop; returns TEST_STOPPABLE_EXIT_STATUS + * test_stoppable stubborn registers a handler that ignores the request + * test_stoppable plain registers nothing (killed right away) + */ + +/** Returned from main() when leaving on a stop request - distinguishes a clean exit from a kill (status 0). */ +#define TEST_STOPPABLE_EXIT_STATUS 42 + +typedef struct +{ + dmosi_semaphore_t wakeup; + volatile bool stop; +} stoppable_t; + +static void on_stop(void* arg) +{ + stoppable_t* s = (stoppable_t*)arg; + s->stop = true; + dmosi_semaphore_post(s->wakeup, 1); +} + +static void ignore_stop(void* arg) +{ + (void)arg; +} + +int main(int argc, char* argv[]) +{ + const char* mode = (argc > 1) ? argv[1] : "plain"; + stoppable_t s = { .wakeup = dmosi_semaphore_create(0, 1), .stop = false }; + if (s.wakeup == NULL) + { + return -ENOMEM; + } + + if (strcmp(mode, "cooperative") == 0) + { + libsystemd_set_stop_handler(on_stop, &s); + } + else if (strcmp(mode, "stubborn") == 0) + { + libsystemd_set_stop_handler(ignore_stop, &s); + } + + while (!s.stop) + { + dmosi_semaphore_wait(s.wakeup, 1, 1000); + } + + libsystemd_set_stop_handler(NULL, NULL); + dmosi_semaphore_destroy(s.wakeup); + return TEST_STOPPABLE_EXIT_STATUS; +}