1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QTimer: fix regression on singleShot-invoking non-mormalized SLOT()s

Browse Source 
The was introduced with the rewrite of QMetaObject::invokeMethod() in
commit 0f76e55bc440a70f5d9530a192c9ce6334a8f06, because we have an
optimization for zero timers to avoid creating a temporary
QSingleShotTimer object. The old implementation did attempt to normalize
the target slot name, but did so because it looked metamethods up using
QMetaObject::indexOfMethod:

    int idx = meta->indexOfMethod(sig.constData());
    if (idx < 0) {
        QByteArray norm =
QMetaObject::normalizedSignature(sig.constData());
        idx = meta->indexOfMethod(norm.constData());
    }

The new implementation does not use this method so it didn't need to
attempt to normalize.

I am fixing this only in QTimer and not in QMetaObject::invokeMethodImpl
(even though it is trivial to do so) because I don't believe spaces in a
pure string to invokeMethod were ever expected to work:

    QMetaObject::invokeMethod(obj, "slotName ", Qt::QueuedConnection);

The Q_ARG and Q_RETURN_ARG (for code not recompiled) still does
normalization inside QMetaType::fromName().

Fixes: QTBUG-116060
Change-Id: I964c2b1e6b834feb9710fffd177cac60c83ef413
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 4368179c37cae05d16109e319fc1c77490754c7d)
This commit is contained in:
Thiago Macieira 2023-08-18 21:51:31 -06:00 committed by Volker Hilsheimer
parent 0cc4424414
commit 7efe78bb5e
2 changed files with 144 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/corelib/kernel/qtimer.cpp
View File

@ -440,7 +440,8 @@ void QTimer::singleShot(int msec, Qt::TimerType timerType, const QObject *receiv
return;
}
QByteArray methodName(member+1, bracketPosition - 1 - member); // extract method name
QMetaObject::invokeMethod(const_cast<QObject *>(receiver), methodName.constData(), Qt::QueuedConnection);
QMetaObject::invokeMethod(const_cast<QObject *>(receiver), methodName.trimmed().constData(),
Qt::QueuedConnection);
return;
}
(void) new QSingleShotTimer(msec, timerType, receiver, member);

142
tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp
View File

