diff options
author | Jøger Hansegård <joger.hansegard@qt.io> | 2024-04-22 16:26:40 +0200 |
---|---|---|
committer | Jøger Hansegård <joger.hansegard@qt.io> | 2024-04-26 15:04:57 +0200 |
commit | 2a217aba34faa5cfe221d36ef3e7b8b03c33364b (patch) | |
tree | 5f06ec1ead77a2dc9b87947f82d0130b2af30b95 | |
parent | 6b06add397dd77dbed9d4a26897db4de4a6ec651 (diff) |
Manage media backend lifetime using Q_APPLICATION_STATIC
This patch reworks lifetime management of the Qt media backend to use
a plain Q_APPLICATION_STATIC instead of a hybrid between a
Q_APPLICATION_STATIC and a Q_GLOBAL_STATIC.
This hybrid mechanism tolerated creating the Qt media backend before a
QApplication was instantiated. This was considered necessary because Qt
example code for enumerating media devices previously demonstrated this
without a QApplication. Unfortunately, the hybrid mechanism prevented
maintenance of the Q_APPLICATION_STATIC implementation, and was on its
own complex to understand and maintain.
By switching to lifetime management using Q_APPLICATION_STATIC, we may
break debug builds of legacy applications that enumerate media devices
without having a QApplication, but we consider this an unlikely
scenario. We have updated Qt documentation to state that a Qt
application object is required to access any Qt Multimedia APIs
(9cf4494d49170eafb9c179d5304c0a70eaf25b96), and error logging is
improved to make developers aware of this requirement
(c2b1fb925798f5a87b21891dcf18cd967436de3e). We are now making this
requirement harder by asserting in debug.
With this change, legacy applications that use Qt Multimedia APIs
without having a Qt application object will assert in debug builds.
Release builds will for now continue to work as before, but with extra
error logging.
[ChangeLog][Changed lifetime management of the Qt Media backend. The Qt
media backend lifetime is now always bound to a Qt application object,
and Qt Multimedia APIs require that a Qt application object exists
before use. Using Qt multimedia APIs without a Qt application object
will assert in debug builds and result in undefined behavior in release
builds.]
Fixes: QTBUG-124577
Task-number: QTBUG-120198
Task-number: QTBUG-124578
Change-Id: I2b9864d8ee3dc937a80796891ff13854ce8968ee
Reviewed-by: Artem Dyomin <artem.dyomin@qt.io>
-rw-r--r-- | src/multimedia/platform/qplatformmediaintegration.cpp | 41 | ||||
-rw-r--r-- | tests/auto/integration/multiapp/tst_multiapp.cpp | 28 |
2 files changed, 8 insertions, 61 deletions
diff --git a/src/multimedia/platform/qplatformmediaintegration.cpp b/src/multimedia/platform/qplatformmediaintegration.cpp index c8f662963..dda00de61 100644 --- a/src/multimedia/platform/qplatformmediaintegration.cpp +++ b/src/multimedia/platform/qplatformmediaintegration.cpp @@ -91,48 +91,10 @@ struct InstanceHolder qCDebug(qLcMediaPlugin) << "Released media backend"; } - // Play nice with QtGlobalStatic::ApplicationHolder - using QAS_Type = InstanceHolder; - static void innerFunction(void *pointer) - { - new (pointer) InstanceHolder(); - } - std::unique_ptr<QPlatformMediaIntegration> instance; }; -// Specialized implementation of Q_APPLICATION_STATIC which behaves as -// an application static if a Qt application is present, otherwise as a Q_GLOBAL_STATIC. -// By doing this, and we have a Qt application, all system resources allocated by the -// backend is released when application lifetime ends. This is important on Windows, -// where Windows Media Foundation instances should not be released during static destruction. -// -// If we don't have a Qt application available when instantiating the instance holder, -// it will be created once, and not destroyed until static destruction. This can cause -// abrupt termination of Windows applications during static destruction. This is not a -// supported use case, but we keep this as a fallback to keep old applications functional. -// See also QTBUG-120198 -struct ApplicationHolder : QtGlobalStatic::ApplicationHolder<InstanceHolder> -{ - // Replace QtGlobalStatic::ApplicationHolder::pointer to prevent crash if - // no application is present - static InstanceHolder* pointer() - { - if (guard.loadAcquire() == QtGlobalStatic::Initialized) - return realPointer(); - - QMutexLocker locker(&mutex); - if (guard.loadRelaxed() == QtGlobalStatic::Uninitialized) { - InstanceHolder::innerFunction(&storage); - - if (const QCoreApplication *app = QCoreApplication::instance()) - QObject::connect(app, &QObject::destroyed, app, reset, Qt::DirectConnection); - - guard.storeRelease(QtGlobalStatic::Initialized); - } - return realPointer(); - } -}; +Q_APPLICATION_STATIC(InstanceHolder, s_instanceHolder); } // namespace @@ -140,7 +102,6 @@ QT_BEGIN_NAMESPACE QPlatformMediaIntegration *QPlatformMediaIntegration::instance() { - static QGlobalStatic<ApplicationHolder> s_instanceHolder; return s_instanceHolder->instance.get(); } diff --git a/tests/auto/integration/multiapp/tst_multiapp.cpp b/tests/auto/integration/multiapp/tst_multiapp.cpp index 607b0b5e7..793a56e9d 100644 --- a/tests/auto/integration/multiapp/tst_multiapp.cpp +++ b/tests/auto/integration/multiapp/tst_multiapp.cpp @@ -33,37 +33,23 @@ public slots: } private slots: - void mediaDevices_doesNotCrash_whenCalledWithoutApplication() + void mediaDevices_doesNotCrash_whenRecreatingApplication() { QVERIFY(executeTestOutOfProcess( - "mediaDevices_doesNotCrash_whenCalledWithoutApplication_impl"_L1)); + "mediaDevices_doesNotCrash_whenRecreatingApplication_impl"_L1)); } - bool mediaDevices_doesNotCrash_whenCalledWithoutApplication_impl(int /*argc*/, char ** /*argv*/) + bool mediaDevices_doesNotCrash_whenRecreatingApplication_impl(int argc, char ** argv) { - Q_ASSERT(!qApp); - - QMediaDevices::defaultAudioOutput(); // Just verify that we don't crash - return true; - } - - void mediaDevices_doesNotCrash_whenCalledAfterApplicationExit() - { - QVERIFY(executeTestOutOfProcess( - "mediaDevices_doesNotCrash_whenCalledAfterApplicationExit_impl"_L1)); - } - - bool mediaDevices_doesNotCrash_whenCalledAfterApplicationExit_impl(int argc, char **argv) - { - Q_ASSERT(!qApp); - { QCoreApplication app{ argc, argv }; - // Create the backend bound to the lifetime of the app + QMediaDevices::defaultAudioOutput(); + } + { + QCoreApplication app{ argc, argv }; QMediaDevices::defaultAudioOutput(); } - QMediaDevices::defaultAudioOutput(); // Just verify that we don't crash return true; } |