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.

Sign up for GitHub

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 memory leaks caused from the http_connection #633

Merged
merged 1 commit into from
May 22, 2023
Merged

Fix memory leaks caused from the http_connection #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 47 additions & 73 deletions include/crow/http_connection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <atomic>
#include <chrono>
#include <vector>
#include <memory>

#include "crow/http_parser_merged.h"
#include "crow/common.h"
Expand All @@ -31,7 +32,7 @@ namespace crow

/// An HTTP connection.
template<typename Adaptor, typename Handler, typename... Middlewares>
class Connection
class Connection: public std::enable_shared_from_this<Connection<Adaptor, Handler, Middlewares...>>
{
friend struct crow::response;

Expand Down Expand Up @@ -64,8 +65,6 @@ namespace crow

~Connection()
{
res.complete_request_handler_ = nullptr;
cancel_deadline_timer();
#ifdef CROW_ENABLE_DEBUG
connectionCount--;
CROW_LOG_DEBUG << "Connection (" << this << ") freed, total: " << connectionCount;
Expand All @@ -80,18 +79,18 @@ namespace crow

void start()
{
adaptor_.start([this](const asio::error_code& ec) {
auto self = this->shared_from_this();
adaptor_.start([self](const asio::error_code& ec) {
if (!ec)
{
start_deadline();
parser_.clear();
self->start_deadline();
self->parser_.clear();

do_read();
self->do_read();
}
else
{
CROW_LOG_ERROR << "Could not start adaptor: " << ec.message();
check_destroy();
}
});
}
Expand Down Expand Up @@ -161,9 +160,10 @@ namespace crow
need_to_call_after_handlers_ = false;
if (!is_invalid_request)
{
res.complete_request_handler_ = [] {};
res.is_alive_helper_ = [this]() -> bool {
return adaptor_.is_open();
res.complete_request_handler_ = nullptr;
auto self = this->shared_from_this();
res.is_alive_helper_ = [self]() -> bool {
return self->adaptor_.is_open();
};

ctx_ = detail::context<Middlewares...>();
Expand All @@ -176,8 +176,9 @@ namespace crow

if (!res.completed_)
{
res.complete_request_handler_ = [this] {
this->complete_request();
auto self = this->shared_from_this();
res.complete_request_handler_ = [self] {
self->complete_request();
};
need_to_call_after_handlers_ = true;
handler_->handle(req_, res, routing_handle_result_);
Expand All @@ -199,6 +200,7 @@ namespace crow
void complete_request()
{
CROW_LOG_INFO << "Response: " << this << ' ' << req_.raw_url << ' ' << res.code << ' ' << close_connection_;
res.is_alive_helper_ = nullptr;

if (need_to_call_after_handlers_)
{
Expand Down Expand Up @@ -267,8 +269,8 @@ namespace crow
private:
void prepare_buffers()
{
//auto self = this->shared_from_this();
res.complete_request_handler_ = nullptr;
res.is_alive_helper_ = nullptr;

if (!adaptor_.is_open())
{
Expand Down Expand Up @@ -384,7 +386,6 @@ namespace crow

void do_write_static()
{
is_writing = true;
The-EDev marked this conversation as resolved.
Show resolved Hide resolved
asio::write(adaptor_.socket(), buffers_);

if (res.file_info.statResult == 0)
Expand All @@ -400,13 +401,11 @@ namespace crow
is.read(buf, sizeof(buf));
}
}
is_writing = false;
if (close_connection_)
{
adaptor_.shutdown_readwrite();
adaptor_.close();
CROW_LOG_DEBUG << this << " from write (static)";
check_destroy();
}

res.end();
Expand All @@ -433,7 +432,6 @@ namespace crow
}
else
{
is_writing = true;
asio::write(adaptor_.socket(), buffers_); // Write the response start / headers
cancel_deadline_timer();
if (res.body.length() > 0)
Expand All @@ -459,13 +457,11 @@ namespace crow
buffers.push_back(asio::buffer(buf));
do_write_sync(buffers);
}
is_writing = false;
if (close_connection_)
{
adaptor_.shutdown_readwrite();
adaptor_.close();
CROW_LOG_DEBUG << this << " from write (res_stream)";
check_destroy();
}

res.end();
Expand All @@ -477,77 +473,68 @@ namespace crow

void do_read()
{
//auto self = this->shared_from_this();
is_reading = true;
auto self = this->shared_from_this();
adaptor_.socket().async_read_some(
asio::buffer(buffer_),
[this](const asio::error_code& ec, std::size_t bytes_transferred) {
[self](const asio::error_code& ec, std::size_t bytes_transferred) {
bool error_while_reading = true;
if (!ec)
{
bool ret = parser_.feed(buffer_.data(), bytes_transferred);
if (ret && adaptor_.is_open())
bool ret = self->parser_.feed(self->buffer_.data(), bytes_transferred);
if (ret && self->adaptor_.is_open())
{
error_while_reading = false;
}
}

if (error_while_reading)
{
cancel_deadline_timer();
parser_.done();
adaptor_.shutdown_read();
adaptor_.close();
is_reading = false;
CROW_LOG_DEBUG << this << " from read(1) with description: \"" << http_errno_description(static_cast<http_errno>(parser_.http_errno)) << '\"';
check_destroy();
self->cancel_deadline_timer();
self->parser_.done();
self->adaptor_.shutdown_read();
self->adaptor_.close();
CROW_LOG_DEBUG << self << " from read(1) with description: \"" << http_errno_description(static_cast<http_errno>(self->parser_.http_errno)) << '\"';
}
else if (close_connection_)
else if (self->close_connection_)
{
cancel_deadline_timer();
parser_.done();
is_reading = false;
check_destroy();
self->cancel_deadline_timer();
self->parser_.done();
// adaptor will close after write
}
else if (!need_to_call_after_handlers_)
else if (!self->need_to_call_after_handlers_)
{
start_deadline();
do_read();
self->start_deadline();
self->do_read();
}
else
{
// res will be completed later by user
need_to_start_read_after_complete_ = true;
self->need_to_start_read_after_complete_ = true;
}
});
}

void do_write()
{
//auto self = this->shared_from_this();
is_writing = true;
auto self = this->shared_from_this();
asio::async_write(
adaptor_.socket(), buffers_,
[&](const asio::error_code& ec, std::size_t /*bytes_transferred*/) {
is_writing = false;
res.clear();
res_body_copy_.clear();
parser_.clear();
[self](const asio::error_code& ec, std::size_t /*bytes_transferred*/) {
self->res.clear();
self->res_body_copy_.clear();
self->parser_.clear();
if (!ec)
{
if (close_connection_)
if (self->close_connection_)
{
adaptor_.shutdown_write();
adaptor_.close();
CROW_LOG_DEBUG << this << " from write(1)";
check_destroy();
self->adaptor_.shutdown_write();
self->adaptor_.close();
CROW_LOG_DEBUG << self << " from write(1)";
}
}
else
{
CROW_LOG_DEBUG << this << " from write(2)";
check_destroy();
CROW_LOG_DEBUG << self << " from write(2)";
}
});
}
Expand All @@ -564,23 +551,11 @@ namespace crow
{
CROW_LOG_ERROR << ec << " - happened while sending buffers";
CROW_LOG_DEBUG << this << " from write (sync)(2)";
check_destroy();
return true;
}
});
}

void check_destroy()
{
CROW_LOG_DEBUG << this << " is_reading " << is_reading << " is_writing " << is_writing;
if (!is_reading && !is_writing)
{
queue_length_--;
CROW_LOG_DEBUG << this << " delete (idle) (queue length: " << queue_length_ << ')';
delete this;
}
}

void cancel_deadline_timer()
{
CROW_LOG_DEBUG << this << " timer cancelled: " << &task_timer_ << ' ' << task_id_;
Expand All @@ -591,13 +566,14 @@ namespace crow
{
cancel_deadline_timer();

task_id_ = task_timer_.schedule([this] {
if (!adaptor_.is_open())
auto self = this->shared_from_this();
task_id_ = task_timer_.schedule([self] {
if (!self->adaptor_.is_open())
{
return;
}
adaptor_.shutdown_readwrite();
adaptor_.close();
self->adaptor_.shutdown_readwrite();
self->adaptor_.close();
});
CROW_LOG_DEBUG << this << " timer added: " << &task_timer_ << ' ' << task_id_;
}
Expand All @@ -624,8 +600,6 @@ namespace crow

detail::task_timer::identifier_type task_id_{};

bool is_reading{};
bool is_writing{};
bool need_to_call_after_handlers_{};
bool need_to_start_read_after_complete_{};
bool add_keep_alive_{};
Expand Down
3 changes: 1 addition & 2 deletions include/crow/http_server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ namespace crow
task_queue_length_pool_[service_idx]++;
CROW_LOG_DEBUG << &is << " {" << service_idx << "} queue length: " << task_queue_length_pool_[service_idx];

auto p = new Connection<Adaptor, Handler, Middlewares...>(
auto p = std::make_shared<Connection<Adaptor, Handler, Middlewares...>>(
is, handler_, server_name_, middlewares_,
get_cached_date_str_pool_[service_idx], *task_timer_pool_[service_idx], adaptor_ctx_, task_queue_length_pool_[service_idx]);

Expand All @@ -239,7 +239,6 @@ namespace crow
{
task_queue_length_pool_[service_idx]--;
CROW_LOG_DEBUG << &is << " {" << service_idx << "} queue length: " << task_queue_length_pool_[service_idx];
delete p;
}
do_accept();
});
Expand Down
2 changes: 1 addition & 1 deletion include/crow/routing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1718,7 +1718,7 @@ namespace crow
return;
}

res.complete_request_handler_ = [&rule, &ctx, &container, &req, &res, &glob_completion_handler] {
res.complete_request_handler_ = [&rule, &ctx, &container, &req, &res, glob_completion_handler] {
detail::middleware_call_criteria_dynamic<true> crit_bwd(rule->mw_indices_.indices());

detail::after_handlers_call_helper<
Expand Down