IPC: fix cleaning up after tst_QSharedMemory tests

If a previous test in the same process or previously leaked a shared
memory or semaphore handle, tests could fail and cascade down. They
could also interfere with one another. So prevent this issue by
assigning a monotonically-increasing identifier per test function or row
tested and improving how we clean up those we did create.

This shows we need the API to explicitly clean up.

Change-Id: I12a088d1ae424825abd3fffd171db61d6b68a411
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Thiago Macieira 2022-10-13 12:01:14 -07:00
parent 6bc3a89c90
commit 4141c5e221
2 changed files with 34 additions and 12 deletions

View File

@ -20,12 +20,13 @@
#include "private/qtcore-config_p.h" #include "private/qtcore-config_p.h"
#define EXISTING_SHARE "existing"
#define EXISTING_SIZE 1024 #define EXISTING_SIZE 1024
Q_DECLARE_METATYPE(QSharedMemory::SharedMemoryError) Q_DECLARE_METATYPE(QSharedMemory::SharedMemoryError)
Q_DECLARE_METATYPE(QSharedMemory::AccessMode) Q_DECLARE_METATYPE(QSharedMemory::AccessMode)
using namespace Qt::StringLiterals;
class tst_QSharedMemory : public QObject class tst_QSharedMemory : public QObject
{ {
Q_OBJECT Q_OBJECT
@ -81,10 +82,18 @@ private slots:
protected: protected:
void remove(const QNativeIpcKey &key); void remove(const QNativeIpcKey &key);
QString mangleKey(QStringView key)
{
if (key.isEmpty())
return key.toString();
return u"tstshm_%1-%2_%3"_s.arg(QCoreApplication::applicationPid())
.arg(seq).arg(key);
}
QNativeIpcKey platformSafeKey(const QString &key) QNativeIpcKey platformSafeKey(const QString &key)
{ {
QNativeIpcKey::Type keyType = QNativeIpcKey::DefaultTypeForOs; QNativeIpcKey::Type keyType = QNativeIpcKey::DefaultTypeForOs;
return QSharedMemory::platformSafeKey(key, keyType); return QSharedMemory::platformSafeKey(mangleKey(key), keyType);
} }
QNativeIpcKey rememberKey(const QString &key) QNativeIpcKey rememberKey(const QString &key)
@ -100,6 +109,7 @@ protected:
QList<QNativeIpcKey> keys; QList<QNativeIpcKey> keys;
QList<QSharedMemory*> jail; QList<QSharedMemory*> jail;
QSharedMemory *existingSharedMemory; QSharedMemory *existingSharedMemory;
int seq = 0;
private: private:
const QString m_helperBinary; const QString m_helperBinary;
@ -117,7 +127,7 @@ tst_QSharedMemory::~tst_QSharedMemory()
void tst_QSharedMemory::init() void tst_QSharedMemory::init()
{ {
QNativeIpcKey key = platformSafeKey(EXISTING_SHARE); QNativeIpcKey key = platformSafeKey("existing");
existingSharedMemory = new QSharedMemory(key); existingSharedMemory = new QSharedMemory(key);
if (!existingSharedMemory->create(EXISTING_SIZE)) { if (!existingSharedMemory->create(EXISTING_SIZE)) {
QCOMPARE(existingSharedMemory->error(), QSharedMemory::AlreadyExists); QCOMPARE(existingSharedMemory->error(), QSharedMemory::AlreadyExists);
@ -138,9 +148,10 @@ void tst_QSharedMemory::cleanup()
// qWarning() << "test cleanup: remove failed:" << keys.at(i) << sm.error() << sm.errorString(); // qWarning() << "test cleanup: remove failed:" << keys.at(i) << sm.error() << sm.errorString();
sm.attach(); sm.attach();
sm.detach(); sm.detach();
}
remove(keys.at(i)); remove(keys.at(i));
} }
} ++seq;
} }
#if QT_CONFIG(posix_shm) #if QT_CONFIG(posix_shm)
@ -320,7 +331,7 @@ void tst_QSharedMemory::create_data()
<< false << QSharedMemory::InvalidSize; << false << QSharedMemory::InvalidSize;
QTest::newRow("nor size") << QString("norsize") << 1024 QTest::newRow("nor size") << QString("norsize") << 1024
<< true << QSharedMemory::NoError; << true << QSharedMemory::NoError;
QTest::newRow("already exists") << QString(EXISTING_SHARE) << EXISTING_SIZE QTest::newRow("existing") << QString("existing") << EXISTING_SIZE
<< false << QSharedMemory::AlreadyExists; << false << QSharedMemory::AlreadyExists;
} }
@ -359,7 +370,7 @@ void tst_QSharedMemory::attach_data()
QTest::newRow("null") << QString() << false << QSharedMemory::KeyError; QTest::newRow("null") << QString() << false << QSharedMemory::KeyError;
QTest::newRow("doesntexists") << QString("doesntexist") << false << QSharedMemory::NotFound; QTest::newRow("doesntexists") << QString("doesntexist") << false << QSharedMemory::NotFound;
QTest::newRow(EXISTING_SHARE) << QString(EXISTING_SHARE) << true << QSharedMemory::NoError; QTest::newRow("existing") << QString("existing") << true << QSharedMemory::NoError;
} }
/*! /*!

View File

@ -11,9 +11,10 @@
#include <QtCore/QSystemSemaphore> #include <QtCore/QSystemSemaphore>
#include <QtCore/QTemporaryDir> #include <QtCore/QTemporaryDir>
#define EXISTING_SHARE "existing"
#define HELPERWAITTIME 10000 #define HELPERWAITTIME 10000
using namespace Qt::StringLiterals;
class tst_QSystemSemaphore : public QObject class tst_QSystemSemaphore : public QObject
{ {
Q_OBJECT Q_OBJECT
@ -21,10 +22,18 @@ class tst_QSystemSemaphore : public QObject
public: public:
tst_QSystemSemaphore(); tst_QSystemSemaphore();
QString mangleKey(QStringView key)
{
if (key.isEmpty())
return key.toString();
return u"tstsyssem_%1-%2_%3"_s.arg(QCoreApplication::applicationPid())
.arg(seq).arg(key);
}
QNativeIpcKey platformSafeKey(const QString &key) QNativeIpcKey platformSafeKey(const QString &key)
{ {
QNativeIpcKey::Type keyType = QNativeIpcKey::DefaultTypeForOs; QNativeIpcKey::Type keyType = QNativeIpcKey::DefaultTypeForOs;
return QSystemSemaphore::platformSafeKey(key, keyType); return QSystemSemaphore::platformSafeKey(mangleKey(key), keyType);
} }
public Q_SLOTS: public Q_SLOTS:
@ -50,6 +59,7 @@ private slots:
void initialValue(); void initialValue();
private: private:
int seq = 0;
QSystemSemaphore *existingLock; QSystemSemaphore *existingLock;
const QString m_helperBinary; const QString m_helperBinary;
@ -62,13 +72,14 @@ tst_QSystemSemaphore::tst_QSystemSemaphore()
void tst_QSystemSemaphore::init() void tst_QSystemSemaphore::init()
{ {
QNativeIpcKey key = platformSafeKey(EXISTING_SHARE); QNativeIpcKey key = platformSafeKey("existing");
existingLock = new QSystemSemaphore(key, 1, QSystemSemaphore::Create); existingLock = new QSystemSemaphore(key, 1, QSystemSemaphore::Create);
} }
void tst_QSystemSemaphore::cleanup() void tst_QSystemSemaphore::cleanup()
{ {
delete existingLock; delete existingLock;
++seq;
} }
void tst_QSystemSemaphore::nativeKey_data() void tst_QSystemSemaphore::nativeKey_data()
@ -123,7 +134,7 @@ QT_WARNING_POP
void tst_QSystemSemaphore::basicacquire() void tst_QSystemSemaphore::basicacquire()
{ {
QNativeIpcKey key = platformSafeKey("QSystemSemaphore_basicacquire"); QNativeIpcKey key = platformSafeKey("basicacquire");
QSystemSemaphore sem(key, 1, QSystemSemaphore::Create); QSystemSemaphore sem(key, 1, QSystemSemaphore::Create);
QVERIFY(sem.acquire()); QVERIFY(sem.acquire());
QCOMPARE(sem.error(), QSystemSemaphore::NoError); QCOMPARE(sem.error(), QSystemSemaphore::NoError);
@ -134,7 +145,7 @@ void tst_QSystemSemaphore::basicacquire()
void tst_QSystemSemaphore::complexacquire() void tst_QSystemSemaphore::complexacquire()
{ {
QNativeIpcKey key = platformSafeKey("QSystemSemaphore_complexacquire"); QNativeIpcKey key = platformSafeKey("complexacquire");
QSystemSemaphore sem(key, 2, QSystemSemaphore::Create); QSystemSemaphore sem(key, 2, QSystemSemaphore::Create);
QVERIFY(sem.acquire()); QVERIFY(sem.acquire());
QCOMPARE(sem.error(), QSystemSemaphore::NoError); QCOMPARE(sem.error(), QSystemSemaphore::NoError);
@ -157,7 +168,7 @@ void tst_QSystemSemaphore::complexacquire()
void tst_QSystemSemaphore::release() void tst_QSystemSemaphore::release()
{ {
QNativeIpcKey key = platformSafeKey("QSystemSemaphore_release"); QNativeIpcKey key = platformSafeKey("release");
QSystemSemaphore sem(key, 0, QSystemSemaphore::Create); QSystemSemaphore sem(key, 0, QSystemSemaphore::Create);
QVERIFY(sem.release()); QVERIFY(sem.release());
QCOMPARE(sem.error(), QSystemSemaphore::NoError); QCOMPARE(sem.error(), QSystemSemaphore::NoError);