Skip to content

Commit

Permalink
fix: fix memory leak on libstdc++
Browse files Browse the repository at this point in the history
ditched the std::multimap in queues.cpp for a sorted vector, and established clearer ownership of the request data
yes the requests are still kept alive for a minute
  • Loading branch information
Mishura4 committed Mar 29, 2024
1 parent 44bfb17 commit c9a4a01
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 97 deletions.
61 changes: 52 additions & 9 deletions include/dpp/queues.h
Original file line number Diff line number Diff line change
Expand Up @@ -434,9 +434,9 @@ class DPP_EXPORT in_thread {
std::map<std::string, bucket_t> buckets;

/**
* @brief Queue of requests to be made.
* @brief Queue of requests to be made. Sorted by http_request::endpoint.
*/
std::map<std::string, std::vector<http_request*>> requests_in;
std::vector<std::unique_ptr<http_request>> requests_in;

/**
* @brief Inbound queue thread loop.
Expand Down Expand Up @@ -465,7 +465,7 @@ class DPP_EXPORT in_thread {
* @param req http_request to post. The pointer will be freed when it has
* been executed.
*/
void post_request(http_request* req);
void post_request(std::unique_ptr<http_request> req);
};

/**
Expand Down Expand Up @@ -516,10 +516,25 @@ class DPP_EXPORT request_queue {
*/
std::condition_variable out_ready;

/**
* @brief A completed request. Contains both the request and the response
*/
struct completed_request {
/**
* @brief Request sent
*/
std::unique_ptr<http_request> request;

/**
* @brief Response to the request
*/
std::unique_ptr<http_request_completion_t> response;
};

/**
* @brief Completed requests queue
*/
std::queue<std::pair<http_request_completion_t*, http_request*>> responses_out;
std::queue<completed_request> responses_out;

/**
* @brief A vector of inbound request threads forming a pool.
Expand All @@ -533,9 +548,38 @@ class DPP_EXPORT request_queue {
std::vector<in_thread*> requests_in;

/**
* @brief Completed requests to delete
* @brief A request queued for deletion in the queue.
*/
struct queued_deleting_request {
/**
* @brief Time to delete the request
*/
time_t time_to_delete;

/**
* @brief The request to delete
*/
completed_request request;

/**
* @brief Comparator for sorting purposes
* @param other Other queued request to compare the deletion time with
* @return bool Whether this request comes before another in strict ordering
*/
bool operator<(const queued_deleting_request& other) const noexcept;

/**
* @brief Comparator for sorting purposes
* @param time Time to compare with
* @return bool Whether this request's deletion time is lower than the time given, for strict ordering
*/
bool operator<(time_t time) const noexcept;
};

/**
* @brief Completed requests to delete. Sorted by deletion time
*/
std::multimap<time_t, std::pair<http_request_completion_t*, http_request*>> responses_to_delete;
std::vector<queued_deleting_request> responses_to_delete;

/**
* @brief Set to true if the threads should terminate
Expand Down Expand Up @@ -597,14 +641,13 @@ class DPP_EXPORT request_queue {
~request_queue();

/**
* @brief Put a http_request into the request queue. You should ALWAYS "new" an object
* to pass to here -- don't submit an object that's on the stack!
* @brief Put a http_request into the request queue.
* @note Will use a simple hash function to determine which of the 'in queues' to place
* this request onto.
* @param req request to add
* @return reference to self
*/
request_queue& post_request(http_request *req);
request_queue& post_request(std::unique_ptr<http_request> req);

/**
* @brief Returns true if the bot is currently globally rate limited
Expand Down
6 changes: 3 additions & 3 deletions src/dpp/cluster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ json error_response(const std::string& message, http_request_completion_t& rv)

void cluster::post_rest(const std::string &endpoint, const std::string &major_parameters, const std::string &parameters, http_method method, const std::string &postdata, json_encode_t callback, const std::string &filename, const std::string &filecontent, const std::string &filemimetype, const std::string &protocol) {
/* NOTE: This is not a memory leak! The request_queue will free the http_request once it reaches the end of its lifecycle */
rest->post_request(new http_request(endpoint + (!major_parameters.empty() ? "/" : "") + major_parameters, parameters, [endpoint, callback](http_request_completion_t rv) {
rest->post_request(std::make_unique<http_request>(endpoint + (!major_parameters.empty() ? "/" : "") + major_parameters, parameters, [endpoint, callback](http_request_completion_t rv) {
json j;
if (rv.error == h_success && !rv.body.empty()) {
try {
Expand Down Expand Up @@ -351,7 +351,7 @@ void cluster::post_rest_multipart(const std::string &endpoint, const std::string
}

/* NOTE: This is not a memory leak! The request_queue will free the http_request once it reaches the end of its lifecycle */
rest->post_request(new http_request(endpoint + (!major_parameters.empty() ? "/" : "") + major_parameters, parameters, [endpoint, callback](http_request_completion_t rv) {
rest->post_request(std::make_unique<http_request>(endpoint + (!major_parameters.empty() ? "/" : "") + major_parameters, parameters, [endpoint, callback](http_request_completion_t rv) {
json j;
if (rv.error == h_success && !rv.body.empty()) {
try {
Expand All @@ -370,7 +370,7 @@ void cluster::post_rest_multipart(const std::string &endpoint, const std::string

void cluster::request(const std::string &url, http_method method, http_completion_event callback, const std::string &postdata, const std::string &mimetype, const std::multimap<std::string, std::string> &headers, const std::string &protocol) {
/* NOTE: This is not a memory leak! The request_queue will free the http_request once it reaches the end of its lifecycle */
raw_rest->post_request(new http_request(url, callback, method, postdata, mimetype, headers, protocol));
raw_rest->post_request(std::make_unique<http_request>(url, callback, method, postdata, mimetype, headers, protocol));
}

gateway::gateway() : shards(0), session_start_total(0), session_start_remaining(0), session_start_reset_after(0), session_start_max_concurrency(0) {
Expand Down
Loading

0 comments on commit c9a4a01

Please sign in to comment.