@ -32,6 +32,12 @@ private slots:
void zeroTimer();
void singleShotTimeout();
void timeout();
void singleShotNormalizes_data();
void singleShotNormalizes();
void sequentialTimers_data();
void sequentialTimers();
void singleShotSequentialTimers_data();
void singleShotSequentialTimers();
void remainingTime();
void remainingTimeInitial_data();
void remainingTimeInitial();
@ -120,6 +126,142 @@ void tst_QTimer::timeout()
QTRY_VERIFY_WITH_TIMEOUT(timeoutSpy.size() > oldCount, TIMEOUT_TIMEOUT);
}
void tst_QTimer::singleShotNormalizes_data()
{
QTest::addColumn<QByteArray>("slotName");
QTest::newRow("normalized") << QByteArray(SLOT(exitLoop()));
QTest::newRow("space-before") << QByteArray(SLOT( exitLoop()));
QTest::newRow("space-after") << QByteArray(SLOT(exitLoop ()));
QTest::newRow("space-around") << QByteArray(SLOT( exitLoop ()));
QTest::newRow("spaces-before") << QByteArray(SLOT( exitLoop()));
QTest::newRow("spaces-after") << QByteArray(SLOT(exitLoop ()));
QTest::newRow("spaces-around") << QByteArray(SLOT( exitLoop ()));
QTest::newRow("space-in-parens") << QByteArray(SLOT(exitLoop( )));
QTest::newRow("spaces-in-parens") << QByteArray(SLOT(exitLoop( )));
QTest::newRow("space-after-parens") << QByteArray(SLOT(exitLoop() ));
QTest::newRow("spaces-after-parens") << QByteArray(SLOT(exitLoop() ));
}
void tst_QTimer::singleShotNormalizes()
{
using namespace std::chrono_literals;
static constexpr auto TestTimeout = 250ms;
QFETCH(QByteArray, slotName);
QEventLoop loop;
// control test: regular connection
{
QTimer timer;
QVERIFY(QObject::connect(&timer, SIGNAL(timeout()), &QTestEventLoop::instance(), slotName));
timer.setSingleShot(true);
timer.start(1);
QTestEventLoop::instance().enterLoop(TestTimeout);
QVERIFY(!QTestEventLoop::instance().timeout());
}
// non-zero time
QTimer::singleShot(1, &QTestEventLoop::instance(), slotName);
QTestEventLoop::instance().enterLoop(TestTimeout);
QVERIFY(!QTestEventLoop::instance().timeout());
QTimer::singleShot(1ms, &QTestEventLoop::instance(), slotName);
QTestEventLoop::instance().enterLoop(TestTimeout);
QVERIFY(!QTestEventLoop::instance().timeout());
// zero time
QTimer::singleShot(0, &QTestEventLoop::instance(), slotName);
QTestEventLoop::instance().enterLoop(TestTimeout);
QVERIFY(!QTestEventLoop::instance().timeout());
QTimer::singleShot(0ms, &QTestEventLoop::instance(), slotName);
QTestEventLoop::instance().enterLoop(TestTimeout);
QVERIFY(!QTestEventLoop::instance().timeout());
}
void tst_QTimer::sequentialTimers_data()
{
#ifdef Q_OS_WIN
QSKIP("The API used by QEventDispatcherWin32 doesn't respect the order");
#endif
QTest::addColumn<QList<int>>("timeouts");
auto addRow = [](const QList<int> &l) {
QByteArray name;
int last = -1;
for (int i = 0; i < l.size(); ++i) {
Q_ASSERT_X(l[i] >= last, "tst_QTimer", "input list must be sorted");
name += QByteArray::number(l[i]) + ',';
}
name.chop(1);
QTest::addRow("%s", name.constData()) << l;
};
// PreciseTimers
addRow({0, 0, 0, 0, 0, 0});
addRow({0, 1, 2});
addRow({1, 1, 1, 2, 2, 2, 2});
addRow({1, 2, 3});
addRow({19, 19, 19});
// CoarseTimer for setInterval
addRow({20, 20, 20, 20, 20});
addRow({25, 25, 25, 25, 25, 25, 50});
}
void tst_QTimer::sequentialTimers()
{
QFETCH(const QList<int>, timeouts);
QByteArray result, expected;
std::vector<std::unique_ptr<QTimer>> timers;
expected.resize(timeouts.size());
result.reserve(timeouts.size());
timers.reserve(timeouts.size());
for (int i = 0; i < timeouts.size(); ++i) {
auto timer = std::make_unique<QTimer>();
timer->setSingleShot(true);
timer->setInterval(timeouts[i]);
char c = 'A' + i;
expected[i] = c;
QObject::connect(timer.get(), &QTimer::timeout, this, [&result, c = c]() {
result.append(c);
});
timers.push_back(std::move(timer));
}
// start the timers
for (auto &timer : timers)
timer->start();
QTestEventLoop::instance().enterLoopMSecs(timeouts.last() * 2 + 10);
QCOMPARE(result, expected);
}
void tst_QTimer::singleShotSequentialTimers_data()
{
sequentialTimers_data();
}
void tst_QTimer::singleShotSequentialTimers()
{
QFETCH(const QList<int>, timeouts);
QByteArray result, expected;
expected.resize(timeouts.size());
result.reserve(timeouts.size());
for (int i = 0; i < timeouts.size(); ++i) {
char c = 'A' + i;
expected[i] = c;
QTimer::singleShot(timeouts[i], this, [&result, c = c]() {
result.append(c);
});
}
QTestEventLoop::instance().enterLoopMSecs(timeouts.last() * 2 + 10);
QCOMPARE(result, expected);
}
void tst_QTimer::remainingTime()
{
QTimer tested;