dominic/Tactility
1
0
Fork 0
mirror of https://github.com/ByteWelder/Tactility.git synced 2026-10-17 02:45:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Cleanup

Browse Source
This commit is contained in:
Ken Van Hoeylandt 2026-09-04 00:37:30 +02:00
parent e08300d9aa
commit dd40580165
5 changed files with 73 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Modules/app-module/include/app/execute.h
View File

@ -83,7 +83,7 @@ error_t app_execute_for_result_with_streams(
struct AppStackConfig stack,
int argc,
const char* const argv[],
const struct AppStreamBinding* binding,
const struct AppStreamBinding* bindings,
size_t binding_count,
AppInstanceId parent_instance_id,
AppInstanceId* out_app_instance_id

37
Modules/app-module/private/app/private/arguments.h Normal file
View File

@ -0,0 +1,37 @@
// SPDX-License-Identifier: Apache-2.0
#pragma once
#include <cstring>
/**
* Deep-copies @a argv (@a argc <= 0 => NULL, matching "no parameters"). Caller passes the result
* to app_scheduler_start(), which takes ownership regardless of outcome.
*/
inline char** app_arguments_copy(int argc, const char* const argv[]) {
if (argc <= 0) {
return nullptr;
}
auto* copy = new char*[argc + 1];
for (int i = 0; i < argc; i++) {
size_t length = strlen(argv[i]);
copy[i] = new char[length + 1];
memcpy(copy[i], argv[i], length + 1);
}
copy[argc] = nullptr;
return copy;
}
/**
* Frees a deep-copied argv previously built by app_arguments_copy(): each individually
* heap-allocated string, then the array itself. Safe to call with count == 0 / values == nullptr
* (no-op).
*/
inline void app_arguments_free(int count, char** values) {
if (values == nullptr) {
return;
}
for (int i = 0; i < count; i++) {
delete[] values[i];
}
delete[] values;
}

14
Modules/app-module/private/app/private/ledger.h
View File

@ -71,17 +71,3 @@ inline AppLedger& app_ledger() {
static AppLedger ledger;
return ledger;
}
/**
* Frees a deep-copied argv previously built by app_start()/app_start_for_result():
* each individually heap-allocated string, then the array itself. Safe to call with count == 0 values == nullptr (no-op).
*/
inline void app_ledger_free_arguments(int count, char** values) {
if (values == nullptr) {
return;
}
for (int i = 0; i < count; i++) {
delete[] values[i];
}
delete[] values;
}

83
Modules/app-module/source/manager.cpp
View File

@ -1,6 +1,7 @@
// SPDX-License-Identifier: Apache-2.0
#include <app/manager.h>
#include <app/metadata.h>
#include <app/private/arguments.h>
#include <app/private/fd_table.h>
#include <app/private/fs.h>
#include <app/private/ledger.h>
@ -71,33 +72,11 @@ void app_manager_for_each_manifest(AppManifestVisitorFn visitor, void* context)
mutex_unlock(&ledger.mutex);
}
namespace {
// Deep-copies argv (argc <= 0 => NULL, matching "no parameters"). Caller passes the result to
// app_scheduler_start(), which takes ownership regardless of outcome.
char** copy_arguments(int argc, const char* const argv[]) {
if (argc <= 0) {
return nullptr;
}
auto* copy = new char*[argc + 1];
for (int i = 0; i < argc; i++) {
size_t length = strlen(argv[i]);
copy[i] = new char[length + 1];
memcpy(copy[i], argv[i], length + 1);
}
copy[argc] = nullptr;
return copy;
}
} // namespace
error_t app_manager_start_internal(const AppManifest* manifest, AppLocation location, AppStackConfig stack, AppInstanceId parent_instance_id, int argc, const char* const argv_in[], const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) {
// Owns its own deep copy from here on: app_scheduler_start() takes ownership of it regardless
// of outcome, and the spawned task frees it once its run() returns.
char** argv = copy_arguments(argc, argv_in);
char** argv = app_arguments_copy(argc, argv_in);
if (binding_count != 0 && bindings == nullptr) {
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_INVALID_ARGUMENT;
}
@ -108,9 +87,8 @@ error_t app_manager_start_internal(const AppManifest* manifest, AppLocation loca
AppInstanceRecord record { .id = target_id, .manifest = manifest, .state = APP_INSTANCE_STATE_STARTING, .task = nullptr };
record.parent_id = parent_instance_id;
ledger.instances[target_id] = record;
// Constructed on the map-resident copy, not the local `record` about to go out of scope.
// AppFdTable::fds[] entries point into AppFdTable::slots[] by address (see fd_table.h), so
// constructing before the copy above would leave them pointing at stack storage.
// Construct on the map-resident copy, not `record`: fds[] point into slots[] by address
// (fd_table.h), so constructing on the stack-local record would leave them dangling.
app_fd_table_construct(&ledger.instances[target_id].fd_table);
mutex_unlock(&ledger.mutex);
@ -120,9 +98,8 @@ error_t app_manager_start_internal(const AppManifest* manifest, AppLocation loca
error_t bind_result = app_stream_subscribe(bindings[i].stream, bindings[i].buffer, bindings[i].buffer_capacity, bindings[i].event_group, target_id, bindings[i].producer_fd);
if (bind_result != ERROR_NONE) {
LOG_E(TAG, "[instance %d] Failed to bind stream at fd %d: %s", target_id, bindings[i].producer_fd, error_to_string(bind_result));
// Undo bindings[0..i): app_fd_table_teardown() below only closes each stream. It
// doesn't release the event bits app_stream_subscribe() claimed or destruct
// stream->internal.mutex; only app_stream_unsubscribe() does that.
// Undo bindings[0..i): teardown() below only closes the fd, not the event bits
// or mutex app_stream_subscribe() claimed; only app_stream_unsubscribe() does.
for (size_t j = 0; j < i; j++) {
app_stream_unsubscribe(bindings[j].stream);
}
@ -130,15 +107,13 @@ error_t app_manager_start_internal(const AppManifest* manifest, AppLocation loca
app_fd_table_teardown(&ledger.instances[target_id].fd_table);
ledger.instances.erase(target_id);
mutex_unlock(&ledger.mutex);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return bind_result;
}
}
error_t error = app_scheduler_start(target_id, location, stack, argc, argv);
if (error != ERROR_NONE) {
// Every binding succeeded before app_scheduler_start() failed. Unsubscribe all of them,
// same reasoning as the bind-failure path above.
for (size_t j = 0; j < binding_count; j++) {
app_stream_unsubscribe(bindings[j].stream);
}
@ -172,8 +147,7 @@ error_t app_manager_get_topmost_instance_id(AppInstanceId* out_app_instance_id)
mutex_lock(&ledger.mutex);
AppInstanceId topmost_id = 0;
for (auto& [instance_id, record] : ledger.instances) {
// Instance ids are handed out in increasing order (AppLedger::next_instance_id), so
// the highest Active id is also the most recently started one.
// Ids increase monotonically, so the highest Active id is the most recent.
if (record.state == APP_INSTANCE_STATE_ACTIVE && instance_id > topmost_id) {
topmost_id = instance_id;
}
@ -224,10 +198,8 @@ error_t app_manager_get_topmost_app_id(char* buffer, size_t buffer_size) {
namespace {
// Owns the AppManifest (and its id/name/path strings) that app_manager_add() only keeps a
// non-owning pointer to (see app_manager_add()'s contract), for manifests registered by
// app_manager_install_path_scan() specifically - separate from app_install.cpp's own registry,
// since scanning only ever adds/removes manifest registrations and never touches files on disk
// or running instances (unlike app_install()/app_uninstall()).
// non-owning pointer to. Separate from app_install.cpp's registry: scanning only
// adds/removes registrations, never touches disk or running instances.
struct ScannedAppManifest {
std::string id;
std::string name;
@ -274,15 +246,15 @@ void app_manager_install_path_scan(void) {
app_fs_list_direct_subdirectories(root, found_app_dirs);
}
// Snapshot of what's already registered, taken once so the rest of this scan can run without holding registry.mutex
// Snapshot once so the rest of the scan doesn't hold registry.mutex.
mutex_lock(&registry.mutex);
std::unordered_map<std::string, std::string> known_paths; // id -> path
std::unordered_map<std::string, std::string> known_paths;
for (const auto& [id, record] : registry.scanned) {
known_paths.emplace(id, record->path);
}
mutex_unlock(&registry.mutex);
// Stat each manifest and parse it entirely without registry.mutex held (due to filesystem IO being slow)
// Parses without registry.mutex held; filesystem IO is slow.
std::vector<std::unique_ptr<ScannedAppManifest>> new_records;
for (const auto& app_dir : found_app_dirs) {
auto manifest_path = app_dir + "/manifest.properties";
@ -297,7 +269,7 @@ void app_manager_install_path_scan(void) {
}
if (known_paths.contains(metadata.app_id)) {
continue; // already registered by an earlier scan
continue;
}
auto record = std::make_unique<ScannedAppManifest>();
@ -315,7 +287,6 @@ void app_manager_install_path_scan(void) {
new_records.push_back(std::move(record));
}
// Anything a previous scan registered whose directory has since disappeared gets unregistered below.
std::vector<std::string> missing_ids;
for (const auto& [id, path] : known_paths) {
if (!app_fs_is_directory(path)) {
@ -323,11 +294,9 @@ void app_manager_install_path_scan(void) {
}
}
// app_manager_add()/app_manager_remove() take app-module's own ledger mutex internally -
// calling them while holding registry.mutex would establish a registry.mutex -> ledger-
// mutex lock order that any future opposite-order path would deadlock against, so these
// also run with registry.mutex released. registry.mutex is taken only afterward, briefly,
// to publish the results (plain in-memory map updates, no I/O or other locks involved).
// app_manager_add()/remove() take the ledger mutex internally, so calling them under
// registry.mutex would fix a lock order an opposite-order caller could deadlock against.
// registry.mutex is retaken afterward only to publish the in-memory results.
for (const auto& id : missing_ids) {
app_manager_remove(id.c_str());
}
@ -364,10 +333,8 @@ error_t app_manager_install_path_uninstall(const char* app_id) {
auto path = iterator->second->path;
mutex_unlock(&registry.mutex);
// Stop every running instance that retains this manifest pointer, mirroring
// stop_all_instances_of() in app_install.cpp. Collect under ledger.mutex,
// then call app_manager_stop() outside it (that call bound-joins the
// instance's thread, which itself takes ledger.mutex in its thread_main).
// Mirrors stop_all_instances_of() in app_install.cpp. Collect under ledger.mutex, stop
// outside it: app_manager_stop() bound-joins the thread, which itself takes ledger.mutex.
std::vector<uint32_t> instance_ids;
auto& ledger = app_ledger();
mutex_lock(&ledger.mutex);
@ -382,14 +349,12 @@ error_t app_manager_install_path_uninstall(const char* app_id) {
app_manager_stop(id);
}
// app_manager_remove takes ledger.mutex internally - call outside both
// registry.mutex and ledger.mutex to match the lock ordering in
// app_manager_install_path_scan().
// app_manager_remove() takes ledger.mutex; call outside registry.mutex too, matching
// the lock order in app_manager_install_path_scan().
app_manager_remove(app_id);
// Every instance has stopped and the manifest is unregistered — safe to
// delete the on-disk directory. Delete before erasing the scan record so
// that a failed deletion leaves the entry discoverable for a retry.
// Delete before erasing the scan record, so a failed deletion still leaves the
// entry discoverable for a retry.
if (!app_fs_delete_recursively(path)) {
return ERROR_RESOURCE;
}

21
Modules/app-module/source/scheduler.cpp
View File

@ -1,6 +1,7 @@
// SPDX-License-Identifier: Apache-2.0
#include <app/instance.h>
#include <app/loader.h>
#include <app/private/arguments.h>
#include <app/private/event.h>
#include <app/private/fd_table.h>
#include <app/private/ledger.h>
@ -223,7 +224,7 @@ void app_task_main(void* context) {
// response to APP_EVENT_CLOSE.
set_state(ctx->app_instance_id, APP_INSTANCE_STATE_STOPPED);
app_ledger_free_arguments(ctx->argc, ctx->argv);
app_arguments_free(ctx->argc, ctx->argv);
AppInstanceId app_instance_id = ctx->app_instance_id;
AppCompletionSignal* completion = ctx->completion;
@ -274,7 +275,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
const AppLoaderApi* loader = find_loader_api(location.type);
if (loader == nullptr) {
LOG_E(TAG, "[instance %lu] No app loader is registered (service '%s' not found)", app_instance_id, loader_service_id_for(location.type));
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_NOT_FOUND;
}
@ -282,7 +283,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
error_t load_result = loader->load(location, &runtime);
if (load_result != ERROR_NONE) {
LOG_E(TAG, "[instance %lu] Failed to load app: %s", app_instance_id, error_to_string(load_result));
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return load_result;
}
@ -290,7 +291,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
if (completion == nullptr) {
LOG_E(TAG, "[instance %lu] Failed to allocate app", app_instance_id);
loader->unload(runtime);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_OUT_OF_MEMORY;
}
completion->semaphore = xSemaphoreCreateBinary();
@ -298,7 +299,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
LOG_E(TAG, "[instance %lu] Failed to allocate app", app_instance_id);
delete completion;
loader->unload(runtime);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_OUT_OF_MEMORY;
}
@ -309,7 +310,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
vSemaphoreDelete(completion->semaphore);
delete completion;
loader->unload(runtime);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_INVALID_ARGUMENT;
}
@ -335,7 +336,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
vSemaphoreDelete(completion->semaphore);
delete completion;
loader->unload(runtime);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_OUT_OF_MEMORY;
}
@ -346,7 +347,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
vSemaphoreDelete(completion->semaphore);
delete completion;
loader->unload(runtime);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_OUT_OF_MEMORY;
}
#else
@ -374,7 +375,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
vSemaphoreDelete(completion->semaphore);
delete completion;
loader->unload(runtime);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_OUT_OF_MEMORY;
}
@ -401,7 +402,7 @@ error_t app_scheduler_start(AppInstanceId app_instance_id, AppLocation location,
vSemaphoreDelete(completion->semaphore);
delete completion;
loader->unload(runtime);
app_ledger_free_arguments(argc, argv);
app_arguments_free(argc, argv);
return ERROR_OUT_OF_MEMORY;
}
vTaskSuspend(task_handle);