Revert "Connection and websockets fixes"
This reverts commit c00500bcb9c5145f5cacb78bbe3dd694fb85ba0a.
Reason: Makes image upload fail
Tested: Image upload works again
requests.post(
'https://{}/redfish/v1/UpdateService'.format(args.address),
data=file.read(), verify=False,
auth=(args.username, args.password))
Change-Id: Iaf780d052d98accdead32e87f468002f5141b19a
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/http/http_connection.h b/http/http_connection.h
index 4ef3bc6..7d24fe7 100644
--- a/http/http_connection.h
+++ b/http/http_connection.h
@@ -248,8 +248,7 @@
1024 * 1024 * BMCWEB_HTTP_REQ_BODY_LIMIT_MB;
template <typename Adaptor, typename Handler, typename... Middlewares>
-class Connection : public std::enable_shared_from_this<
- Connection<Adaptor, Handler, Middlewares...>>
+class Connection
{
public:
Connection(boost::asio::io_context& ioService, Handler* handlerIn,
@@ -475,15 +474,16 @@
boost::beast::ssl_stream<
boost::asio::ip::tcp::socket>>)
{
- adaptor.async_handshake(boost::asio::ssl::stream_base::server,
- [this, self(shared_from_this())](
- const boost::system::error_code& ec) {
- if (ec)
- {
- return;
- }
- doReadHeaders();
- });
+ adaptor.async_handshake(
+ boost::asio::ssl::stream_base::server,
+ [this](const boost::system::error_code& ec) {
+ if (ec)
+ {
+ checkDestroy();
+ return;
+ }
+ doReadHeaders();
+ });
}
else
{
@@ -561,21 +561,18 @@
if (!res.completed)
{
- needToCallAfterHandlers = true;
- res.completeRequestHandler = [self(shared_from_this())] {
- self->completeRequest();
- };
if (req->isUpgrade() &&
boost::iequals(
req->getHeaderValue(boost::beast::http::field::upgrade),
"websocket"))
{
handler->handleUpgrade(*req, res, std::move(adaptor));
- // delete lambda with self shared_ptr
- // to enable connection destruction
- res.completeRequestHandler = nullptr;
return;
}
+ res.completeRequestHandler = [this] {
+ this->completeRequest();
+ };
+ needToCallAfterHandlers = true;
handler->handle(*req, res);
}
else
@@ -641,16 +638,15 @@
*middlewares, ctx, *req, res);
}
+ // auto self = this->shared_from_this();
+ res.completeRequestHandler = res.completeRequestHandler = [] {};
+
if (!isAlive())
{
// BMCWEB_LOG_DEBUG << this << " delete (socket is closed) " <<
// isReading
// << ' ' << isWriting;
// delete this;
-
- // delete lambda with self shared_ptr
- // to enable connection destruction
- res.completeRequestHandler = nullptr;
return;
}
if (res.body().empty() && !res.jsonValue.empty())
@@ -687,23 +683,21 @@
res.keepAlive(req->keepAlive());
doWrite();
-
- // delete lambda with self shared_ptr
- // to enable connection destruction
- res.completeRequestHandler = nullptr;
}
private:
void doReadHeaders()
{
+ // auto self = this->shared_from_this();
+ isReading = true;
BMCWEB_LOG_DEBUG << this << " doReadHeaders";
// Clean up any previous Connection.
boost::beast::http::async_read_header(
adaptor, buffer, *parser,
- [this,
- self(shared_from_this())](const boost::system::error_code& ec,
- std::size_t bytes_transferred) {
+ [this](const boost::system::error_code& ec,
+ std::size_t bytes_transferred) {
+ isReading = false;
BMCWEB_LOG_ERROR << this << " async_read_header "
<< bytes_transferred << " Bytes";
bool errorWhileReading = false;
@@ -728,6 +722,7 @@
cancelDeadlineTimer();
close();
BMCWEB_LOG_DEBUG << this << " from read(1)";
+ checkDestroy();
return;
}
@@ -745,15 +740,17 @@
void doRead()
{
+ // auto self = this->shared_from_this();
+ isReading = true;
BMCWEB_LOG_DEBUG << this << " doRead";
boost::beast::http::async_read(
adaptor, buffer, *parser,
- [this,
- self(shared_from_this())](const boost::system::error_code& ec,
- std::size_t bytes_transferred) {
+ [this](const boost::system::error_code& ec,
+ std::size_t bytes_transferred) {
BMCWEB_LOG_DEBUG << this << " async_read " << bytes_transferred
<< " Bytes";
+ isReading = false;
bool errorWhileReading = false;
if (ec)
@@ -774,6 +771,7 @@
cancelDeadlineTimer();
close();
BMCWEB_LOG_DEBUG << this << " from read(1)";
+ checkDestroy();
return;
}
handle();
@@ -782,26 +780,30 @@
void doWrite()
{
+ // auto self = this->shared_from_this();
+ isWriting = true;
BMCWEB_LOG_DEBUG << this << " doWrite";
res.preparePayload();
serializer.emplace(*res.stringResponse);
boost::beast::http::async_write(
adaptor, *serializer,
- [this,
- self(shared_from_this())](const boost::system::error_code& ec,
- std::size_t bytes_transferred) {
+ [&](const boost::system::error_code& ec,
+ std::size_t bytes_transferred) {
+ isWriting = false;
BMCWEB_LOG_DEBUG << this << " async_write " << bytes_transferred
<< " bytes";
if (ec)
{
BMCWEB_LOG_DEBUG << this << " from write(2)";
+ checkDestroy();
return;
}
if (!res.keepAlive())
{
close();
BMCWEB_LOG_DEBUG << this << " from write(1)";
+ checkDestroy();
return;
}
@@ -818,25 +820,29 @@
});
}
+ void checkDestroy()
+ {
+ BMCWEB_LOG_DEBUG << this << " isReading " << isReading << " isWriting "
+ << isWriting;
+ if (!isReading && !isWriting)
+ {
+ BMCWEB_LOG_DEBUG << this << " delete (idle) ";
+ delete this;
+ }
+ }
+
void cancelDeadlineTimer()
{
- if (timerCancelKey)
- {
- BMCWEB_LOG_DEBUG << this << " timer cancelled: " << &timerQueue
- << ' ' << *timerCancelKey;
- timerQueue.cancel(*timerCancelKey);
- timerCancelKey.reset();
- }
+ BMCWEB_LOG_DEBUG << this << " timer cancelled: " << &timerQueue << ' '
+ << timerCancelKey;
+ timerQueue.cancel(timerCancelKey);
}
void startDeadline()
{
cancelDeadlineTimer();
- timerCancelKey = timerQueue.add([this, self(shared_from_this())] {
- // Mark timer as not active to avoid canceling it during
- // Connection destructor which leads to double free issue
- timerCancelKey.reset();
+ timerCancelKey = timerQueue.add([this] {
if (!isAlive())
{
return;
@@ -844,7 +850,7 @@
close();
});
BMCWEB_LOG_DEBUG << this << " timer added: " << &timerQueue << ' '
- << *timerCancelKey;
+ << timerCancelKey;
}
private:
@@ -871,8 +877,10 @@
const std::string& serverName;
- std::optional<size_t> timerCancelKey;
+ size_t timerCancelKey = 0;
+ bool isReading{};
+ bool isWriting{};
bool needToCallAfterHandlers{};
bool needToStartReadAfterComplete{};
@@ -881,8 +889,5 @@
std::function<std::string()>& getCachedDateStr;
detail::TimerQueue& timerQueue;
-
- using std::enable_shared_from_this<
- Connection<Adaptor, Handler, Middlewares...>>::shared_from_this;
};
} // namespace crow
diff --git a/http/http_server.h b/http/http_server.h
index 6e63cbd..df42214 100644
--- a/http/http_server.h
+++ b/http/http_server.h
@@ -44,8 +44,7 @@
ioService(std::move(io)),
acceptor(std::move(acceptor)),
signals(*ioService, SIGINT, SIGTERM, SIGHUP), tickTimer(*ioService),
- timer(*ioService), handler(handler), middlewares(middlewares),
- adaptorCtx(adaptor_ctx)
+ handler(handler), middlewares(middlewares), adaptorCtx(adaptor_ctx)
{
}
@@ -124,9 +123,11 @@
return this->dateStr;
};
+ boost::asio::steady_timer timer(*ioService);
timer.expires_after(std::chrono::seconds(1));
- timerHandler = [this](const boost::system::error_code& ec) {
+ std::function<void(const boost::system::error_code& ec)> timerHandler;
+ timerHandler = [&](const boost::system::error_code& ec) {
if (ec)
{
return;
@@ -230,8 +231,8 @@
boost::asio::ip::tcp::socket>>::value)
{
adaptorTemp = Adaptor(*ioService, *adaptorCtx);
- auto p =
- std::make_shared<Connection<Adaptor, Handler, Middlewares...>>(
+ Connection<Adaptor, Handler, Middlewares...>* p =
+ new Connection<Adaptor, Handler, Middlewares...>(
*ioService, handler, serverName, middlewares,
getCachedDateStr, timerQueue,
std::move(adaptorTemp.value()));
@@ -244,14 +245,18 @@
*this->ioService,
[p] { p->start(); });
}
+ else
+ {
+ delete p;
+ }
doAccept();
});
}
else
{
adaptorTemp = Adaptor(*ioService);
- auto p =
- std::make_shared<Connection<Adaptor, Handler, Middlewares...>>(
+ Connection<Adaptor, Handler, Middlewares...>* p =
+ new Connection<Adaptor, Handler, Middlewares...>(
*ioService, handler, serverName, middlewares,
getCachedDateStr, timerQueue,
std::move(adaptorTemp.value()));
@@ -263,6 +268,10 @@
boost::asio::post(*this->ioService,
[p] { p->start(); });
}
+ else
+ {
+ delete p;
+ }
doAccept();
});
}
@@ -275,7 +284,6 @@
std::unique_ptr<tcp::acceptor> acceptor;
boost::asio::signal_set signals;
boost::asio::steady_timer tickTimer;
- boost::asio::steady_timer timer;
std::string dateStr;
@@ -284,7 +292,6 @@
std::chrono::milliseconds tickInterval{};
std::function<void()> tickFunction;
- std::function<void(const boost::system::error_code& ec)> timerHandler;
std::tuple<Middlewares...>* middlewares;
diff --git a/http/routing.h b/http/routing.h
index c2a7503..f194ad1 100644
--- a/http/routing.h
+++ b/http/routing.h
@@ -324,19 +324,19 @@
res.end();
}
- void handleUpgrade(const Request& req, Response&,
+ void handleUpgrade(const Request& req, Response& res,
boost::asio::ip::tcp::socket&& adaptor) override
{
std::shared_ptr<
crow::websocket::ConnectionImpl<boost::asio::ip::tcp::socket>>
myConnection = std::make_shared<
crow::websocket::ConnectionImpl<boost::asio::ip::tcp::socket>>(
- req, std::move(adaptor), openHandler, messageHandler,
+ req, res, std::move(adaptor), openHandler, messageHandler,
closeHandler, errorHandler);
myConnection->start();
}
#ifdef BMCWEB_ENABLE_SSL
- void handleUpgrade(const Request& req, Response&,
+ void handleUpgrade(const Request& req, Response& res,
boost::beast::ssl_stream<boost::asio::ip::tcp::socket>&&
adaptor) override
{
@@ -344,7 +344,7 @@
boost::beast::ssl_stream<boost::asio::ip::tcp::socket>>>
myConnection = std::make_shared<crow::websocket::ConnectionImpl<
boost::beast::ssl_stream<boost::asio::ip::tcp::socket>>>(
- req, std::move(adaptor), openHandler, messageHandler,
+ req, res, std::move(adaptor), openHandler, messageHandler,
closeHandler, errorHandler);
myConnection->start();
}
diff --git a/http/websocket.h b/http/websocket.h
index f7c818e..80d536a 100644
--- a/http/websocket.h
+++ b/http/websocket.h
@@ -20,8 +20,8 @@
struct Connection : std::enable_shared_from_this<Connection>
{
public:
- explicit Connection(const crow::Request& reqIn) :
- req(reqIn.req), userdataPtr(nullptr){};
+ explicit Connection(const crow::Request& reqIn, crow::Response& res) :
+ req(reqIn), userdataPtr(nullptr){};
virtual void sendBinary(const std::string_view msg) = 0;
virtual void sendBinary(std::string&& msg) = 0;
@@ -40,7 +40,7 @@
return userdataPtr;
}
- boost::beast::http::request<boost::beast::http::string_body> req;
+ crow::Request req;
crow::Response res;
private:
@@ -51,14 +51,14 @@
{
public:
ConnectionImpl(
- const crow::Request& reqIn, Adaptor adaptorIn,
+ const crow::Request& reqIn, crow::Response& res, Adaptor adaptorIn,
std::function<void(Connection&, std::shared_ptr<bmcweb::AsyncResp>)>
open_handler,
std::function<void(Connection&, const std::string&, bool)>
message_handler,
std::function<void(Connection&, const std::string&)> close_handler,
std::function<void(Connection&)> error_handler) :
- Connection(reqIn),
+ Connection(reqIn, res),
ws(std::move(adaptorIn)), inString(), inBuffer(inString, 131088),
openHandler(std::move(open_handler)),
messageHandler(std::move(message_handler)),
@@ -80,11 +80,12 @@
using bf = boost::beast::http::field;
- std::string_view protocol = req[bf::sec_websocket_protocol];
+ std::string_view protocol =
+ req.getHeaderValue(bf::sec_websocket_protocol);
// Perform the websocket upgrade
ws.async_accept_ex(
- req,
+ req.req,
[protocol{std::string(protocol)}](
boost::beast::websocket::response_type& m) {
if (!protocol.empty())