From 32a06e983073080d939b4742996a3dc9cd75bb83 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 12 Oct 2022 18:55:23 -0700 Subject: [PATCH] IPC: add support for multiple backends to QSystemSemaphore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simultaneously. Change-Id: If4c23ea3719947d790d4fffd17152a29e6c217d3 Reviewed-by: Tor Arne Vestbø Reviewed-by: Fabian Kosmale --- src/corelib/CMakeLists.txt | 6 ++ src/corelib/ipc/qsystemsemaphore.cpp | 47 +++++++++-- src/corelib/ipc/qsystemsemaphore_p.h | 46 +++++++--- src/corelib/ipc/qsystemsemaphore_posix.cpp | 9 ++ src/corelib/ipc/qsystemsemaphore_systemv.cpp | 13 +++ src/corelib/ipc/qtipccommon_p.h | 55 ++++++++++++ tests/auto/corelib/ipc/ipctestcommon.h | 83 +++++++++++++++++++ .../ipc/qnativeipckey/tst_qnativeipckey.cpp | 33 +------- .../qsystemsemaphore/tst_qsystemsemaphore.cpp | 46 +++++++++- 9 files changed, 287 insertions(+), 51 deletions(-) create mode 100644 tests/auto/corelib/ipc/ipctestcommon.h diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index ecf237f71a7..60bf9381717 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -1139,7 +1139,13 @@ qt_internal_extend_target(Core CONDITION UNIX SOURCES ipc/qsharedmemory_posix.cpp ipc/qsharedmemory_systemv.cpp +) +qt_internal_extend_target(Core CONDITION QT_FEATURE_posix_sem + SOURCES ipc/qsystemsemaphore_posix.cpp +) +qt_internal_extend_target(Core CONDITION QT_FEATURE_sysv_sem + SOURCES ipc/qsystemsemaphore_systemv.cpp ) diff --git a/src/corelib/ipc/qsystemsemaphore.cpp b/src/corelib/ipc/qsystemsemaphore.cpp index f7a9946ecd4..6607222580d 100644 --- a/src/corelib/ipc/qsystemsemaphore.cpp +++ b/src/corelib/ipc/qsystemsemaphore.cpp @@ -3,14 +3,33 @@ #include "qsystemsemaphore.h" #include "qsystemsemaphore_p.h" -#include + +#if QT_CONFIG(systemsemaphore) +#include QT_BEGIN_NAMESPACE using namespace QtIpcCommon; using namespace Qt::StringLiterals; -#if QT_CONFIG(systemsemaphore) +#if __cplusplus >= 202002L +using std::construct_at; +#else +template static void construct_at(T *ptr) +{ + new (ptr) T; +} +#endif + +inline void QSystemSemaphorePrivate::constructBackend() +{ + visit([](auto p) { construct_at(p); }); +} + +inline void QSystemSemaphorePrivate::destructBackend() +{ + visit([](auto p) { std::destroy_at(p); }); +} /*! \class QSystemSemaphore @@ -113,7 +132,7 @@ QSystemSemaphore::QSystemSemaphore(const QString &key, int initialValue, AccessM \sa acquire(), key() */ QSystemSemaphore::QSystemSemaphore(const QNativeIpcKey &key, int initialValue, AccessMode mode) - : d(new QSystemSemaphorePrivate) + : d(new QSystemSemaphorePrivate(key.type())) { setNativeKey(key, initialValue, mode); } @@ -187,7 +206,15 @@ void QSystemSemaphore::setNativeKey(const QNativeIpcKey &key, int initialValue, d->clearError(); d->cleanHandle(); - d->nativeKey = key; + if (key.type() == d->nativeKey.type()) { + // we can reuse the backend + d->nativeKey = key; + } else { + // we must recreate the backend + d->destructBackend(); + d->nativeKey = key; + d->constructBackend(); + } d->initialValue = initialValue; d->handle(mode); @@ -373,7 +400,13 @@ void QSystemSemaphorePrivate::setUnixErrorString(QLatin1StringView function) bool QSystemSemaphore::isKeyTypeSupported(QNativeIpcKey::Type type) { - return QSystemSemaphorePrivate::DefaultBackend::supports(type); + if (!isIpcSupported(IpcType::SystemSemaphore, type)) + return false; + using Variant = decltype(QSystemSemaphorePrivate::backend); + return Variant::staticVisit(type, [](auto ptr) { + using Impl = std::decay_t; + return Impl::runtimeSupportCheck(); + }); } QNativeIpcKey QSystemSemaphore::platformSafeKey(const QString &key, QNativeIpcKey::Type type) @@ -386,8 +419,8 @@ QNativeIpcKey QSystemSemaphore::legacyNativeKey(const QString &key, QNativeIpcKe return { legacyPlatformSafeKey(key, IpcType::SystemSemaphore, type), type }; } -#endif // QT_CONFIG(systemsemaphore) - QT_END_NAMESPACE #include "moc_qsystemsemaphore.cpp" + +#endif // QT_CONFIG(systemsemaphore) diff --git a/src/corelib/ipc/qsystemsemaphore_p.h b/src/corelib/ipc/qsystemsemaphore_p.h index 45d27f1f518..bdeca205dc7 100644 --- a/src/corelib/ipc/qsystemsemaphore_p.h +++ b/src/corelib/ipc/qsystemsemaphore_p.h @@ -42,8 +42,10 @@ class QSystemSemaphorePrivate; struct QSystemSemaphorePosix { + static constexpr bool Enabled = QT_CONFIG(posix_sem); static bool supports(QNativeIpcKey::Type type) { return type == QNativeIpcKey::Type::PosixRealtime; } + static bool runtimeSupportCheck(); bool handle(QSystemSemaphorePrivate *self, QSystemSemaphore::AccessMode mode); void cleanHandle(QSystemSemaphorePrivate *self); @@ -55,8 +57,10 @@ struct QSystemSemaphorePosix struct QSystemSemaphoreSystemV { + static constexpr bool Enabled = QT_CONFIG(sysv_sem); static bool supports(QNativeIpcKey::Type type) { return quint16(type) <= 0xff; } + static bool runtimeSupportCheck(); #if QT_CONFIG(sysv_sem) key_t handle(QSystemSemaphorePrivate *self, QSystemSemaphore::AccessMode mode); @@ -73,10 +77,16 @@ struct QSystemSemaphoreSystemV struct QSystemSemaphoreWin32 { +#ifdef Q_OS_WIN32 + static constexpr bool Enabled = true; +#else + static constexpr bool Enabled = false; +#endif static bool supports(QNativeIpcKey::Type type) { return type == QNativeIpcKey::Type::Windows; } + static bool runtimeSupportCheck() { return Enabled; } -//#ifdef Q_OS_WIN32 but there's nothing Windows-specific in the header + // we can declare the members without the #if Qt::HANDLE handle(QSystemSemaphorePrivate *self, QSystemSemaphore::AccessMode mode); void cleanHandle(QSystemSemaphorePrivate *self); bool modifySemaphore(QSystemSemaphorePrivate *self, int count); @@ -87,6 +97,10 @@ struct QSystemSemaphoreWin32 class QSystemSemaphorePrivate { public: + QSystemSemaphorePrivate(QNativeIpcKey::Type type) : nativeKey(type) + { constructBackend(); } + ~QSystemSemaphorePrivate() { destructBackend(); } + void setWindowsErrorString(QLatin1StringView function); // Windows only void setUnixErrorString(QLatin1StringView function); inline void setError(QSystemSemaphore::SystemSemaphoreError e, const QString &message) @@ -99,26 +113,34 @@ public: int initialValue; QSystemSemaphore::SystemSemaphoreError error = QSystemSemaphore::NoError; -#if defined(Q_OS_WIN) - using DefaultBackend = QSystemSemaphoreWin32; -#elif defined(QT_POSIX_IPC) - using DefaultBackend = QSystemSemaphorePosix; -#else - using DefaultBackend = QSystemSemaphoreSystemV; -#endif - DefaultBackend backend; + union Backend { + Backend() {} + ~Backend() {} + QSystemSemaphorePosix posix; + QSystemSemaphoreSystemV sysv; + QSystemSemaphoreWin32 win32; + }; + QtIpcCommon::IpcStorageVariant<&Backend::posix, &Backend::sysv, &Backend::win32> backend; + + void constructBackend(); + void destructBackend(); + + template auto visit(const Lambda &lambda) + { + return backend.visit(nativeKey.type(), lambda); + } void handle(QSystemSemaphore::AccessMode mode) { - backend.handle(this, mode); + visit([=](auto p) { p->handle(this, mode); }); } void cleanHandle() { - backend.cleanHandle(this); + visit([=](auto p) { p->cleanHandle(this); }); } bool modifySemaphore(int count) { - return backend.modifySemaphore(this, count); + return visit([=](auto p) { return p->modifySemaphore(this, count); }); } QString legacyKey; // deprecated diff --git a/src/corelib/ipc/qsystemsemaphore_posix.cpp b/src/corelib/ipc/qsystemsemaphore_posix.cpp index a5765c5da89..b290e14de4e 100644 --- a/src/corelib/ipc/qsystemsemaphore_posix.cpp +++ b/src/corelib/ipc/qsystemsemaphore_posix.cpp @@ -35,6 +35,15 @@ QT_BEGIN_NAMESPACE using namespace Qt::StringLiterals; +bool QSystemSemaphorePosix::runtimeSupportCheck() +{ + static const bool result = []() { + sem_open("/", 0, 0, 0); // this WILL fail + return errno != ENOSYS; + }(); + return result; +} + bool QSystemSemaphorePosix::handle(QSystemSemaphorePrivate *self, QSystemSemaphore::AccessMode mode) { if (semaphore != SEM_FAILED) diff --git a/src/corelib/ipc/qsystemsemaphore_systemv.cpp b/src/corelib/ipc/qsystemsemaphore_systemv.cpp index 2078caea9ac..3d73a622623 100644 --- a/src/corelib/ipc/qsystemsemaphore_systemv.cpp +++ b/src/corelib/ipc/qsystemsemaphore_systemv.cpp @@ -33,6 +33,19 @@ QT_BEGIN_NAMESPACE using namespace Qt::StringLiterals; +bool QSystemSemaphoreSystemV::runtimeSupportCheck() +{ +#if defined(Q_OS_DARWIN) + if (qt_apple_isSandboxed()) + return false; +#endif + static const bool result = []() { + semget(IPC_PRIVATE, -1, 0); // this will fail + return errno != ENOSYS; + }(); + return result; +} + /*! \internal diff --git a/src/corelib/ipc/qtipccommon_p.h b/src/corelib/ipc/qtipccommon_p.h index 32891905d78..72b624c78f8 100644 --- a/src/corelib/ipc/qtipccommon_p.h +++ b/src/corelib/ipc/qtipccommon_p.h @@ -63,6 +63,61 @@ bool isIpcSupportedAtRuntime(IpcType type, QNativeIpcKey::Type); static constexpr auto isIpcSupportedAtRuntime = isIpcSupported; #endif +template class IpcStorageVariant +{ + template static C extractClass(T C::*); + template static T extractObject(T C::*); + + template + static constexpr bool IsEnabled = decltype(extractObject(M))::Enabled; + + static_assert(std::is_member_object_pointer_v); + using StorageType = decltype(extractClass(Member1)); + StorageType d; + +public: + template static auto + visit_internal(StorageType &storage, QNativeIpcKey::Type keyType, const Lambda &lambda) + { + if constexpr ((IsEnabled || ... || IsEnabled)) { + if constexpr (IsEnabled) { + using MemberType1 = decltype(extractObject(Member1)); + if (MemberType1::supports(keyType)) + return lambda(&(storage.*Member1)); + } + if constexpr ((... || IsEnabled)) + return IpcStorageVariant::visit_internal(storage, keyType, lambda); + Q_UNREACHABLE(); + } else { + // no backends enabled, but we can't return void + return false; + } + } + + template auto visit(QNativeIpcKey::Type keyType, const Lambda &lambda) + { + return visit_internal(d, keyType, lambda); + } + + template static auto + staticVisit(QNativeIpcKey::Type keyType, const Lambda &lambda) + { + if constexpr ((IsEnabled || ... || IsEnabled)) { + if constexpr (IsEnabled) { + using MemberType1 = decltype(extractObject(Member1)); + if (MemberType1::supports(keyType)) + return lambda(static_cast(nullptr)); + } + if constexpr ((... || IsEnabled)) + return IpcStorageVariant::staticVisit(keyType, lambda); + Q_UNREACHABLE(); + } else { + // no backends enabled, but we can't return void + return false; + } + } +}; + Q_AUTOTEST_EXPORT QString legacyPlatformSafeKey(const QString &key, IpcType ipcType, QNativeIpcKey::Type type = QNativeIpcKey::legacyDefaultTypeForOs()); diff --git a/tests/auto/corelib/ipc/ipctestcommon.h b/tests/auto/corelib/ipc/ipctestcommon.h new file mode 100644 index 00000000000..25c7b8ce440 --- /dev/null +++ b/tests/auto/corelib/ipc/ipctestcommon.h @@ -0,0 +1,83 @@ +// Copyright (C) 2022 Intel Corporation. + +#include +#include + +namespace IpcTestCommon { +static QList supportedKeyTypes; + +template void addGlobalTestRows() +{ + qDebug() << "Default key type is" << QNativeIpcKey::DefaultTypeForOs + << "and legacy key type is" << QNativeIpcKey::legacyDefaultTypeForOs(); + +#if defined(Q_OS_FREEBSD) || defined(Q_OS_DARWIN) || defined(Q_OS_WIN) || (defined(Q_OS_LINUX) && !defined(Q_OS_ANDROID)) + // only enforce that IPC works on the platforms above; other platforms may + // have no working backends (notably, Android) + QVERIFY(IpcClass::isKeyTypeSupported(QNativeIpcKey::DefaultTypeForOs)); + QVERIFY(IpcClass::isKeyTypeSupported(QNativeIpcKey::legacyDefaultTypeForOs())); +#endif + + auto addRowIfSupported = [](const char *name, QNativeIpcKey::Type type) { + if (IpcClass::isKeyTypeSupported(type)) { + supportedKeyTypes << type; + QTest::newRow(name) << type; + } + }; + + QTest::addColumn("keyType"); + + addRowIfSupported("Windows", QNativeIpcKey::Type::Windows); + addRowIfSupported("POSIX", QNativeIpcKey::Type::PosixRealtime); + addRowIfSupported("SystemV-Q", QNativeIpcKey::Type::SystemV); + addRowIfSupported("SystemV-T", QNativeIpcKey::Type('T')); + + if (supportedKeyTypes.isEmpty()) + QSKIP("System reports no supported IPC types."); +} + +// rotate through the supported types and find another +inline QNativeIpcKey::Type nextKeyType(QNativeIpcKey::Type type) +{ + qsizetype idx = supportedKeyTypes.indexOf(type); + Q_ASSERT(idx >= 0); + + ++idx; + if (idx == supportedKeyTypes.size()) + idx = 0; + return supportedKeyTypes.at(idx); +} +} // namespace IpcTestCommon + +QT_BEGIN_NAMESPACE +namespace QTest { +template<> inline char *toString(const QNativeIpcKey::Type &type) +{ + switch (type) { + case QNativeIpcKey::Type::SystemV: return qstrdup("SystemV"); + case QNativeIpcKey::Type::PosixRealtime: return qstrdup("PosixRealTime"); + case QNativeIpcKey::Type::Windows: return qstrdup("Windows"); + } + if (type == QNativeIpcKey::Type{}) + return qstrdup("Invalid"); + + char buf[32]; + qsnprintf(buf, sizeof(buf), "%u", unsigned(type)); + return qstrdup(buf); +} + +template<> inline char *toString(const QNativeIpcKey &key) +{ + if (!key.isValid()) + return qstrdup(""); + + const char *type = toString(key.type()); + const char *text = toString(key.nativeKey()); + char buf[256]; + qsnprintf(buf, sizeof(buf), "QNativeIpcKey(%s, %s)", text, type); + delete[] type; + delete[] text; + return qstrdup(buf); +} +} // namespace QTest +QT_END_NAMESPACE diff --git a/tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp b/tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp index 0f4bce80978..bca3e7e0109 100644 --- a/tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp +++ b/tests/auto/corelib/ipc/qnativeipckey/tst_qnativeipckey.cpp @@ -4,39 +4,10 @@ #include #include +#include "../ipctestcommon.h" + using namespace Qt::StringLiterals; -namespace QTest { -template<> inline char *toString(const QNativeIpcKey::Type &type) -{ - switch (type) { - case QNativeIpcKey::Type::SystemV: return qstrdup("SystemV"); - case QNativeIpcKey::Type::PosixRealtime: return qstrdup("PosixRealTime"); - case QNativeIpcKey::Type::Windows: return qstrdup("Windows"); - } - if (type == QNativeIpcKey::Type{}) - return qstrdup("Invalid"); - - char buf[32]; - qsnprintf(buf, sizeof(buf), "%u", unsigned(type)); - return qstrdup(buf); -} - -template<> inline char *toString(const QNativeIpcKey &key) -{ - if (!key.isValid()) - return qstrdup(""); - - const char *type = toString(key.type()); - const char *text = toString(key.nativeKey()); - char buf[256]; - qsnprintf(buf, sizeof(buf), "QNativeIpcKey(%s, %s)", text, type); - delete[] type; - delete[] text; - return qstrdup(buf); -} -} // namespace QTest - class tst_QNativeIpcKey : public QObject { Q_OBJECT diff --git a/tests/auto/corelib/ipc/qsystemsemaphore/tst_qsystemsemaphore.cpp b/tests/auto/corelib/ipc/qsystemsemaphore/tst_qsystemsemaphore.cpp index 685dda71c7d..560f216cf8e 100644 --- a/tests/auto/corelib/ipc/qsystemsemaphore/tst_qsystemsemaphore.cpp +++ b/tests/auto/corelib/ipc/qsystemsemaphore/tst_qsystemsemaphore.cpp @@ -11,6 +11,8 @@ #include #include +#include "../ipctestcommon.h" + #define HELPERWAITTIME 10000 using namespace Qt::StringLiterals; @@ -32,11 +34,12 @@ public: QNativeIpcKey platformSafeKey(const QString &key) { - QNativeIpcKey::Type keyType = QNativeIpcKey::DefaultTypeForOs; + QFETCH_GLOBAL(QNativeIpcKey::Type, keyType); return QSystemSemaphore::platformSafeKey(mangleKey(key), keyType); } public Q_SLOTS: + void initTestCase(); void init(); void cleanup(); @@ -46,9 +49,11 @@ private slots: void legacyKey_data() { nativeKey_data(); } void legacyKey(); + void changeKeyType(); void basicacquire(); void complexacquire(); void release(); + void twoSemaphores(); void basicProcesses(); @@ -70,6 +75,11 @@ tst_QSystemSemaphore::tst_QSystemSemaphore() { } +void tst_QSystemSemaphore::initTestCase() +{ + IpcTestCommon::addGlobalTestRows(); +} + void tst_QSystemSemaphore::init() { QNativeIpcKey key = platformSafeKey("existing"); @@ -111,6 +121,14 @@ void tst_QSystemSemaphore::nativeKey() QCOMPARE(sem.nativeIpcKey(), setIpcKey); QCOMPARE(sem.error(), QSystemSemaphore::NoError); QCOMPARE(sem.errorString(), QString()); + + // change the key type + QNativeIpcKey::Type nextKeyType = IpcTestCommon::nextKeyType(setIpcKey.type()); + if (nextKeyType != setIpcKey.type()) { + QNativeIpcKey setIpcKey2 = QSystemSemaphore::platformSafeKey(setKey, nextKeyType); + sem.setNativeKey(setIpcKey2); + QCOMPARE(sem.nativeIpcKey(), setIpcKey2); + } } QT_WARNING_PUSH @@ -132,6 +150,21 @@ void tst_QSystemSemaphore::legacyKey() } QT_WARNING_POP +void tst_QSystemSemaphore::changeKeyType() +{ + QString keyName = "changeKeyType"; + QNativeIpcKey key = platformSafeKey(keyName); + QNativeIpcKey otherKey = + QSystemSemaphore::platformSafeKey(mangleKey(keyName), IpcTestCommon::nextKeyType(key.type())); + if (key == otherKey) + QSKIP("System only supports one key type"); + + QSystemSemaphore sem1(key, 1, QSystemSemaphore::Create); + QSystemSemaphore sem2(otherKey); + sem1.setNativeKey(otherKey); + sem2.setNativeKey(key); +} + void tst_QSystemSemaphore::basicacquire() { QNativeIpcKey key = platformSafeKey("basicacquire"); @@ -185,6 +218,17 @@ void tst_QSystemSemaphore::release() QCOMPARE(sem.errorString(), QString()); } +void tst_QSystemSemaphore::twoSemaphores() +{ + QNativeIpcKey key = platformSafeKey("twoSemaphores"); + QSystemSemaphore sem1(key, 1, QSystemSemaphore::Create); + QSystemSemaphore sem2(key); + QVERIFY(sem1.acquire()); + QVERIFY(sem2.release()); + QVERIFY(sem1.acquire()); + QVERIFY(sem2.release()); +} + void tst_QSystemSemaphore::basicProcesses() { #if !QT_CONFIG(process)