Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leaks detected by running testlibrary with AddressSanitizer #5690

Merged
merged 9 commits into from
Feb 15, 2024

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented Feb 15, 2024

This PR just fixes leaks detected in the production code. A follow-up PR will get testlibrary passing.


  • main: Use g_autoptr for the GOptionContext

    No functional change, but it will make it easier to avoid leaking it.

  • main: Return from flatpak_run() instead of calling exit()

    This allows g_autoptr destructors to run, avoiding memory leaks being
    reported by AddressSanitizer; they would be harmless, since we're about
    to exit anyway, but AddressSanitizer can't tell the difference between
    an O(n) problem and an O(1) harmless "leak".

  • dir: Don't store a pointer in a gsize

    This is, strictly speaking, not allowed. On uncommon architectures like
    CHERI, a pointer can be larger than a gsize.

    This might also help to avoid AddressSanitizer losing track of
    reachability, so that it won't think the array and its contents have
    been leaked.

  • populate_commit_data_cache: Don't leak child value

    g_variant_get_child_value() returns a non-floating reference, so
    g_variant_builder_add() will not sink it.

  • populate_commit_data_cache: Don't leak a floating GVariant

    var_variant_dup_to_gvariant() returns a floating GVariant, and
    g_variant_get_child_value() won't sink it, so we need to free it.

  • session-helper: Don't leak the GOptionContext

  • flatpak_remote_commit_filter: Don't leak config GKeyFile

  • flatpak-bwrap: Don't leak runtime_dir_members

  • system-helper: Don't leak the GCancellable for each OngoingPull

No functional change, but it will make it easier to avoid leaking it.

Signed-off-by: Simon McVittie <[email protected]>
This allows g_autoptr destructors to run, avoiding memory leaks being
reported by AddressSanitizer; they would be harmless, since we're about
to exit anyway, but AddressSanitizer can't tell the difference between
an O(n) problem and an O(1) harmless "leak".

Signed-off-by: Simon McVittie <[email protected]>
This is, strictly speaking, not allowed. On uncommon architectures like
CHERI, a pointer can be larger than a gsize.

This might also help to avoid AddressSanitizer losing track of
reachability, so that it won't think the array and its contents have
been leaked.

Signed-off-by: Simon McVittie <[email protected]>
g_variant_get_child_value() returns a non-floating reference, so
g_variant_builder_add() will not sink it.

Signed-off-by: Simon McVittie <[email protected]>
var_variant_dup_to_gvariant() returns a floating GVariant, and
g_variant_get_child_value() won't sink it, so we need to free it.

Signed-off-by: Simon McVittie <[email protected]>
Copy link
Member

@TingPing TingPing left a comment

Choose a reason for hiding this comment

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

Looks good

@smcv smcv merged commit a607246 into flatpak:main Feb 15, 2024
9 checks passed
@smcv smcv mentioned this pull request Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants