Skip to content

Fix globalcontext_destroy leaks#2356

Open
petermm wants to merge 1 commit into
atomvm:mainfrom
petermm:fix-globalcontext-leaks
Open

Fix globalcontext_destroy leaks#2356
petermm wants to merge 1 commit into
atomvm:mainfrom
petermm:fix-globalcontext-leaks

Conversation

@petermm

@petermm petermm commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Surfaced when doing test_main.c work for zephyr CI.

Knew they were there, should help the esp32 test_main.c harness, especially on the low memory boards and wifi tests.

Mostly (only?) relevant in those scenarios.

note that there is still a leak for: "atom_table_destroy still does not free atom strings allocated with AtomTableCopyAtom."
llm tells me:

I’d estimate 30-60 minutes for a clean fix plus focused tests. The likely shape:
Add an ownership bit to the internal atom-table node, e.g. bool owns_key.
Set owns_key = true only when AtomTableCopyAtom allocates buf.
Set owns_key = false for BEAM/default atoms that point into other owned storage.
In atom_table_destroy, walk all node groups and free((void *) node->key) only for owned keys.
Add/update test-structs coverage for copied atoms.
Let CI/valgrind prove the leak is gone.
Risk level: low to medium. The main thing is not accidentally freeing atom strings that belong to module binaries/default atom storage. But since HNode is private to atom_table.c, the API impact should be tiny.

but that is punted for a follow up.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@pguyot pguyot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably can squash this in a single commit

Fix modules table cleanup on globalcontext_new errors

Use valueshashtable_destroy when unwinding after modules_table has been allocated so its buckets and SMP lock are released consistently with normal GlobalContext teardown.

Signed-off-by: Peter M <petermm@gmail.com>
@petermm petermm force-pushed the fix-globalcontext-leaks branch from 131dbf5 to f283ce2 Compare July 4, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants