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

Event queue compressing proxy #4566

Merged
merged 12 commits into from
Dec 20, 2021
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Added safety limit of recursions to prevent potential stack overflow
Improved testcase
JoergAtGithub committed Dec 14, 2021
commit 599762554f1ed87836d99d5c251eadc44fabac9a
11 changes: 11 additions & 0 deletions src/control/controlcompressingproxy.cpp
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
// Event queue compressing proxy
CompressingProxy::CompressingProxy(QObject* parent)
: QObject(parent) {
m_recursionCounter = 0;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
}

// This function is called recursive by QCoreApplication::sendPostedEvents, until no more events are in the queue.
@@ -15,10 +16,20 @@ CompressingProxy::CompressingProxy(QObject* parent)
stateOfprocessQueuedEvent CompressingProxy::processQueuedEvents() {
m_recursiveSearchForLastEventOngoing = true;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

if (m_recursionCounter >= kMaxNumOfRecursions) {
// To many events in queue -> Delete all unprocessed events in queue, to prevent stack overflow
QCoreApplication::removePostedEvents(this, QEvent::MetaCall);
// We just return, without resetting m_recursiveSearchForLastEventOngoing,
// this ensures that the event found in the last iteration will be send
return stateOfprocessQueuedEvent::no_event;
}

m_recursionCounter++;
// sendPostedEvents recursive executes slotValueChanged until no more events for this slot are in the queue
// Each call of QCoreApplication::sendPostedEvents triggers the processing of the next event in the queue,
// by sending the signal to execute slotValueChanged again
QCoreApplication::sendPostedEvents(this, QEvent::MetaCall);
m_recursionCounter--;

// Execution continues here, when last event for this slot is processed

15 changes: 11 additions & 4 deletions src/control/controlcompressingproxy.h
Original file line number Diff line number Diff line change
@@ -3,15 +3,23 @@
#include <QApplication>
#include <QMetaObject>

enum class stateOfprocessQueuedEvent { last_event,
outdated_event };
namespace {
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
constexpr int kMaxNumOfRecursions = 128;
}

enum class stateOfprocessQueuedEvent {
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
last_event,
outdated_event,
no_event
};

class CompressingProxy : public QObject {
Q_OBJECT
private:
stateOfprocessQueuedEvent processQueuedEvents();

bool m_recursiveSearchForLastEventOngoing;
int m_recursionCounter;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

public slots:
void slotValueChanged(double value, QObject* obj);
@@ -20,7 +28,6 @@ class CompressingProxy : public QObject {
void signalValueChanged(double, QObject*);

public:
// No default constructor, since the proxy must be a child of the
// target object.
// No default constructor, since the proxy must be a child of the object with the Qt event queue
explicit CompressingProxy(QObject* parent);
};
61 changes: 60 additions & 1 deletion src/test/controlobjectscripttest.cpp
Original file line number Diff line number Diff line change
@@ -52,6 +52,12 @@ class ControlObjectScriptTest : public MixxxTest {
conn2.callback = "mock_callback2";
conn2.id = QUuid::createUuid();

conn3.key = ck2;
conn3.engineJSProxy = nullptr;
conn3.controllerEngine = nullptr;
conn3.callback = "mock_callback3";
conn3.id = QUuid::createUuid();

coScript1->addScriptConnection(conn1);
coScript2->addScriptConnection(conn2);
}
@@ -68,7 +74,7 @@ class ControlObjectScriptTest : public MixxxTest {
std::unique_ptr<MockControlObjectScript> coScript1;
std::unique_ptr<MockControlObjectScript> coScript2;

ScriptConnection conn1, conn2;
ScriptConnection conn1, conn2, conn3;
};

TEST_F(ControlObjectScriptTest, CompressingProxyCompareCount1) {
@@ -144,4 +150,57 @@ TEST_F(ControlObjectScriptTest, CompressingProxyCompareValueMulti) {
application()->processEvents();
}

TEST_F(ControlObjectScriptTest, CompressingProxyMultiConnection) {
// Check that slotValueChanged callback is called 1 time if multiple connections exist forthe same slot
EXPECT_CALL(*coScript1, slotValueChanged(32.0, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript2, slotValueChanged(33.0, _))
.Times(1)
.WillOnce(Return());

coScript2->addScriptConnection(conn3);
co1->set(30.0);
co2->set(31.0);
co1->set(32.0);
co2->set(33.0);
application()->processEvents();

coScript2->removeScriptConnection(conn3);
}

TEST_F(ControlObjectScriptTest, CompressingProxyManyEvents) {
// Check maximum number of recursions
EXPECT_CALL(*coScript1, slotValueChanged(kMaxNumOfRecursions, _))
.Times(1)
.WillOnce(Return());
EXPECT_CALL(*coScript2, slotValueChanged(42.0, _))
.Times(1)
.WillOnce(Return());

// Add more 3x more events than the recursion limit of the compressing proxy
for (int i = 1; i <= kMaxNumOfRecursions * 3; i++) {
co1->set(i);
}

// Event queue of second slot
co2->set(41);
co2->set(42);

// Event queue should be cleared
application()->processEvents();
EXPECT_CALL(*coScript1, slotValueChanged(_, _))
.Times(0)
.WillOnce(Return());
application()->processEvents();

// Verify that compressing proxy works again after clearing event queue
EXPECT_CALL(*coScript1, slotValueChanged(2, _))
.Times(1)
.WillOnce(Return());
co1->set(1);
co1->set(2);
application()->processEvents();
}

} // namespace