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 #5683

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Fix memory leaks #5683

merged 1 commit into from
Feb 13, 2024

Conversation

hfiguiere
Copy link
Collaborator

Build with address sanitizer and run the tests

@hfiguiere
Copy link
Collaborator Author

I guess that why we skipp the valgrind test?

CI is green now. There are some leaks left though, not sure how to address them.

Build with address sanitizer and run the tests

Signed-off-by: Hubert Figuière <[email protected]>
@TingPing TingPing merged commit 6e3cc82 into flatpak:main Feb 13, 2024
9 checks passed
@TingPing
Copy link
Member

I guess that why we skipp the valgrind test?

I think it was too slow, like beyond the limits of Github CI.

@hfiguiere hfiguiere deleted the leaks branch February 13, 2024 15:39
@hfiguiere
Copy link
Collaborator Author

yeah I saw after that we have asan without leaksanitizer.

@smcv
Copy link
Collaborator

smcv commented Feb 15, 2024

There are some leaks left though, not sure how to address them

#5690 fixed some more (in production code), and #5691 will fix more (in tests); reviews welcome. I think the only way to eliminate memory leaks is to work our way through like that, one test or a few tests at a time, until they all pass, and then add AddressSanitizer to the Github CI so that it won't regress.

@hfiguiere
Copy link
Collaborator Author

Sadly some of the remaining leaks in test haven't been fixed with your PR.

I will file issue for them.

@smcv
Copy link
Collaborator

smcv commented Feb 15, 2024

Sadly some of the remaining leaks in test haven't been fixed with your PR

Yeah, it's a slow process - I only got as far as making testlibrary pass (with GLib 2.79.2, other versions might have different leaks).

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.

None yet

3 participants