From 2f3c830b010412ff030e923b0c34455074bb598d Mon Sep 17 00:00:00 2001 From: Paul Guyot Date: Thu, 14 Nov 2024 22:29:27 +0100 Subject: [PATCH] Allow to run beams on CLI on generic_unix Also fix a memory leak where modules were not properly destroyed when the global context is destroyed Signed-off-by: Paul Guyot --- CHANGELOG.md | 5 +++ src/libAtomVM/globalcontext.c | 8 +++++ src/libAtomVM/globalcontext.h | 2 +- src/platforms/emscripten/src/main.c | 1 - src/platforms/generic_unix/main.c | 53 +++++++++++++++-------------- tests/test.c | 1 - 6 files changed, 42 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83a75e5f4..02b588dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.6.6] - Unreleased +### Added + +- Added the ability to run beams from the CLI for Generic Unix platform (it was already possible with nodejs and emscripten). + ### Fixed - Fixed specifications of nifs from `esp_adc` module @@ -14,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 might lead to a crash in certain situations. - Fixed several bugs in `http_server` (#1366) - Fixed generic\_unix `socket_driver` to return `{gen_tcp, closed}` when socket is closed on Linux instead of `{gen_tcp, {recv, 104}}` +- Fixed a memory leak where modules were not properly destroyed when the global context is destroyd ## [0.6.5] - 2024-10-15 diff --git a/src/libAtomVM/globalcontext.c b/src/libAtomVM/globalcontext.c index 54a8d291e..7cdfc020c 100644 --- a/src/libAtomVM/globalcontext.c +++ b/src/libAtomVM/globalcontext.c @@ -194,6 +194,11 @@ COLD_FUNC void globalcontext_destroy(GlobalContext *glb) struct ListHead *item; struct ListHead *tmp; + int module_index = glb->loaded_modules_count; + for (int i = 0; i < module_index; i++) { + module_destroy(glb->modules_by_index[i]); + } + struct ListHead *open_avm_packs = synclist_nolock(&glb->avmpack_data); MUTABLE_LIST_FOR_EACH (item, tmp, open_avm_packs) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); @@ -622,6 +627,9 @@ Module *globalcontext_get_module(GlobalContext *global, AtomString module_name_a free(module_name); if (UNLIKELY(!loaded_module || (globalcontext_insert_module(global, loaded_module) < 0))) { + if (loaded_module) { + module_destroy(loaded_module); + } return NULL; } diff --git a/src/libAtomVM/globalcontext.h b/src/libAtomVM/globalcontext.h index 71f88b633..617162ab6 100644 --- a/src/libAtomVM/globalcontext.h +++ b/src/libAtomVM/globalcontext.h @@ -435,7 +435,7 @@ term globalcontext_existing_term_from_atom_string(GlobalContext *glb, AtomString /** * @brief Inserts a module to the modules table. * - * @details Inserts an already loaded module to the modules table and assigns and index to it so it can be retrieved later by name or index. + * @details Inserts an already loaded module to the modules table and assigns and index to it so it can be retrieved later by name or index. The module is then owned by the global context that will destroy it when globalcontext_destroy is invoked. * @param global the global context. * @param module the module that will be added to the modules table. * @returns the module index if successful, otherwise -1. diff --git a/src/platforms/emscripten/src/main.c b/src/platforms/emscripten/src/main.c index 3c168960e..fd3f00eef 100644 --- a/src/platforms/emscripten/src/main.c +++ b/src/platforms/emscripten/src/main.c @@ -182,7 +182,6 @@ int main(int argc, char **argv) global = NULL; if (main_module) { - module_destroy(main_module); main_module = NULL; } diff --git a/src/platforms/generic_unix/main.c b/src/platforms/generic_unix/main.c index de22155f2..a5b5fbacb 100644 --- a/src/platforms/generic_unix/main.c +++ b/src/platforms/generic_unix/main.c @@ -46,7 +46,7 @@ void print_help(const char *program_name) "\n" "Syntax:\n" "\n" - " %s [-h] [-v] +\n" + " %s [-h] [-v] +\n" "\n" "Options:\n" "\n" @@ -88,9 +88,7 @@ int main(int argc, char **argv) GlobalContext *glb = globalcontext_new(); - const void *startup_beam = NULL; - uint32_t startup_beam_size; - const char *startup_module_name; + Module *startup_module = NULL; for (int i = 1; i < argc; ++i) { const char *ext = strrchr(argv[i], '.'); @@ -102,50 +100,56 @@ int main(int argc, char **argv) } synclist_append(&glb->avmpack_data, &avmpack_data->avmpack_head); - if (IS_NULL_PTR(startup_beam)) { + if (IS_NULL_PTR(startup_module)) { + const void *startup_beam = NULL; + const char *startup_module_name; + uint32_t startup_beam_size; avmpack_find_section_by_flag(avmpack_data->data, 1, &startup_beam, &startup_beam_size, &startup_module_name); if (startup_beam) { avmpack_data->in_use = true; + startup_module = module_new_from_iff_binary(glb, startup_beam, startup_beam_size); + if (IS_NULL_PTR(startup_module)) { + fprintf(stderr, "Cannot load startup module: %s\n", startup_module_name); + return EXIT_FAILURE; + } + globalcontext_insert_module(glb, startup_module); + startup_module->module_platform_data = NULL; } } - } else if (i == 1 && ext && (strcmp(ext, ".beam") == 0)) { + } else if (ext && (strcmp(ext, ".beam") == 0)) { MappedFile *mapped_file = mapped_file_open_beam(argv[i]); if (!iff_is_valid_beam(mapped_file->mapped)) { - fprintf(stderr, "%s has invalid AVM Pack format.\n", argv[i]); + fprintf(stderr, "%s has invalid beam format.\n", argv[i]); return EXIT_FAILURE; } - startup_module_name = basename(argv[1]); - startup_beam = mapped_file->mapped; - startup_beam_size = mapped_file->size; - - } else if (i == 1) { - fprintf(stderr, "%s is not an AVM or a BEAM file.\n", argv[i]); - return EXIT_FAILURE; + Module *mod = module_new_from_iff_binary(glb, mapped_file->mapped, mapped_file->size); + if (IS_NULL_PTR(mod)) { + fprintf(stderr, "Cannot load module: %s\n", argv[i]); + return EXIT_FAILURE; + } + globalcontext_insert_module(glb, mod); + mod->module_platform_data = NULL; + if (IS_NULL_PTR(startup_module) && module_search_exported_function(mod, ATOM_STR("\5", "start"), 0, glb) != 0) { + startup_module = mod; + } } else { - fprintf(stderr, "%s is not an AVM file.\n", argv[i]); + fprintf(stderr, "%s is not an AVM or a BEAM file.\n", argv[i]); return EXIT_FAILURE; } } - if (IS_NULL_PTR(startup_beam)) { + if (IS_NULL_PTR(startup_module)) { fprintf(stderr, "Unable to locate entrypoint.\n"); return EXIT_FAILURE; } - Module *mod = module_new_from_iff_binary(glb, startup_beam, startup_beam_size); - if (IS_NULL_PTR(mod)) { - fprintf(stderr, "Cannot load startup module: %s\n", startup_module_name); - return EXIT_FAILURE; - } - globalcontext_insert_module(glb, mod); - mod->module_platform_data = NULL; Context *ctx = context_new(glb); ctx->leader = 1; - context_execute_loop(ctx, mod, "start", 0); + context_execute_loop(ctx, startup_module, "start", 0); term ret_value = ctx->x[0]; fprintf(stderr, "Return value: "); @@ -161,7 +165,6 @@ int main(int argc, char **argv) context_destroy(ctx); globalcontext_destroy(glb); - module_destroy(mod); return status; } diff --git a/tests/test.c b/tests/test.c index 9b9f81035..0722f8410 100644 --- a/tests/test.c +++ b/tests/test.c @@ -608,7 +608,6 @@ static int test_atom(struct Test *test) context_destroy(ctx); globalcontext_destroy(glb); - module_destroy(mod); mapped_file_close(beam_file); return result; }