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.

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

defercall: use timers instead of QMetaObject::invokeMethod #48132

Merged
merged 4 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/core/coretests.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ int httpheaders_test(int argc, char **argv);
int jwt_test(int argc, char **argv);
int timer_test(int argc, char **argv);
int eventloop_test(int argc, char **argv);
int defercall_test(int argc, char **argv);

#endif
90 changes: 71 additions & 19 deletions src/core/defercall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,97 @@
*/

#include "defercall.h"
#include <assert.h>

static thread_local DeferCall *g_instance = nullptr;
#include "timer.h"

class DeferCall::Manager
{
public:
Manager()
{
timer_.setSingleShot(true);
timer_.timeout.connect(boost::bind(&Manager::timer_timeout, this));
}

void add(const std::weak_ptr<Call> &c)
{
calls_.push_back(c);

if(!timer_.isActive())
timer_.start(0);
}

void flush()
{
while(!calls_.empty())
process();
}

private:
Timer timer_;
std::list<std::weak_ptr<Call>> calls_;

void process()
{
// process all calls queued so far, but not any that may get queued
// during processing
std::list<std::weak_ptr<Call>> ready;
ready.swap(calls_);

for(auto c : ready)
{
if(auto p = c.lock())
p->handler();
}
}

void timer_timeout()
{
process();

// no need to re-arm the timer. if new calls were queued during
// processing, add() will have taken care of that
}
};

DeferCall::DeferCall() = default;

DeferCall::~DeferCall() = default;

void DeferCall::defer(std::function<void ()> handler)
{
Call c;
c.handler = handler;
std::shared_ptr<Call> c = std::make_shared<Call>();
c->handler = handler;

deferredCalls_.push_back(c);

QMetaObject::invokeMethod(this, "callNext", Qt::QueuedConnection);
if(!manager)
manager = new Manager;

// manager keeps a weak pointer, so we can invalidate pending calls by
// simply deleting them
manager->add(c);
}

DeferCall *DeferCall::global()
{
if(!g_instance)
g_instance = new DeferCall;
if(!instance)
instance = new DeferCall;

return g_instance;
return instance;
}

void DeferCall::cleanup()
{
delete g_instance;
g_instance = nullptr;
}
if(manager)
manager->flush();

void DeferCall::callNext()
{
// there can't be more invokeMethod resolutions than queued calls
assert(!deferredCalls_.empty());

Call c = deferredCalls_.front();
deferredCalls_.pop_front();
delete instance;
instance = nullptr;

c.handler();
delete manager;
manager = nullptr;
}

thread_local DeferCall::Manager *DeferCall::manager = nullptr;
thread_local DeferCall *DeferCall::instance = nullptr;
19 changes: 11 additions & 8 deletions src/core/defercall.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
#ifndef DEFERCALL_H
#define DEFERCALL_H

#include <QObject>
#include <functional>
#include <memory>
#include <list>

// queues calls to be run after returning to the event loop
class DeferCall : public QObject
class DeferCall
{
Q_OBJECT

public:
DeferCall();
~DeferCall();
Expand All @@ -51,17 +51,20 @@ class DeferCall : public QObject
global()->defer([=] { delete p; });
}

private slots:
void callNext();

private:
class Call
{
public:
std::function<void ()> handler;
};

std::list<Call> deferredCalls_;
class Manager;
friend class Manager;

std::list<std::shared_ptr<Call>> deferredCalls_;

static thread_local Manager *manager;
static thread_local DeferCall *instance;
};

#endif
146 changes: 146 additions & 0 deletions src/core/defercalltest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/*
* Copyright (C) 2025 Fastly, Inc.
*
* This file is part of Pushpin.
*
* $FANOUT_BEGIN_LICENSE:APACHE2$
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* $FANOUT_END_LICENSE$
*/

#include <QtTest/QtTest>
#include "timer.h"
#include "defercall.h"
#include "eventloop.h"

