Fix: Memory leaks in core components#8716
Conversation
- Fix dlgTriggerEditor StringTextAutoCompleteProvider leak (96KB per session) * Add proper destructor with removeProvider() cleanup * Add member variable to track auto-complete provider instance * Include proper edbee textautocompleteprovider header - Fix Host QKeySequence leak (272 bytes per profile load) * Add qDeleteAll(profileShortcuts) cleanup in destructor - Fix Discord event handlers memory leak (48 bytes per shutdown) * Add explicit mpHandlers cleanup in destructor
- Convert per-editor auto-complete provider to shared static instance - Add reference counting to track editor instances - Only initialize provider once and clean up when last editor closes - Prevents use-after-free and ownership conflicts with multiple editors - Fixes issue where closing one editor would clobber another's provider - Remove unnecessary explicit disconnect calls (Qt auto-disconnects)
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
|
Thanks for tackling these memory leaks! A couple of suggestions: discord.cpp: The dlgTriggerEditor: There's a simpler approach that avoids manual reference counting entirely. The edbee library has // In constructor - no destructor needed
static bool sAutoCompleteInitialized = false;
if (!sAutoCompleteInitialized) {
auto* provider = new edbee::StringTextAutoCompleteProvider();
// ... populate provider ...
// Transfer ownership to Edbee - deleted automatically at app shutdown
edbee::Edbee::instance()->autoCompleteProviderList()->giveProvider(provider);
sAutoCompleteInitialized = true;
}This eliminates the static ref counter, custom destructor, and parent provider cleanup. The provider stays in memory until app exit, but it's just autocomplete strings so that's fine. Host.cpp looks good 👍 |
- discord.cpp: Move mpHandlers cleanup outside if(mLoaded) block
* Handlers are allocated unconditionally, so cleanup should be too
* Prevents potential leak if Discord library fails to load after allocation
- dlgTriggerEditor: Simplify auto-complete provider using giveProvider()
* Use edbee's giveProvider() to transfer ownership to library
* Provider is automatically deleted at app shutdown
* Eliminates manual reference counting and custom destructor
* Replaces smSharedAutoCompleteProvider/smAutoCompleteProviderRefCount
with simple smAutoCompleteInitialized boolean flag
|
Done |
vadi2
left a comment
vadi2
left a comment
There was a problem hiding this comment.
It looks good now, it's just a few KB of savings in a profile but anything is welcome.
The LLM added a few obvious comments, think a pass on removing those to make the code easier to read for humans would be good 👍
Minor code cleanup: - Remove duplicate explanatory comments about memory ownership - Clean up whitespace in Host destructor
2 participants
Brief overview of PR changes/additions
This PR fixes three memory leaks in Mudlet's core components that were causing gradual memory growth during extended sessions:
giveProvider()Motivation for adding to Mudlet
These memory leaks were causing Mudlet to gradually consume more memory over time, particularly noticeable during extended play sessions. The fixes prevent unnecessary memory growth and improve overall application stability.
Other info (issues closed, discussion etc)
giveProvider(), which handles cleanup automatically at app shutdownBased off of logs message.txt message_1.txt from this Discord discussion.