From ffb9dee1b0954e4d4f9e9791175609a80ecafc31 Mon Sep 17 00:00:00 2001 From: Andreas Buhr Date: Wed, 15 Jun 2022 16:14:47 +0200 Subject: [PATCH] Proper clearing of WinRT factory cache If we use winrt's factories we have to make sure to to clear the factory cache when one of our dlls is unloaded or we will run into dangling factory entries which might result in crashes. So we have to make sure that winrt::clear_factory_cache is called on every dll unload. In order not to increase compile times and dependencies too much qfactorycacheregistration_p.h needs to be included in Qt code whenever we use winrt's factory cache. A rule of thumb being: Include qfactorycacheregistration_p.h whenever including winrt/base.h. Other Qt modules which use winrt's factories need to be updated too. Fixes: QTBUG-103611 Pick-to: 6.2 6.4 Change-Id: I7ab24e4b18bffaca653c5b7f56a66ce99212e339 Reviewed-by: Volker Hilsheimer --- src/corelib/CMakeLists.txt | 6 +++ src/corelib/configure.cmake | 2 +- .../windows/qfactorycacheregistration.cpp | 53 +++++++++++++++++++ .../windows/qfactorycacheregistration_p.h | 52 ++++++++++++++++++ src/corelib/text/qlocale_win.cpp | 1 + .../qnetworklistmanagerevents.cpp | 1 + .../qnetworklistmanagerevents.h | 1 + .../platforms/windows/qwindowscontext.cpp | 7 ++- .../kernel/qapplication/tst_qapplication.cpp | 15 ++++++ 9 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 src/corelib/platform/windows/qfactorycacheregistration.cpp create mode 100644 src/corelib/platform/windows/qfactorycacheregistration_p.h diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index f9c5690037c..f583d3180c8 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -1015,6 +1015,12 @@ qt_internal_extend_target(Core CONDITION ANDROID platform/android/qandroidnativeinterface.cpp ) +qt_internal_extend_target(Core CONDITION WIN32 + SOURCES + platform/windows/qfactorycacheregistration_p.h + platform/windows/qfactorycacheregistration.cpp +) + qt_internal_extend_target(Core CONDITION HAIKU AND NOT ANDROID SOURCES io/qstandardpaths_haiku.cpp diff --git a/src/corelib/configure.cmake b/src/corelib/configure.cmake index 5563729a5be..a8d6037a7d6 100644 --- a/src/corelib/configure.cmake +++ b/src/corelib/configure.cmake @@ -746,7 +746,7 @@ qt_feature("xmlstream" PUBLIC LABEL "XML Streaming APIs" PURPOSE "Provides a simple streaming API for XML." ) -qt_feature("cpp-winrt" PRIVATE +qt_feature("cpp-winrt" PRIVATE PUBLIC LABEL "cpp/winrt base" PURPOSE "basic cpp/winrt language projection support" CONDITION WIN32 AND TEST_cpp_winrt diff --git a/src/corelib/platform/windows/qfactorycacheregistration.cpp b/src/corelib/platform/windows/qfactorycacheregistration.cpp new file mode 100644 index 00000000000..6bd69c66d14 --- /dev/null +++ b/src/corelib/platform/windows/qfactorycacheregistration.cpp @@ -0,0 +1,53 @@ +// Copyright (C) 2022 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only + +#include "qfactorycacheregistration_p.h" + +#include + +QT_BEGIN_NAMESPACE + +#ifdef QT_USE_FACTORY_CACHE_REGISTRATION + +static QBasicMutex registrationMutex; +static detail::QWinRTFactoryCacheRegistration *firstElement; + +detail::QWinRTFactoryCacheRegistration::QWinRTFactoryCacheRegistration( + QFunctionPointer clearFunction) + : m_clearFunction(clearFunction) +{ + QMutexLocker lock(®istrationMutex); + + // forward pointers + m_next = std::exchange(firstElement, this); + + // backward pointers + m_prevNext = &firstElement; + if (m_next) + m_next->m_prevNext = &m_next; +} + +detail::QWinRTFactoryCacheRegistration::~QWinRTFactoryCacheRegistration() +{ + QMutexLocker lock(®istrationMutex); + + *m_prevNext = m_next; + + if (m_next) + m_next->m_prevNext = m_prevNext; +} + +void detail::QWinRTFactoryCacheRegistration::clearAllCaches() +{ + QMutexLocker lock(®istrationMutex); + + detail::QWinRTFactoryCacheRegistration *element; + + for (element = firstElement; element != nullptr; element = element->m_next) { + element->m_clearFunction(); + } +} + +#endif + +QT_END_NAMESPACE diff --git a/src/corelib/platform/windows/qfactorycacheregistration_p.h b/src/corelib/platform/windows/qfactorycacheregistration_p.h new file mode 100644 index 00000000000..db1582e94be --- /dev/null +++ b/src/corelib/platform/windows/qfactorycacheregistration_p.h @@ -0,0 +1,52 @@ +// Copyright (C) 2022 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only + +#ifndef QFACTORYCACHEREGISTRATION_P_H +#define QFACTORYCACHEREGISTRATION_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists purely as an +// implementation detail. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. +// + +#include + +#if !defined(QT_BOOTSTRAPPED) && defined(Q_OS_WIN) && !defined(Q_CC_CLANG) && QT_CONFIG(cpp_winrt) +# define QT_USE_FACTORY_CACHE_REGISTRATION +#endif + +#ifdef QT_USE_FACTORY_CACHE_REGISTRATION + +#include + +QT_BEGIN_NAMESPACE + +namespace detail { + +class QWinRTFactoryCacheRegistration +{ +public: + Q_CORE_EXPORT explicit QWinRTFactoryCacheRegistration(QFunctionPointer clearFunction); + Q_CORE_EXPORT ~QWinRTFactoryCacheRegistration(); + Q_CORE_EXPORT static void clearAllCaches(); + + Q_DISABLE_COPY_MOVE(QWinRTFactoryCacheRegistration) +private: + QWinRTFactoryCacheRegistration **m_prevNext = nullptr; + QWinRTFactoryCacheRegistration *m_next = nullptr; + QFunctionPointer m_clearFunction; +}; + +inline QWinRTFactoryCacheRegistration reg([]() noexcept { winrt::clear_factory_cache(); }); +} + +QT_END_NAMESPACE + +#endif +#endif // QFACTORYCACHEREGISTRATION_P_H diff --git a/src/corelib/text/qlocale_win.cpp b/src/corelib/text/qlocale_win.cpp index 3a3ae443c66..821369ff890 100644 --- a/src/corelib/text/qlocale_win.cpp +++ b/src/corelib/text/qlocale_win.cpp @@ -21,6 +21,7 @@ #if QT_CONFIG(cpp_winrt) # include +# include // Workaround for Windows SDK bug. // See https://github.com/microsoft/Windows.UI.Composition-Win32-Samples/issues/47 namespace winrt::impl diff --git a/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.cpp b/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.cpp index 8d4411c10b3..db789df244c 100644 --- a/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.cpp +++ b/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.cpp @@ -5,6 +5,7 @@ #if QT_CONFIG(cpp_winrt) #include +#include // Workaround for Windows SDK bug. // See https://github.com/microsoft/Windows.UI.Composition-Win32-Samples/issues/47 namespace winrt::impl diff --git a/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.h b/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.h index d5a23a18158..9d4d0381036 100644 --- a/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.h +++ b/src/plugins/networkinformation/networklistmanager/qnetworklistmanagerevents.h @@ -17,6 +17,7 @@ #if QT_CONFIG(cpp_winrt) #include +#include #endif using namespace Microsoft::WRL; diff --git a/src/plugins/platforms/windows/qwindowscontext.cpp b/src/plugins/platforms/windows/qwindowscontext.cpp index 80ff40acc5a..eeb1e08e0cc 100644 --- a/src/plugins/platforms/windows/qwindowscontext.cpp +++ b/src/plugins/platforms/windows/qwindowscontext.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include @@ -221,8 +222,12 @@ QWindowsContext::~QWindowsContext() DestroyWindow(d->m_powerDummyWindow); unregisterWindowClasses(); - if (d->m_oleInitializeResult == S_OK || d->m_oleInitializeResult == S_FALSE) + if (d->m_oleInitializeResult == S_OK || d->m_oleInitializeResult == S_FALSE) { +#ifdef QT_USE_FACTORY_CACHE_REGISTRATION + detail::QWinRTFactoryCacheRegistration::clearAllCaches(); +#endif OleUninitialize(); + } d->m_screenManager.clearScreens(); // Order: Potentially calls back to the windows. if (d->m_displayContext) diff --git a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp index 90f365668d1..6e2e6b3e497 100644 --- a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp +++ b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp @@ -128,6 +128,7 @@ private slots: void wheelEventPropagation(); void qtbug_12673(); + void qtbug_103611(); void noQuitOnHide(); void globalStaticObjectDestruction(); // run this last @@ -2513,6 +2514,20 @@ void tst_QApplication::qtbug_12673() #endif } +void tst_QApplication::qtbug_103611() +{ + { + int argc = 0; + QApplication app(argc, nullptr); + auto ll = QLocale().uiLanguages(); + } + { + int argc = 0; + QApplication app(argc, nullptr); + auto ll = QLocale().uiLanguages(); + } +} + class NoQuitOnHideWidget : public QWidget { Q_OBJECT