class DeferCallTest : public QObject
{
Q_OBJECT

private:
// loop_advance should process enough events to cause the calls to run,
// without sleeping, in order to prove the calls are run immediately
int runDeferCall(std::function<void ()> loop_advance)
{
int count = 0;

DeferCall::global()->defer([&] {
++count;

DeferCall::global()->defer([&] {
++count;
});
});

loop_advance();

return count;
}

private slots:
void deferCall()
{
EventLoop loop(1);

int count = runDeferCall([&] {
// run the first call and queue the second
loop.step();

// run the second
loop.step();
});

QCOMPARE(count, 2);

DeferCall::cleanup();
}

void deferCallQt()
{
Timer::init(1);

int count = runDeferCall([] {
// the underlying timer's qt-based implementation will process
// both timeouts during a single timer processing pass.
// therefore, both calls should run within a single event loop
// pass
QCoreApplication::processEvents(QEventLoop::AllEvents);
});

QCOMPARE(count, 2);

DeferCall::cleanup();
Timer::deinit();
}

void retract()
{
Timer::init(1);

bool called = false;

{
DeferCall deferCall;

deferCall.defer([&] {
called = true;
});
}

DeferCall::cleanup();
QVERIFY(!called);

Timer::deinit();
}

void managerCleanup()
{
Timer::init(1);

int count = 0;

DeferCall::global()->defer([&] {
++count;

DeferCall::global()->defer([&] {
++count;
});
});

// cleanup should process deferred calls queued so far as well as
// those queued during processing
DeferCall::cleanup();
QCOMPARE(count, 2);

Timer::deinit();
}
};

namespace {
namespace Main {
QTEST_MAIN(DeferCallTest)
}
}

extern "C" {

int defercall_test(int argc, char **argv)
{
return Main::main(argc, argv);
}

}

#include "defercalltest.moc"
5 changes: 0 additions & 5 deletions src/core/eventlooptest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ class EventLoopTest : public QObject
Q_OBJECT

private slots:
void cleanupTestCase()
{
DeferCall::cleanup();
}

void socketNotifier()
{
EventLoop loop(1);
Expand Down
10 changes: 10 additions & 0 deletions src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ mod tests {
unsafe { call_c_main(ffi::timer_test, args) as u8 }
}

fn defercall_test(args: &[&OsStr]) -> u8 {
// SAFETY: safe to call
unsafe { call_c_main(ffi::defercall_test, args) as u8 }
}

fn eventloop_test(args: &[&OsStr]) -> u8 {
// SAFETY: safe to call
unsafe { call_c_main(ffi::eventloop_test, args) as u8 }
Expand All @@ -140,6 +145,11 @@ mod tests {
assert!(qtest::run(timer_test));
}

#[test]
fn defercall() {
assert!(qtest::run(defercall_test));
}

#[test]
fn eventloop() {
assert!(qtest::run(eventloop_test));
Expand Down
1 change: 1 addition & 0 deletions src/core/tests.pri
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ SOURCES += \
$$PWD/httpheaderstest.cpp \
$$PWD/jwttest.cpp \
$$PWD/timertest.cpp \
$$PWD/defercalltest.cpp \
$$PWD/eventlooptest.cpp
3 changes: 2 additions & 1 deletion src/handler/filtertest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,15 @@ private slots:

void cleanupTestCase()
{
limiter.reset();
zhttpOut.reset();
filterServer.reset();

// ensure deferred deletes are processed
QCoreApplication::instance()->sendPostedEvents();

Timer::deinit();
DeferCall::cleanup();
Timer::deinit();
}

void messageFilters()
Expand Down
4 changes: 4 additions & 0 deletions src/handler/handlerengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,10 @@ 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) +
Expand Down
2 changes: 1 addition & 1 deletion src/handler/handlerenginetest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ private slots:
// ensure deferred deletes are processed
QCoreApplication::instance()->sendPostedEvents();

Timer::deinit();
DeferCall::cleanup();
Timer::deinit();
}

void acceptNoHold()
Expand Down
Loading
Loading