Skip to content

Commit

Permalink
Added safety limit of recursions to prevent potential stack overflow
Browse files Browse the repository at this point in the history
Improved testcase
  • Loading branch information
JoergAtGithub committed Dec 14, 2021
1 parent cd4d6d4 commit 0dcbed4
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 5 deletions.
11 changes: 11 additions & 0 deletions src/control/controlcompressingproxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// Event queue compressing proxy
CompressingProxy::CompressingProxy(QObject* parent)
: QObject(parent) {
m_recursionCounter = 0;
}

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

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 reseting 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

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

enum class stateOfprocessQueuedEvent { last_event,
outdated_event };
namespace {
constexpr int kMaxNumOfRecursions = 128;
}

enum class stateOfprocessQueuedEvent {
last_event,
outdated_event,
no_event
};

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

bool m_recursiveSearchForLastEventOngoing;
int m_recursionCounter;

public slots:
void slotValueChanged(double value, QObject* obj);
Expand All @@ -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
Expand Up @@ -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);
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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 compresing 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

0 comments on commit 0dcbed4

Please sign in to comment.