brainboxdotcc / DPP

C++ Discord API Bot Library - D++ is Lightweight and scalable for small and huge bots!
https://dpp.dev/
Apache License 2.0
1.07k stars 163 forks source link

Memory leak in request_queue when sending messages with files #1091

Closed Jaskowicz1 closed 2 months ago

Jaskowicz1 commented 8 months ago

Git commit reference 9c549f56feff00b13fc83193ce4aea6dc009058a

Describe the bug Sending messages, with files, causes a memory leak. The leak is bigger if the file is bigger.

The issue seems to be coming from:

/* Check for deletable items every second regardless of select status */
while (responses_to_delete.size() && now >= responses_to_delete.begin()->first) {
    delete responses_to_delete.begin()->second.first;
    delete responses_to_delete.begin()->second.second;
    responses_to_delete.erase(responses_to_delete.begin());
}

and the queue itself.

The data doesn't actually get removed until the final request has been deleted from the queue.

Iteration 11 running to check RAM from queue. 46837760 RAM in use.
responses_to_delete: 1
Deleting response...
Iteration 12 running to check RAM from queue. 27660288 RAM in use

if you're making frequent requests, the map will never empty, meaning the data is never freed (not sure why).

However, even when the queue is now empty, the ram doesn't fully recover. For example:

Test 0 started, 14778368 RAM in use.
// .......
Test 11 started, 45486080 RAM in use.
Iteration 0 running to check RAM from queue. 46837760 RAM in use.
// .......
Iteration 11 running to check RAM from queue. 46837760 RAM in use.
// .......
Iteration 12 running to check RAM from queue. 27660288 RAM in use.

To Reproduce 1) Run the following code:

int main() {
    /* Create the bot */
    dpp::cluster bot("Token");
    bot.on_log(dpp::utility::cout_logger());

    bot.on_ready([&bot](const dpp::ready_t& event) {
        for (int i = 0; i < 12; i++) {
            bot.message_create(dpp::message(channel_id, "").add_file("test.jpg", dpp::utility::read_file("image.jpg")));
            std::this_thread::sleep_for(std::chrono::seconds(5)); // Just to avoid rate limits.
        }
    });

    bot.start(dpp::st_wait);

    return 0;
}

2) Watch the memory increase, then wait 120 seconds (for all 12 messages to clear) and watch the memory massively decrease.

Expected behavior The memory should slowly clear with every remove from the queue.

Ideally, we should use unique_ptr so we can avoid the 60 second queue sillyness.

System Details:

braindigitalis commented 8 months ago

as well as unique_ptr you may have to rehash the maps periodically by copying all data to a new one. This is the only way to force unordered map to release its buckets.

Jaskowicz1 commented 8 months ago

This does not seem to occur on Windows and currently looks like a Linux only bug...

Jaskowicz1 commented 7 months ago

I'm now working on this.

Jaskowicz1 commented 7 months ago

To further make this confusing, this leak does not happen on Mac OSX either. So it really is Linux only...

Neko-Life commented 7 months ago

:thonk: I think i can look into that, ima test on my machine

Jaskowicz1 commented 7 months ago

The fix that @Mishura4 implemented doesn't seem to work. This also seems to be an issue with only libstdc++. Upgrading to a newer g++ fixes it (as long as you use that newer g++) as it appears newer versions of libstdc++ have this fixed.

braindigitalis commented 7 months ago

updating to what? I'm on g++ 12.3.0 on my production machines, where the problem persists...? It's literally the latest available g++ on ubuntu 22.04.

The fix that @Mishura4 implemented doesn't seem to work. This also seems to be an issue with only libstdc++. Upgrading to a newer g++ fixes it (as long as you use that newer g++) as it appears newer versions of libstdc++ have this fixed.

Jaskowicz1 commented 7 months ago

updating to what? I'm on g++ 12.3.0 on my production machines, where the problem persists...? It's literally the latest available g++ on ubuntu 22.04.

The fix that @Mishura4 implemented doesn't seem to work. This also seems to be an issue with only libstdc++. Upgrading to a newer g++ fixes it (as long as you use that newer g++) as it appears newer versions of libstdc++ have this fixed.

Oh? Maybe upgrading isn't a fix then.

github-actions[bot] commented 2 months ago

This issue has had no activity and is being marked as stale. If you still wish to continue with this issue please comment to reopen it.