Fix memory leaks in swaybar and compositor#325
Conversation
jbms
force-pushed
the
scroll-fix-swaybar-and-sway-exit-leaks
branch
4 times, most recently
from
bdfad6e to
b5d7b58
Compare
|
For this PR, I would rather wait until sway decides what to do. My point of view about this is memory leaks should be fixed. To me, a memory leak is a run time error that prevents the application to recover allocated memory, and it is additive, creating an increase in memory consumption that could be avoided by properly freeing the memory involved. It is usually created by overwriting an address that was storing a reference to a heap allocated memory block, and then the reference gets lost, making it impossible to free that memory. However, unfreed, memory at exit is not a memory leak if the memory was in use before then. This memory will be recovered by the process scheduler when the application exits. So, in this case, if there are any real memory leaks, like maybe for example the config include problem you mention, or the Pango issue, I am all for fixing them, but not freeing the active containers before the application exits is not a problem as long as the application has been freeing the unused ones during its lifecycle. I know it is annoying if you are running Address Sanitizer, but it doesn't add any memory usage to the runtime, and you have also discovered freeing these structures creates certain problems, like having to synchronize with clients. Being strict, there could be cases in the future where exiting the compositor wouldn't necessarily mean the clients should die, maybe they could be attached to a different compositor or another instance of the same one, and I don't know how that would play with this effort to have a "clean" shutdown. |
jbms
force-pushed
the
scroll-fix-swaybar-and-sway-exit-leaks
branch
2 times, most recently
from
5513b83 to
9ec566c
Compare
jbms
changed the title
|
I have now revised this to be much more minimal --- it fixes a couple real runtime leaks, triggers lsan to run earlier in the shutdown when all of the nodes are still reachable and therefore won't count as leaks. |
jbms
force-pushed
the
scroll-fix-swaybar-and-sway-exit-leaks
branch
from
9ec566c to
9a42f29
Compare
This change fixes several runtime memory leaks in the compositor and swaybar,
and introduces a cleaner strategy for handling exit-time leaks under
LeakSanitizer (LSan) without requiring verbose shutdown-only cleanup code.
Runtime Leak Fixes:
- Compositor:
- Free temporary file list in `cmd_include` when processing `include` commands.
- Free `font_description` in `free_config` during config reloads.
- End seat operations in `handle_seat_destroy` to free seatop-specific state.
- Swaybar:
- Free `status->buffer` in `status_line_free` to avoid leaking the status line on updates.
Exit-Time Leak Strategy:
Instead of adding extensive code to traverse and free all global structures
(like container trees) at shutdown just to satisfy LSan, we run LSan checks
early before these structures are orphaned:
- Compositor:
- Manually invoke `__lsan_do_leak_check()` in `main.c` after `server_fini()`
has completed but before `root_destroy` or `free_config` are called.
- Prior to the leak check, clean up `pango`, `cairo`, and `fontconfig`
global static caches to avoid false positives.
- Since the global pointers `root` and `config` are still intact when LSan
runs, LSan correctly treats these structures as reachable rather than leaked,
allowing us to still run `root_destroy` and `free_config` afterwards
unconditionally.
- Added direct dependency on `fontconfig` in Meson build files to support direct calls to `FcFini()`.
- Swaybar:
- Wrap library cleanups and manual LSan invocation in `#ifdef __SANITIZE_ADDRESS__`
after `bar_teardown` in `main.c`.
- Add targeted LSan suppressions in `tests/lsan.supp` for exit-only DBus slot
allocations in the tray (`create_watcher`, `init_host`, etc.) that are not
worth cleaning up at shutdown.
jbms
force-pushed
the
scroll-fix-swaybar-and-sway-exit-leaks
branch
from
9a42f29 to
948691a
Compare
|
But now, fonts and pango remainings only get freed if built with the sanitizer, so that code doesn't really do anything for regular users, it is just so the output of libasan is less verbose. We introduce a new dependency ( I would like the PR to be about real memory leaks that can be solved to improve the user experience, if there are any, like the config stuff. The rest is for developers, and I don't think it should be here. I rarely commit things that I use to ease my development, otherwise it could be very confusing and too verbose for people reading the code. We all use different tools and have different routines. |
2 participants
See swaywm/sway#9195