From d20b2f6976698af4ce4af175b2feedfb452fbee4 Mon Sep 17 00:00:00 2001 From: Shadowtrance Date: Sat, 24 Oct 2026 13:05:43 +1000 Subject: [PATCH 1/2] Fix Apps stopping & starting lvgl Fixes apps like Doom, GraphicsDemo, MystifyDemo etc being able to stop and start lvgl properly. After the kernel modules in apps addition it was broken and the mentioned apps would start and run but lvgl would still linger in the background. --- Modules/app-esp32-module/source/module.cpp | 10 ++- Modules/app-esp32-module/source/module_refs.h | 2 +- TactilityKernel/include/tactility/module.h | 38 ++++++++- TactilityKernel/source/module.cpp | 85 +++++++++++++++---- TactilityKernel/source/symbols.c | 3 + TactilityKernel/tests/source/module_test.cpp | 67 +++++++++++++++ 6 files changed, 180 insertions(+), 25 deletions(-) diff --git a/Modules/app-esp32-module/source/module.cpp b/Modules/app-esp32-module/source/module.cpp index 66fdfc5d4..20792ccff 100644 --- a/Modules/app-esp32-module/source/module.cpp +++ b/Modules/app-esp32-module/source/module.cpp @@ -47,7 +47,7 @@ void module_refs_end() { void module_refs_release(ModuleRefs* refs) { for (auto* module : refs->modules) { - module_put(module); + module_put_code(module); } refs->modules.clear(); } @@ -57,7 +57,9 @@ extern "C" { // Overrides elf_loader's default KConfig-based symbol resolver with one that looks up symbols // across every started kernel module's own symbol table instead. This module's own symbols come // first, as they replace libc functions for apps (see app_symbols.cpp). -// While an ELF is relocated, the modules it resolves symbols from are referenced, so they can't stop while it runs. +// While an ELF is relocated, the modules it resolves symbols from are referenced, so that their +// code stays mapped while it runs. It's a code reference, so it doesn't stop an app from stopping +// such a module on purpose (e.g. to take over the display) and starting it again afterwards. uintptr_t app_esp32_symbol_resolver(const char* symbolName) { uintptr_t symbol_address; if (module_resolve_symbol(&app_esp32_module, symbolName, &symbol_address)) { @@ -69,13 +71,13 @@ uintptr_t app_esp32_symbol_resolver(const char* symbolName) { } Module* owner = nullptr; - if (!module_resolve_symbol_global_get(symbolName, &symbol_address, &owner)) { + if (!module_resolve_symbol_global_get_code(symbolName, &symbol_address, &owner)) { return 0; } // One reference per module is enough for (auto* module : current_refs->modules) { if (module == owner) { - module_put(owner); + module_put_code(owner); return symbol_address; } } diff --git a/Modules/app-esp32-module/source/module_refs.h b/Modules/app-esp32-module/source/module_refs.h index 919f92d83..ed1008d90 100644 --- a/Modules/app-esp32-module/source/module_refs.h +++ b/Modules/app-esp32-module/source/module_refs.h @@ -5,7 +5,7 @@ #include -/** The kernel modules whose symbols an ELF resolved while it was relocated. Each one is held with module_get(). */ +/** The kernel modules whose symbols an ELF resolved while it was relocated. Each one is held with a code reference (module_get_code()). */ struct ModuleRefs { std::vector modules; }; diff --git a/TactilityKernel/include/tactility/module.h b/TactilityKernel/include/tactility/module.h index ab076ba38..831a87913 100644 --- a/TactilityKernel/include/tactility/module.h +++ b/TactilityKernel/include/tactility/module.h @@ -103,7 +103,7 @@ error_t module_add(struct Module* module); * @brief Remove a module from the system. * @param module module to remove * @retval ERROR_NONE if successful - * @retval ERROR_RESOURCE_BUSY the module is referenced (see module_get()). It stays added. + * @retval ERROR_RESOURCE_BUSY the module is referenced (see module_get() and module_get_code()). It stays added. */ error_t module_remove(struct Module* module); @@ -118,7 +118,8 @@ error_t module_start(struct Module* module); * @brief Stop the module. * @param module module * @retval ERROR_NONE if successful or if the module wasn't started - * @retval ERROR_RESOURCE_BUSY the module is referenced (see module_get()) or a driver of it is bound to a device. The module keeps running. + * @retval ERROR_RESOURCE_BUSY the module is referenced (see module_get(), or module_get_code() for a + * module that came from a package) or a driver of it is bound to a device. The module keeps running. * @return otherwise the result of the module's stop function */ error_t module_stop(struct Module* module); @@ -187,6 +188,26 @@ error_t module_get(struct Module* module); */ void module_put(struct Module* module); +/** + * @brief Take a reference on a module for code that was relocated against its symbols. + * @details Unlike module_get(), this reference doesn't prevent module_stop(): relocated code only + * needs the module's code and data to stay mapped, which only module_remove() would break. It does + * prevent module_remove() for as long as the reference is held. + * This is what an app's ELF gets for every module it resolves symbols from, so that the app can + * still stop a module it was linked against (e.g. to take exclusive control of a device) and start + * it again afterwards. Every successful call must be paired with one module_put_code(). + * @param module module to reference + * @retval ERROR_NONE the module is referenced + * @retval ERROR_INVALID_STATE the module isn't added, so its symbols can't be resolved + */ +error_t module_get_code(struct Module* module); + +/** + * @brief Release a reference taken with a successful module_get_code() or module_resolve_symbol_global_get_code(). + * @param module module that was referenced + */ +void module_put_code(struct Module* module); + /** * @brief Resolve a symbol from the module. * @details The module must be added with module_add() for symbol resolution to succeed. It doesn't have to be started. @@ -216,6 +237,19 @@ bool module_resolve_symbol_global(const char* symbol_name, uintptr_t* symbol_add */ bool module_resolve_symbol_global_get(const char* symbol_name, uintptr_t* symbol_address, struct Module** out_module); +/** + * @brief Same as module_resolve_symbol_global(), but also takes a code reference on the module + * that has the symbol (see module_get_code()). + * @details Use this when the resolved symbol ends up in code that outlives the lookup (e.g. an ELF + * relocation), where the module's code must stay mapped but where the module itself may still be + * stopped and started again. + * @param symbol_name name of the symbol to resolve + * @param symbol_address pointer to store the address of the resolved symbol + * @param out_module pointer to store the module that has the symbol. Call module_put_code() on it. + * @return true if the symbol was found. The module is referenced then. + */ +bool module_resolve_symbol_global_get_code(const char* symbol_name, uintptr_t* symbol_address, struct Module** out_module); + #ifdef __cplusplus } #endif diff --git a/TactilityKernel/source/module.cpp b/TactilityKernel/source/module.cpp index 7197226d2..2569e29f5 100644 --- a/TactilityKernel/source/module.cpp +++ b/TactilityKernel/source/module.cpp @@ -17,6 +17,9 @@ struct ModuleInternal { bool external = false; // All three are guarded by the ledger mutex int use_count = 0; + // References from relocated code (see module_get_code()): they keep the module mapped, but + // they don't keep it running + int code_count = 0; bool stopping = false; // In the ledger. Its symbols can be resolved from then on, also while it hasn't started yet. bool added = false; @@ -76,7 +79,7 @@ error_t module_add(Module* module) { error_t module_remove(Module* module) { mutex_lock(&ledger.mutex); // Checked under the same lock as the removal, so no reference can be taken in between - if (module->internal != nullptr && module->internal->use_count != 0) { + if (module->internal != nullptr && (module->internal->use_count != 0 || module->internal->code_count != 0)) { mutex_unlock(&ledger.mutex); return ERROR_RESOURCE_BUSY; } @@ -140,6 +143,18 @@ static void end_stopping(ModuleInternal* internal) { mutex_unlock(&ledger.mutex); } +/** + * A usage reference always blocks stopping. A code reference (from relocated code) doesn't: an app + * stops a firmware module it was linked against on purpose, to take over the device that module + * drives, and starts it again when it's done. + * Modules from packages are the exception: the user can stop those from Kernel settings while an + * app is running, so there the references from that app must keep it running. + * Caller must hold the ledger mutex. + */ +static bool is_stop_blocked(const ModuleInternal* internal) { + return internal->use_count != 0 || (internal->external && internal->code_count != 0); +} + error_t module_stop(Module* module) { LOG_I(TAG, "stop %s", module->name); @@ -148,7 +163,7 @@ error_t module_stop(Module* module) { // A module that hasn't started can be referenced too, through its symbols mutex_lock(&ledger.mutex); - const bool referenced = internal->use_count != 0; + const bool referenced = is_stop_blocked(internal); mutex_unlock(&ledger.mutex); if (referenced) { LOG_W(TAG, "%s can't stop: it is in use", module->name); @@ -159,7 +174,7 @@ error_t module_stop(Module* module) { // Checking and refusing new references is one atomic step, so a module_get() can't slip in afterwards mutex_lock(&ledger.mutex); - if (internal->use_count != 0) { + if (is_stop_blocked(internal)) { mutex_unlock(&ledger.mutex); LOG_W(TAG, "%s can't stop: it is in use", module->name); return ERROR_RESOURCE_BUSY; @@ -266,6 +281,27 @@ void module_put(Module* module) { mutex_unlock(&ledger.mutex); } +error_t module_get_code(Module* module) { + auto* internal = module->internal; + if (internal == nullptr) { return ERROR_INVALID_STATE; } + + mutex_lock(&ledger.mutex); + // Also allowed while the module isn't started: relocated code needs the symbols, not the state + const bool available = internal->added; + if (available) { + internal->code_count++; + } + mutex_unlock(&ledger.mutex); + return available ? ERROR_NONE : ERROR_INVALID_STATE; +} + +void module_put_code(Module* module) { + mutex_lock(&ledger.mutex); + check(module->internal != nullptr && module->internal->code_count > 0); + module->internal->code_count--; + mutex_unlock(&ledger.mutex); +} + bool module_resolve_symbol(Module* module, const char* symbol_name, uintptr_t* symbol_address) { if (module->internal == nullptr || !module->internal->added) return false; auto* symbol_ptr = module->symbols; @@ -280,28 +316,29 @@ bool module_resolve_symbol(Module* module, const char* symbol_name, uintptr_t* s return false; } -bool module_resolve_symbol_global(const char* symbol_name, uintptr_t* symbol_address) { - mutex_lock(&ledger.mutex); - for (auto* module : ledger.modules) { - if (module_resolve_symbol(module, symbol_name, symbol_address)) { - mutex_unlock(&ledger.mutex); - return true; - } - } - mutex_unlock(&ledger.mutex); - return false; -} +enum class RefKind { NONE, USE, CODE }; -bool module_resolve_symbol_global_get(const char* symbol_name, uintptr_t* symbol_address, Module** out_module) { +static bool resolve_symbol_global(const char* symbol_name, uintptr_t* symbol_address, Module** out_module, RefKind ref_kind) { mutex_lock(&ledger.mutex); for (auto* module : ledger.modules) { auto* internal = module->internal; - if (internal == nullptr || internal->stopping) + if (internal == nullptr) { continue; + } + // A module that's being stopped may still be resolvable, but no new reference may be taken + if (ref_kind != RefKind::NONE && internal->stopping) { + continue; + } if (module_resolve_symbol(module, symbol_name, symbol_address)) { - internal->use_count++; + if (ref_kind == RefKind::USE) { + internal->use_count++; + } else if (ref_kind == RefKind::CODE) { + internal->code_count++; + } mutex_unlock(&ledger.mutex); - *out_module = module; + if (out_module != nullptr) { + *out_module = module; + } return true; } } @@ -309,4 +346,16 @@ bool module_resolve_symbol_global_get(const char* symbol_name, uintptr_t* symbol return false; } +bool module_resolve_symbol_global(const char* symbol_name, uintptr_t* symbol_address) { + return resolve_symbol_global(symbol_name, symbol_address, nullptr, RefKind::NONE); +} + +bool module_resolve_symbol_global_get(const char* symbol_name, uintptr_t* symbol_address, Module** out_module) { + return resolve_symbol_global(symbol_name, symbol_address, out_module, RefKind::USE); +} + +bool module_resolve_symbol_global_get_code(const char* symbol_name, uintptr_t* symbol_address, Module** out_module) { + return resolve_symbol_global(symbol_name, symbol_address, out_module, RefKind::CODE); +} + } diff --git a/TactilityKernel/source/symbols.c b/TactilityKernel/source/symbols.c index 424f516fe..cf150072d 100644 --- a/TactilityKernel/source/symbols.c +++ b/TactilityKernel/source/symbols.c @@ -620,8 +620,11 @@ const struct ModuleSymbol KERNEL_SYMBOLS[] = { DEFINE_MODULE_SYMBOL(module_resolve_symbol), DEFINE_MODULE_SYMBOL(module_resolve_symbol_global), DEFINE_MODULE_SYMBOL(module_resolve_symbol_global_get), + DEFINE_MODULE_SYMBOL(module_resolve_symbol_global_get_code), DEFINE_MODULE_SYMBOL(module_get), DEFINE_MODULE_SYMBOL(module_put), + DEFINE_MODULE_SYMBOL(module_get_code), + DEFINE_MODULE_SYMBOL(module_put_code), // bundle DEFINE_MODULE_SYMBOL(bundle_alloc), DEFINE_MODULE_SYMBOL(bundle_clone), diff --git a/TactilityKernel/tests/source/module_test.cpp b/TactilityKernel/tests/source/module_test.cpp index 29168199b..00e2cc78b 100644 --- a/TactilityKernel/tests/source/module_test.cpp +++ b/TactilityKernel/tests/source/module_test.cpp @@ -229,6 +229,73 @@ TEST_CASE("module_stop is refused while the module is referenced") { CHECK_EQ(module_destruct(&module), ERROR_NONE); } +TEST_CASE("A code reference keeps a module mapped without preventing a stop") { + static const struct ModuleSymbol symbols[] = { + DEFINE_MODULE_SYMBOL(symbol_test_function), + MODULE_SYMBOL_TERMINATOR + }; + start_called = false; + stop_called = false; + test_start_result = ERROR_NONE; + test_stop_result = ERROR_NONE; + struct Module module = { + .name = "code_referenced_test", + .start = test_start, + .stop = test_stop, + .symbols = symbols, + .internal = nullptr + }; + + REQUIRE_EQ(module_construct_add_start(&module), ERROR_NONE); + + uintptr_t address = 0; + struct Module* owner = nullptr; + REQUIRE(module_resolve_symbol_global_get_code("symbol_test_function", &address, &owner)); + CHECK_EQ(owner, &module); + + // Relocated code only needs the symbols, so it doesn't keep the module running + CHECK_EQ(module_stop(&module), ERROR_NONE); + CHECK_EQ(stop_called, true); + + // It does keep the module from being removed or destructed + CHECK_EQ(module_remove(&module), ERROR_RESOURCE_BUSY); + CHECK_EQ(module_ensure_destructed(&module), ERROR_RESOURCE_BUSY); + + // And it can be started again while the code reference is held + CHECK_EQ(module_start(&module), ERROR_NONE); + CHECK_EQ(module_is_started(&module), true); + + module_put_code(owner); + CHECK_EQ(module_ensure_destructed(&module), ERROR_NONE); +} + +TEST_CASE("A code reference prevents stopping a module from a package") { + stop_called = false; + test_start_result = ERROR_NONE; + test_stop_result = ERROR_NONE; + struct Module module = { + .name = "external_code_referenced_test", + .start = test_start, + .stop = test_stop, + .symbols = nullptr, + .internal = nullptr + }; + + REQUIRE_EQ(module_construct_add_start(&module), ERROR_NONE); + REQUIRE_EQ(module_set_external(&module, true), ERROR_NONE); + REQUIRE_EQ(module_get_code(&module), ERROR_NONE); + + // The user can stop package modules from Kernel settings, so an app's use of one must protect it + CHECK_EQ(module_stop(&module), ERROR_RESOURCE_BUSY); + CHECK_EQ(stop_called, false); + + module_put_code(&module); + CHECK_EQ(module_stop(&module), ERROR_NONE); + CHECK_EQ(stop_called, true); + CHECK_EQ(module_remove(&module), ERROR_NONE); + CHECK_EQ(module_destruct(&module), ERROR_NONE); +} + TEST_CASE("module_remove is refused while the module is referenced") { static const struct ModuleSymbol symbols[] = { DEFINE_MODULE_SYMBOL(symbol_test_function), From 0b62703c747e18953062edd6aea7dca4bae80678 Mon Sep 17 00:00:00 2001 From: Shadowtrance Date: Sat, 24 Oct 2026 14:03:49 +1000 Subject: [PATCH 2/2] Update module.cpp --- TactilityKernel/source/module.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TactilityKernel/source/module.cpp b/TactilityKernel/source/module.cpp index 2569e29f5..36cd5a9d6 100644 --- a/TactilityKernel/source/module.cpp +++ b/TactilityKernel/source/module.cpp @@ -287,7 +287,7 @@ error_t module_get_code(Module* module) { mutex_lock(&ledger.mutex); // Also allowed while the module isn't started: relocated code needs the symbols, not the state - const bool available = internal->added; + const bool available = internal->added && !internal->stopping; if (available) { internal->code_count++; }