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 in X11 display backend #1483

Merged
merged 5 commits into from
Mar 31, 2023
Merged

Conversation

bi4k8
Copy link
Collaborator

@bi4k8 bi4k8 commented Mar 31, 2023

These are not ongoing leaks but just missing cleanup that offends valgrind. One of these destructors was previously associated with crashes, but I believe the cases where it would crash no longer apply.

Checklist

  • I have described the changes
  • [n/a] I have linked to any relevant GitHub issues, if applicable
  • [n/a] Documentation in doc/ has been updated
  • All new code is licensed under GPLv3

@netlify
Copy link

netlify bot commented Mar 31, 2023

Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit a80384e
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/64270ed65c01240008c0911e

@github-actions github-actions bot added the sources PR modifies project sources label Mar 31, 2023
FcFini() was never being called, which meant FontConfig did not clean up its internal storage.

furthermore, restore a commented-out call to XftFontClose()

9acf0bb commented this call out, but it does not seem to cause any problems now on reload now
@@ -603,6 +603,9 @@ void display_output_x11::cleanup() {
XDestroyRegion(x11_stuff.region);
x11_stuff.region = nullptr;
}
#ifdef BUILD_XFT
FcFini();
Copy link
Owner

Choose a reason for hiding this comment

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

This function comes from fontconfig, rather than Xft, which is fine but we currently don't do the necessary platform checks for fontconfig (which I assume is why the build fails on macos).

brndnmtthws and others added 2 commits March 31, 2023 12:41
Bumps [actions/stale](https://github.com/actions/stale) from 7 to 8.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](actions/stale@v7...v8)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@github-actions github-actions bot added gh-actions Issue or PR that suggest changing GitHub actions web Issue or PR related to documentation website labels Mar 31, 2023
@github-actions github-actions bot removed gh-actions Issue or PR that suggest changing GitHub actions web Issue or PR related to documentation website labels Mar 31, 2023
@brndnmtthws brndnmtthws merged commit dab87c5 into brndnmtthws:main Mar 31, 2023
51 checks passed
simotek pushed a commit to simotek/conky that referenced this pull request Apr 5, 2023
* display-x11: fix memory leaks

FcFini() was never being called, which meant FontConfig did not clean up its internal storage.

furthermore, restore a commented-out call to XftFontClose()

9acf0bb commented this call out, but it does not seem to cause any problems now on reload now

* feat: Move docs to website, rearrange bits (brndnmtthws#1475)

* build(deps): bump actions/stale from 7 to 8 (brndnmtthws#1472)

Bumps [actions/stale](https://github.com/actions/stale) from 7 to 8.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](actions/stale@v7...v8)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add fontconfig platform checks

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: bi4k8 <bi4k8@github>
Co-authored-by: Brenden Matthews <brenden@brndn.io>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Brenden Matthews <github@brenden.brndn.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sources PR modifies project sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants