From 30a4f7699001a4af383600af2452d12b392ae2c2 Mon Sep 17 00:00:00 2001 From: Justin Karneges Date: Mon, 24 Feb 2025 15:10:17 -0800 Subject: [PATCH 1/2] handler: read config before starting event loop This is prep work for moving to the new event loop which requires the config in advance. --- src/handler/handlerapp.cpp | 137 +++++++++++++++++----------------- src/handler/handlerapp.h | 19 ++--- src/handler/handlerengine.cpp | 14 ---- src/handler/handlerengine.h | 2 + src/handler/handlermain.cpp | 39 +--------- 5 files changed, 79 insertions(+), 132 deletions(-) diff --git a/src/handler/handlerapp.cpp b/src/handler/handlerapp.cpp index e2152257..0f578260 100644 --- a/src/handler/handlerapp.cpp +++ b/src/handler/handlerapp.cpp @@ -1,6 +1,6 @@ /* * Copyright (C) 2015-2022 Fanout, Inc. - * Copyright (C) 2024 Fastly, Inc. + * Copyright (C) 2024-2025 Fastly, Inc. * * This file is part of Pushpin. * @@ -29,8 +29,13 @@ #include #include #include +#include "timer.h" +#include "defercall.h" #include "processquit.h" #include "log.h" +#include "httpsession.h" +#include "wssession.h" +#include "httpsessionupdatemanager.h" #include "settings.h" #include "handlerengine.h" #include "config.h" @@ -169,27 +174,10 @@ static CommandLineParseResult parseCommandLine(QCommandLineParser *parser, ArgsD return CommandLineOk; } -class HandlerApp::Private : public QObject +class HandlerApp::Private { - Q_OBJECT - public: - HandlerApp *q; - ArgsData args; - HandlerEngine *engine; - Connection quitConnection; - Connection hupConnection; - - Private(HandlerApp *_q) : - QObject(_q), - q(_q), - engine(0) - { - quitConnection = ProcessQuit::instance()->quit.connect(boost::bind(&Private::doQuit, this)); - hupConnection = ProcessQuit::instance()->hup.connect(boost::bind(&Private::reload, this)); - } - - void start() + static int run() { QCoreApplication::setApplicationName("pushpin-handler"); QCoreApplication::setApplicationVersion(Config::get().version); @@ -197,6 +185,7 @@ class HandlerApp::Private : public QObject QCommandLineParser parser; parser.setApplicationDescription("Pushpin handler component."); + ArgsData args; QString errorMessage; switch(parseCommandLine(&parser, &args, &errorMessage)) { @@ -204,13 +193,11 @@ class HandlerApp::Private : public QObject break; case CommandLineError: fprintf(stderr, "%s\n\n%s", qPrintable(errorMessage), qPrintable(parser.helpText())); - q->quit(1); - return; + return 1; case CommandLineVersionRequested: printf("%s %s\n", qPrintable(QCoreApplication::applicationName()), qPrintable(QCoreApplication::applicationVersion())); - q->quit(0); - return; + return 0; case CommandLineHelpRequested: parser.showHelp(); Q_UNREACHABLE(); @@ -226,8 +213,7 @@ class HandlerApp::Private : public QObject if(!log_setFile(args.logFile)) { log_error("failed to open log file: %s", qPrintable(args.logFile)); - q->quit(1); - return; + return 1; } } @@ -243,8 +229,7 @@ class HandlerApp::Private : public QObject if(!file.open(QIODevice::ReadOnly)) { log_error("failed to open %s, and --config not passed", qPrintable(configFile)); - q->quit(0); - return; + return 1; } } @@ -340,15 +325,13 @@ class HandlerApp::Private : public QObject if(m2a_in_stream_specs.isEmpty() || m2a_out_specs.isEmpty()) { log_error("must set m2a_in_stream_specs and m2a_out_specs"); - q->quit(0); - return; + return 1; } if(proxy_inspect_specs.isEmpty() || proxy_accept_specs.isEmpty() || proxy_retry_out_specs.isEmpty()) { log_error("must set proxy_inspect_specs, proxy_accept_specs, and proxy_retry_out_specs"); - q->quit(0); - return; + return 1; } HandlerEngine::Configuration config; @@ -402,53 +385,73 @@ class HandlerApp::Private : public QObject config.prometheusPort = prometheusPort; config.prometheusPrefix = prometheusPrefix; - engine = new HandlerEngine(this); - if(!engine->start(config)) - { - q->quit(0); - return; - } - - log_info("started"); + return runLoop(config); } private: - void reload() + static int runLoop(const HandlerEngine::Configuration &config) { - log_info("reloading"); - log_rotate(); - engine->reload(); - } + // includes worst-case subscriptions and update registrations + int timersPerSession = qMax(TIMERS_PER_HTTPSESSION, TIMERS_PER_WSSESSION) + + (config.connectionSubscriptionMax * TIMERS_PER_SUBSCRIPTION) + + TIMERS_PER_UNIQUE_UPDATE_REGISTRATION; - void doQuit() - { - log_info("stopping..."); + // enough timers for sessions, plus an extra 100 for misc + Timer::init((config.connectionsMax * timersPerSession) + 100); + + std::unique_ptr engine; + + DeferCall deferCall; + deferCall.defer([&] { + engine = std::make_unique(); + + ProcessQuit::instance()->quit.connect([&] { + log_info("stopping..."); - // remove the handler, so if we get another signal then we crash out - ProcessQuit::cleanup(); + // remove the handler, so if we get another signal then we crash out + ProcessQuit::cleanup(); - delete engine; - engine = 0; + engine.reset(); - log_debug("stopped"); - q->quit(0); + log_debug("stopped"); + + QCoreApplication::exit(0); + }); + + ProcessQuit::instance()->hup.connect([&] { + log_info("reloading"); + log_rotate(); + engine->reload(); + }); + + if(!engine->start(config)) + { + engine.reset(); + QCoreApplication::exit(1); + return; + } + + log_info("started"); + }); + + int ret = QCoreApplication::exec(); + + // ensure deferred deletes are processed + QCoreApplication::instance()->sendPostedEvents(); + + // deinit here, after all event loop activity has completed + Timer::deinit(); + DeferCall::cleanup(); + + return ret; } }; -HandlerApp::HandlerApp(QObject *parent) : - QObject(parent) -{ - d = new Private(this); -} +HandlerApp::HandlerApp() = default; -HandlerApp::~HandlerApp() -{ - delete d; -} +HandlerApp::~HandlerApp() = default; -void HandlerApp::start() +int HandlerApp::run() { - d->start(); + return Private::run(); } - -#include "handlerapp.moc" diff --git a/src/handler/handlerapp.h b/src/handler/handlerapp.h index 9b617338..a62bb49a 100644 --- a/src/handler/handlerapp.h +++ b/src/handler/handlerapp.h @@ -1,5 +1,6 @@ /* * Copyright (C) 2016 Fanout, Inc. + * Copyright (C) 2025 Fastly, Inc. * * This file is part of Pushpin. * @@ -23,28 +24,18 @@ #ifndef HANDLERAPP_H #define HANDLERAPP_H -#include -#include +#include -using SignalInt = boost::signals2::signal; -using Connection = boost::signals2::scoped_connection; - -class HandlerApp : public QObject +class HandlerApp { - Q_OBJECT - public: - HandlerApp(QObject *parent = 0); + HandlerApp(); ~HandlerApp(); - void start(); - - SignalInt quit; + int run(); private: class Private; - friend class Private; - Private *d; }; #endif diff --git a/src/handler/handlerengine.cpp b/src/handler/handlerengine.cpp index 9e46fffc..8c9ab773 100644 --- a/src/handler/handlerengine.cpp +++ b/src/handler/handlerengine.cpp @@ -1156,8 +1156,6 @@ class AcceptWorker : public Deferred } }; -#define TIMERS_PER_SUBSCRIPTION 1 - class Subscription : public QObject { Q_OBJECT @@ -1340,18 +1338,6 @@ class HandlerEngine::Private : public QObject { config = _config; - // destroy known timers and deinit, so we can reinit below - DeferCall::cleanup(); - Timer::deinit(); - - // includes worst-case subscriptions and update registrations - int timersPerSession = qMax(TIMERS_PER_HTTPSESSION, TIMERS_PER_WSSESSION) + - (config.connectionSubscriptionMax * TIMERS_PER_SUBSCRIPTION) + - TIMERS_PER_UNIQUE_UPDATE_REGISTRATION; - - // enough timers for sessions, plus an extra 100 for misc - Timer::init((config.connectionsMax * timersPerSession) + 100); - publishLimiter->setRate(config.messageRate); publishLimiter->setHwm(config.messageHwm); diff --git a/src/handler/handlerengine.h b/src/handler/handlerengine.h index 3743d954..9ab9a565 100644 --- a/src/handler/handlerengine.h +++ b/src/handler/handlerengine.h @@ -30,6 +30,8 @@ #include #include +#define TIMERS_PER_SUBSCRIPTION 1 + using std::map; using Signal = boost::signals2::signal; using Connection = boost::signals2::scoped_connection; diff --git a/src/handler/handlermain.cpp b/src/handler/handlermain.cpp index 68b17b9a..5ce2273d 100644 --- a/src/handler/handlermain.cpp +++ b/src/handler/handlermain.cpp @@ -22,51 +22,16 @@ */ #include -#include "timer.h" -#include "defercall.h" #include "handlerapp.h" -class HandlerAppMain -{ -public: - HandlerApp *app; - - void start() - { - app = new HandlerApp; - app->quit.connect(boost::bind(&HandlerAppMain::app_quit, this, boost::placeholders::_1)); - app->start(); - } - - void app_quit(int returnCode) - { - delete app; - QCoreApplication::exit(returnCode); - } -}; - extern "C" { int handler_main(int argc, char **argv) { QCoreApplication qapp(argc, argv); - // plenty to kick things off. will reinit after loading config - Timer::init(100); - - HandlerAppMain appMain; - DeferCall deferCall; - deferCall.defer([&] { appMain.start(); }); - int ret = qapp.exec(); - - // ensure deferred deletes are processed - QCoreApplication::instance()->sendPostedEvents(); - - // deinit here, after all event loop activity has completed - DeferCall::cleanup(); - Timer::deinit(); - - return ret; + HandlerApp app; + return app.run(); } } From e4f005d58eb284c7f8276ce8d7915e4633a3dc26 Mon Sep 17 00:00:00 2001 From: Justin Karneges Date: Wed, 26 Feb 2025 13:02:53 -0800 Subject: [PATCH 2/2] fix --- src/handler/handlerenginetest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/handler/handlerenginetest.cpp b/src/handler/handlerenginetest.cpp index 047f1cbf..4b86e7fc 100644 --- a/src/handler/handlerenginetest.cpp +++ b/src/handler/handlerenginetest.cpp @@ -280,6 +280,8 @@ private slots: QDir outDir(qgetenv("OUT_DIR")); QDir workDir(QDir::current().relativeFilePath(outDir.filePath("test-work"))); + Timer::init(100); + wrapper = new Wrapper(this, workDir); wrapper->startHttp(); wrapper->startProxy();