Move io context to singleton
The way we pass around io contexts is somewhat odd. Boost maintainers
in slack recommended that we just have a method that returns an io
context, and from there we can control this (context link lost years
ago).
The new version of clang claims the singleton pattern of passing in an
io_context pattern is a potential nullptr dereference. It's technically
correct, as calling the singleton without immediately initializing the
io context will lead to a crash.
This commit implements what the boost maintainers suggested, having a
single method that returns "the context" that should be used. This also
helps to maintain isolation, as some pieces are no longer tied directly
to dbus to get their reactor.
Tested: WIP
Change-Id: Ifaa11335ae00a3d092ecfdfb26a38380227e8576
Signed-off-by: Ed Tanous <etanous@nvidia.com>
diff --git a/http/app.hpp b/http/app.hpp
index f982618..f8d4d97 100644
--- a/http/app.hpp
+++ b/http/app.hpp
@@ -7,6 +7,7 @@
#include "async_resp.hpp"
#include "http_request.hpp"
#include "http_server.hpp"
+#include "io_context_singleton.hpp"
#include "logging.hpp"
#include "routing.hpp"
#include "routing/dynamicrule.hpp"
@@ -14,7 +15,6 @@
#include <sys/socket.h>
#include <systemd/sd-daemon.h>
-#include <boost/asio/io_context.hpp>
#include <boost/asio/ip/address.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/ssl/context.hpp>
@@ -44,11 +44,6 @@
raw_socket_t, ssl_socket_t>;
using server_type = Server<App, socket_type>;
- explicit App(std::shared_ptr<boost::asio::io_context> ioIn =
- std::make_shared<boost::asio::io_context>()) :
- io(std::move(ioIn))
- {}
-
template <typename Adaptor>
void handleUpgrade(const std::shared_ptr<Request>& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
@@ -89,13 +84,8 @@
server->loadCertificate();
}
- std::optional<boost::asio::ip::tcp::acceptor> setupSocket()
+ static std::optional<boost::asio::ip::tcp::acceptor> setupSocket()
{
- if (io == nullptr)
- {
- BMCWEB_LOG_CRITICAL("IO was nullptr?");
- return std::nullopt;
- }
constexpr int defaultPort = 18080;
if (sd_listen_fds(0) == 1)
{
@@ -106,7 +96,8 @@
BMCWEB_LOG_INFO("Starting webserver on socket handle {}",
SD_LISTEN_FDS_START);
return boost::asio::ip::tcp::acceptor(
- *io, boost::asio::ip::tcp::v6(), SD_LISTEN_FDS_START);
+ getIoContext(), boost::asio::ip::tcp::v6(),
+ SD_LISTEN_FDS_START);
}
BMCWEB_LOG_ERROR(
"bad incoming socket, starting webserver on port {}",
@@ -114,8 +105,9 @@
}
BMCWEB_LOG_INFO("Starting webserver on port {}", defaultPort);
return boost::asio::ip::tcp::acceptor(
- *io, boost::asio::ip::tcp::endpoint(
- boost::asio::ip::make_address("0.0.0.0"), defaultPort));
+ getIoContext(),
+ boost::asio::ip::tcp::endpoint(
+ boost::asio::ip::make_address("0.0.0.0"), defaultPort));
}
void run()
@@ -128,7 +120,7 @@
BMCWEB_LOG_CRITICAL("Couldn't start server");
return;
}
- server.emplace(this, std::move(*acceptor), sslContext, io);
+ server.emplace(this, std::move(*acceptor), sslContext, getIoContext());
server->run();
}
@@ -158,14 +150,6 @@
std::shared_ptr<boost::asio::ssl::context> sslContext = nullptr;
- boost::asio::io_context& ioContext()
- {
- return *io;
- }
-
- private:
- std::shared_ptr<boost::asio::io_context> io;
-
std::optional<server_type> server;
Router router;
diff --git a/http/http_server.hpp b/http/http_server.hpp
index b48137f..a7c2459 100644
--- a/http/http_server.hpp
+++ b/http/http_server.hpp
@@ -38,10 +38,10 @@
public:
Server(Handler* handlerIn, boost::asio::ip::tcp::acceptor&& acceptorIn,
std::shared_ptr<boost::asio::ssl::context> adaptorCtxIn,
- std::shared_ptr<boost::asio::io_context> io) :
- ioService(std::move(io)), acceptor(std::move(acceptorIn)),
+ boost::asio::io_context& io) :
+ ioService(io), acceptor(std::move(acceptorIn)),
// NOLINTNEXTLINE(misc-include-cleaner)
- signals(*ioService, SIGINT, SIGTERM, SIGHUP), handler(handlerIn),
+ signals(ioService, SIGINT, SIGTERM, SIGHUP), handler(handlerIn),
adaptorCtx(std::move(adaptorCtxIn))
{}
@@ -120,7 +120,7 @@
void stop()
{
- ioService->stop();
+ ioService.stop();
}
using Socket = boost::beast::lowest_layer_type<Adaptor>;
using SocketPtr = std::unique_ptr<Socket>;
@@ -133,7 +133,7 @@
return;
}
- boost::asio::steady_timer timer(*ioService);
+ boost::asio::steady_timer timer(ioService);
std::shared_ptr<Connection<Adaptor, Handler>> connection;
if constexpr (std::is_same<Adaptor,
@@ -157,20 +157,14 @@
Adaptor(std::move(*socket)));
}
- boost::asio::post(*ioService, [connection] { connection->start(); });
+ boost::asio::post(ioService, [connection] { connection->start(); });
doAccept();
}
void doAccept()
{
- if (ioService == nullptr)
- {
- BMCWEB_LOG_CRITICAL("IoService was null");
- return;
- }
-
- SocketPtr socket = std::make_unique<Socket>(*ioService);
+ SocketPtr socket = std::make_unique<Socket>(ioService);
// Keep a raw pointer so when the socket is moved, the pointer is still
// valid
Socket* socketPtr = socket.get();
@@ -181,7 +175,7 @@
}
private:
- std::shared_ptr<boost::asio::io_context> ioService;
+ boost::asio::io_context& ioService;
std::function<std::string()> getCachedDateStr;
boost::asio::ip::tcp::acceptor acceptor;
boost::asio::signal_set signals;
diff --git a/include/io_context_singleton.hpp b/include/io_context_singleton.hpp
new file mode 100644
index 0000000..6d933c1
--- /dev/null
+++ b/include/io_context_singleton.hpp
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: Copyright OpenBMC Authors
+#pragma once
+
+#include <boost/asio/io_context.hpp>
+
+inline boost::asio::io_context& getIoContext()
+{
+ static boost::asio::io_context io;
+ return io;
+}
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 61b837c..a5c22f4 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -10,6 +10,7 @@
#include "event_matches_filter.hpp"
#include "event_service_store.hpp"
#include "filesystem_log_watcher.hpp"
+#include "io_context_singleton.hpp"
#include "logging.hpp"
#include "metric_report.hpp"
#include "ossl_random.hpp"
@@ -18,7 +19,6 @@
#include "subscription.hpp"
#include "utils/time_utils.hpp"
-#include <boost/asio/io_context.hpp>
#include <boost/circular_buffer.hpp>
#include <boost/circular_buffer/base.hpp>
#include <boost/container/flat_map.hpp>
@@ -77,8 +77,6 @@
constexpr static size_t maxMessages = 200;
boost::circular_buffer<Event> messages{maxMessages};
- boost::asio::io_context& ioc;
-
public:
EventServiceManager(const EventServiceManager&) = delete;
EventServiceManager& operator=(const EventServiceManager&) = delete;
@@ -86,16 +84,15 @@
EventServiceManager& operator=(EventServiceManager&&) = delete;
~EventServiceManager() = default;
- explicit EventServiceManager(boost::asio::io_context& iocIn) : ioc(iocIn)
+ explicit EventServiceManager()
{
// Load config from persist store.
initConfig();
}
- static EventServiceManager&
- getInstance(boost::asio::io_context* ioc = nullptr)
+ static EventServiceManager& getInstance()
{
- static EventServiceManager handler(*ioc);
+ static EventServiceManager handler;
return handler;
}
@@ -127,7 +124,7 @@
continue;
}
std::shared_ptr<Subscription> subValue =
- std::make_shared<Subscription>(newSub, *url, ioc);
+ std::make_shared<Subscription>(newSub, *url, getIoContext());
std::string id = subValue->userSub->id;
subValue->deleter = [id]() {
EventServiceManager::getInstance().deleteSubscription(id);
@@ -283,7 +280,7 @@
{
if (!filesystemLogMonitor)
{
- filesystemLogMonitor.emplace(ioc);
+ filesystemLogMonitor.emplace(getIoContext());
}
}
}
@@ -380,7 +377,7 @@
{
if (!filesystemLogMonitor)
{
- filesystemLogMonitor.emplace(ioc);
+ filesystemLogMonitor.emplace(getIoContext());
}
}
}
diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp
index 853f958..9535122 100644
--- a/redfish-core/include/redfish_aggregator.hpp
+++ b/redfish-core/include/redfish_aggregator.hpp
@@ -9,12 +9,12 @@
#include "http_client.hpp"
#include "http_request.hpp"
#include "http_response.hpp"
+#include "io_context_singleton.hpp"
#include "logging.hpp"
#include "parsing.hpp"
#include "ssl_key_handler.hpp"
#include "utility.hpp"
-#include <boost/asio/io_context.hpp>
#include <boost/beast/http/field.hpp>
#include <boost/beast/http/status.hpp>
#include <boost/beast/http/verb.hpp>
@@ -857,8 +857,8 @@
}
public:
- explicit RedfishAggregator(boost::asio::io_context& ioc) :
- client(ioc,
+ explicit RedfishAggregator() :
+ client(getIoContext(),
std::make_shared<crow::ConnectionPolicy>(getAggregationPolicy()))
{
getSatelliteConfigs(constructorCallback);
@@ -869,9 +869,9 @@
RedfishAggregator& operator=(RedfishAggregator&&) = delete;
~RedfishAggregator() = default;
- static RedfishAggregator& getInstance(boost::asio::io_context* io = nullptr)
+ static RedfishAggregator& getInstance()
{
- static RedfishAggregator handler(*io);
+ static RedfishAggregator handler;
return handler;
}
diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp
index 42eb638..fcde1a8 100644
--- a/redfish-core/lib/event_service.hpp
+++ b/redfish-core/lib/event_service.hpp
@@ -12,6 +12,7 @@
#include "generated/enums/event_destination.hpp"
#include "http/utility.hpp"
#include "http_request.hpp"
+#include "io_context_singleton.hpp"
#include "logging.hpp"
#include "query.hpp"
#include "registries.hpp"
@@ -491,7 +492,7 @@
std::shared_ptr<Subscription> subValue =
std::make_shared<Subscription>(
std::make_shared<persistent_data::UserSubscription>(), *url,
- app.ioContext());
+ getIoContext());
if (subscriptionType)
{
diff --git a/redfish-core/lib/eventservice_sse.hpp b/redfish-core/lib/eventservice_sse.hpp
index 86bf1d1..eb0552f 100644
--- a/redfish-core/lib/eventservice_sse.hpp
+++ b/redfish-core/lib/eventservice_sse.hpp
@@ -25,8 +25,7 @@
inline void createSubscription(crow::sse_socket::Connection& conn,
const crow::Request& req)
{
- EventServiceManager& manager =
- EventServiceManager::getInstance(&conn.getIoContext());
+ EventServiceManager& manager = EventServiceManager::getInstance();
if ((manager.getNumberOfSubscriptions() >= maxNoOfSubscriptions) ||
manager.getNumberOfSSESubscriptions() >= maxNoOfSSESubscriptions)
{
@@ -79,8 +78,7 @@
inline void deleteSubscription(crow::sse_socket::Connection& conn)
{
- redfish::EventServiceManager::getInstance(&conn.getIoContext())
- .deleteSseSubscription(conn);
+ EventServiceManager::getInstance().deleteSseSubscription(conn);
}
inline void requestRoutesEventServiceSse(App& app)
diff --git a/src/webserver_run.cpp b/src/webserver_run.cpp
index 557bbe2..bf1168e 100644
--- a/src/webserver_run.cpp
+++ b/src/webserver_run.cpp
@@ -12,6 +12,7 @@
#include "hostname_monitor.hpp"
#include "ibm/management_console_rest.hpp"
#include "image_upload.hpp"
+#include "io_context_singleton.hpp"
#include "kvm_websocket.hpp"
#include "logging.hpp"
#include "login_routes.hpp"
@@ -47,11 +48,11 @@
int run()
{
- auto io = std::make_shared<boost::asio::io_context>();
- App app(io);
+ boost::asio::io_context& io = getIoContext();
+ App app;
std::shared_ptr<sdbusplus::asio::connection> systemBus =
- std::make_shared<sdbusplus::asio::connection>(*io);
+ std::make_shared<sdbusplus::asio::connection>(io);
crow::connections::systemBus = systemBus.get();
auto server = sdbusplus::asio::object_server(systemBus);
@@ -82,12 +83,12 @@
redfish::RedfishService redfish(app);
// Create EventServiceManager instance and initialize Config
- redfish::EventServiceManager::getInstance(&*io);
+ redfish::EventServiceManager::getInstance();
if constexpr (BMCWEB_REDFISH_AGGREGATION)
{
// Create RedfishAggregator instance and initialize Config
- redfish::RedfishAggregator::getInstance(&*io);
+ redfish::RedfishAggregator::getInstance();
}
}
@@ -129,7 +130,7 @@
systemBus->request_name("xyz.openbmc_project.bmcweb");
- io->run();
+ io.run();
crow::connections::systemBus = nullptr;
diff --git a/test/redfish-core/include/redfish_test.cpp b/test/redfish-core/include/redfish_test.cpp
index 57f9cea..9147a7b 100644
--- a/test/redfish-core/include/redfish_test.cpp
+++ b/test/redfish-core/include/redfish_test.cpp
@@ -1,9 +1,6 @@
#include "app.hpp"
#include "redfish.hpp"
-#include <boost/asio/io_context.hpp>
-
-#include <memory>
#include <string>
#include <gmock/gmock.h>
@@ -17,8 +14,7 @@
TEST(Redfish, PathsShouldValidate)
{
- auto io = std::make_shared<boost::asio::io_context>();
- crow::App app(io);
+ crow::App app;
RedfishService redfish(app);