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 list votes load memory leak #31

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

Adrianilloo
Copy link
Contributor

@Adrianilloo Adrianilloo commented Dec 19, 2022

Hi,

Previous contributor here (fixed past memory leaks). I have casually found there were other leaks, still, arising every map change.

This PR fixes a memory leak at config parsing for List votes, as the code was pushing array strings in the wrong scope (a search loop).

The commit also enhances same parsing as the code located the assumed "Options" section in a brute force way (inadequately). Now it searches for the exact "Options" key for adding the List vote options.

Here you can see a SM Handles dump difference (using this viewer), after running a HL2DM server for nearly 6 days, at ~15 minutes average per map:

customvotes handles dump

@caxanga334 caxanga334 self-assigned this Dec 19, 2022
@caxanga334 caxanga334 added the bug Something isn't working label Dec 19, 2022
@caxanga334 caxanga334 added this to To do in Development via automation Dec 19, 2022
@caxanga334 caxanga334 merged commit 7933f1e into caxanga334:master Dec 19, 2022
Development automation moved this from To do to Done Dec 19, 2022
@Adrianilloo Adrianilloo deleted the fix-listvotes-memleak branch December 19, 2022 16:04
@Adrianilloo
Copy link
Contributor Author

It turns out that, while my fix solved a potential memory leak (by design), the apparent one I observed in my server (as reflected in the SM handles dump comparison screenshot) wasn't related and not even a memory leak. It was just the recent map list array (g_hArrayRecentMaps) increasing on map change, as of normal functioning.

Nevertheless, this PR indeed fixes a memory leak if the user had a custom configuration like this due to the "brute force" KV loop approach, on a List type:

"List type vote"
{
    "type" "list"
    "vote" "1"

    "Options"
    {
        "Option 1" "1" // g_hArrayVoteOptionName[iVote] = CreateArray(16);
        "Option 2" "2"
    }

    "Options2" // Now ignored as of this PR
    {
        "Option 1" "1" // g_hArrayVoteOptionName[iVote] = CreateArray(16); -> LEAK (iVote = old iVote)
        "Option 2" "2"
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants