diff options
author | Tor Arne Vestbø <tor.arne.vestbo@qt.io> | 2022-08-25 10:15:16 +0200 |
---|---|---|
committer | Tor Arne Vestbø <tor.arne.vestbo@qt.io> | 2022-08-29 15:39:34 +0200 |
commit | 1bc7e9e77b2d6b03b995f376b622d4c8c97dd51c (patch) | |
tree | 23aabcab8feb2489a144d5229a6c8ee40b05bd56 | |
parent | 49f9cadb52b5ef29e527b60c3881cb15a69c1625 (diff) |
Add QComHelper class for dealing with COM on Windows
Unifies our approach to calling CoInitializeEx and CoUninitialize,
removing a lot of boilerplate in the process, and also fixes a few
bugs where we would incorrectly balance our calls to CoInitializeEx
and CoUninitialize.
The optimistic approach of qfilesystemengine_win.cpp of calling
CoCreateInstance without initializing the COM library explicitly
has been removed, as calling CoInitializeEx should be a noop in
the situation where it's already been loaded.
Change-Id: I9e2ec101678c2ebb9946504b5e8034e58f1bb56a
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
-rw-r--r-- | src/corelib/CMakeLists.txt | 2 | ||||
-rw-r--r-- | src/corelib/io/qfilesystemengine_win.cpp | 29 | ||||
-rw-r--r-- | src/corelib/kernel/qfunctions_win.cpp | 28 | ||||
-rw-r--r-- | src/corelib/kernel/qfunctions_win_p.h | 24 | ||||
-rw-r--r-- | src/network/kernel/qnetconmonitor_win.cpp | 16 | ||||
-rw-r--r-- | src/plugins/networkinformation/networklistmanager/qnetworklistmanagernetworkinformationbackend.cpp | 17 | ||||
-rw-r--r-- | src/plugins/platforms/windows/qwindowsservices.cpp | 6 | ||||
-rw-r--r-- | src/plugins/platforms/windows/qwindowstheme.cpp | 12 | ||||
-rw-r--r-- | src/tools/bootstrap/CMakeLists.txt | 1 | ||||
-rw-r--r-- | tests/auto/corelib/io/qfile/tst_qfile.cpp | 11 | ||||
-rw-r--r-- | tests/auto/other/qaccessibility/tst_qaccessibility.cpp | 8 |
11 files changed, 87 insertions, 67 deletions
diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index ddb32ac5fb..0f0c576a64 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -538,7 +538,7 @@ qt_internal_extend_target(Core CONDITION WIN32 kernel/qcoreapplication_win.cpp kernel/qelapsedtimer_win.cpp kernel/qeventdispatcher_win.cpp kernel/qeventdispatcher_win_p.h - kernel/qfunctions_win_p.h kernel/qfunctions_winrt_p.h + kernel/qfunctions_win.cpp kernel/qfunctions_win_p.h kernel/qfunctions_winrt_p.h kernel/qsharedmemory_win.cpp kernel/qsystemsemaphore_win.cpp kernel/qwineventnotifier.cpp kernel/qwineventnotifier.h kernel/qwineventnotifier_p.h diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp index d36e896305..f80a6805e9 100644 --- a/src/corelib/io/qfilesystemengine_win.cpp +++ b/src/corelib/io/qfilesystemengine_win.cpp @@ -37,6 +37,8 @@ #define SECURITY_WIN32 #include <security.h> +#include <QtCore/private/qfunctions_win_p.h> + #ifndef SPI_GETPLATFORMTYPE #define SPI_GETPLATFORMTYPE 257 #endif @@ -668,21 +670,16 @@ static QString readLink(const QFileSystemEntry &link) #if QT_CONFIG(fslibs) QString ret; - bool neededCoInit = false; IShellLink *psl; // pointer to IShellLink i/f WIN32_FIND_DATA wfd; wchar_t szGotPath[MAX_PATH]; + QComHelper comHelper; + // Get pointer to the IShellLink interface. HRESULT hres = CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_IShellLink, (LPVOID *)&psl); - if (hres == CO_E_NOTINITIALIZED) { // COM was not initialized - neededCoInit = true; - CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); - hres = CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_IShellLink, - (LPVOID *)&psl); - } if (SUCCEEDED(hres)) { // Get pointer to the IPersistFile interface. IPersistFile *ppf; hres = psl->QueryInterface(IID_IPersistFile, (LPVOID *)&ppf); @@ -698,8 +695,6 @@ static QString readLink(const QFileSystemEntry &link) } psl->Release(); } - if (neededCoInit) - CoUninitialize(); return ret; #else @@ -1661,18 +1656,11 @@ bool QFileSystemEngine::createLink(const QFileSystemEntry &source, const QFileSy QSystemError &error) { bool ret = false; + QComHelper comHelper; IShellLink *psl = nullptr; HRESULT hres = CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_IShellLink, reinterpret_cast<void **>(&psl)); - bool neededCoInit = false; - if (hres == CO_E_NOTINITIALIZED) { // COM was not initialized - neededCoInit = true; - CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); - hres = CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_IShellLink, - reinterpret_cast<void **>(&psl)); - } - if (SUCCEEDED(hres)) { const auto name = QDir::toNativeSeparators(source.filePath()); const auto pathName = QDir::toNativeSeparators(source.path()); @@ -1692,9 +1680,6 @@ bool QFileSystemEngine::createLink(const QFileSystemEntry &source, const QFileSy if (!ret) error = QSystemError(::GetLastError(), QSystemError::NativeError); - if (neededCoInit) - CoUninitialize(); - return ret; } @@ -1762,7 +1747,8 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source, // we need the "display name" of the file, so can't use nativeAbsoluteFilePath const QString sourcePath = QDir::toNativeSeparators(absoluteName(source).filePath()); - CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); + QComHelper comHelper; + IFileOperation *pfo = nullptr; IShellItem *deleteItem = nullptr; FileOperationProgressSink *sink = nullptr; @@ -1775,7 +1761,6 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source, deleteItem->Release(); if (pfo) pfo->Release(); - CoUninitialize(); if (!SUCCEEDED(hres)) error = QSystemError(hres, QSystemError::NativeError); }); diff --git a/src/corelib/kernel/qfunctions_win.cpp b/src/corelib/kernel/qfunctions_win.cpp new file mode 100644 index 0000000000..ca72c395a9 --- /dev/null +++ b/src/corelib/kernel/qfunctions_win.cpp @@ -0,0 +1,28 @@ +// 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 "qfunctions_win_p.h" + +#include <combaseapi.h> +#include <objbase.h> + +QT_BEGIN_NAMESPACE + +QComHelper::QComHelper(COINIT concurrencyModel) +{ + // Avoid overhead of initializing and using obsolete technology + concurrencyModel = COINIT(concurrencyModel | COINIT_DISABLE_OLE1DDE); + + m_initResult = CoInitializeEx(nullptr, concurrencyModel); + + if (FAILED(m_initResult)) + qErrnoWarning(m_initResult, "Failed to initialize COM library"); +} + +QComHelper::~QComHelper() +{ + if (SUCCEEDED(m_initResult)) + CoUninitialize(); +} + +QT_END_NAMESPACE diff --git a/src/corelib/kernel/qfunctions_win_p.h b/src/corelib/kernel/qfunctions_win_p.h index 9e4f3c83bd..46c4bfa9fe 100644 --- a/src/corelib/kernel/qfunctions_win_p.h +++ b/src/corelib/kernel/qfunctions_win_p.h @@ -15,9 +15,33 @@ // We mean it. // +#include <QtCore/private/qglobal_p.h> + #if defined(Q_OS_WIN) +#if !defined(QT_BOOTSTRAPPED) #include <QtCore/private/qfunctions_winrt_p.h> +#endif + +#include <QtCore/qt_windows.h> + +QT_BEGIN_NAMESPACE + +class Q_CORE_EXPORT QComHelper +{ + Q_DISABLE_COPY_MOVE(QComHelper) +public: + QComHelper(COINIT concurrencyModel = COINIT_APARTMENTTHREADED); + ~QComHelper(); + + bool isValid() const { return SUCCEEDED(m_initResult); } + explicit operator bool() const { return isValid(); } + +private: + HRESULT m_initResult = E_FAIL; +}; + +QT_END_NAMESPACE #endif // Q_OS_WIN diff --git a/src/network/kernel/qnetconmonitor_win.cpp b/src/network/kernel/qnetconmonitor_win.cpp index df19ed5f29..22bbed2bea 100644 --- a/src/network/kernel/qnetconmonitor_win.cpp +++ b/src/network/kernel/qnetconmonitor_win.cpp @@ -8,6 +8,8 @@ #include <QtCore/quuid.h> #include <QtCore/qmetaobject.h> +#include <QtCore/private/qfunctions_win_p.h> + #include <QtNetwork/qnetworkinterface.h> #include <objbase.h> @@ -124,6 +126,8 @@ public: void setConnectivity(NLM_CONNECTIVITY newConnectivity); private: + QComHelper comHelper; + ComPtr<QNetworkConnectionEvents> connectionEvents; // We can assume we have access to internet/subnet when this class is created because // connection has already been established to the peer: @@ -136,7 +140,6 @@ private: bool sameSubnet = false; bool isLinkLocal = false; bool monitoring = false; - bool comInitFailed = false; bool remoteIsIPv6 = false; }; @@ -299,30 +302,25 @@ bool QNetworkConnectionEvents::stopMonitoring() QNetworkConnectionMonitorPrivate::QNetworkConnectionMonitorPrivate() { - auto hr = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); - if (FAILED(hr)) { - qCDebug(lcNetMon) << "Failed to initialize COM:" << errorStringFromHResult(hr); - comInitFailed = true; + if (!comHelper.isValid()) return; - } connectionEvents = new QNetworkConnectionEvents(this); } QNetworkConnectionMonitorPrivate::~QNetworkConnectionMonitorPrivate() { - if (comInitFailed) + if (!comHelper.isValid()) return; if (monitoring) stopMonitoring(); connectionEvents.Reset(); - CoUninitialize(); } bool QNetworkConnectionMonitorPrivate::setTargets(const QHostAddress &local, const QHostAddress &remote) { - if (comInitFailed) + if (!comHelper.isValid()) return false; QNetworkInterface iface = getInterfaceFromHostAddress(local); diff --git a/src/plugins/networkinformation/networklistmanager/qnetworklistmanagernetworkinformationbackend.cpp b/src/plugins/networkinformation/networklistmanager/qnetworklistmanagernetworkinformationbackend.cpp index 6862077aba..f5c5131894 100644 --- a/src/plugins/networkinformation/networklistmanager/qnetworklistmanagernetworkinformationbackend.cpp +++ b/src/plugins/networkinformation/networklistmanager/qnetworklistmanagernetworkinformationbackend.cpp @@ -9,6 +9,8 @@ #include <QtCore/private/qobject_p.h> #include <QtCore/qscopeguard.h> +#include <QtCore/private/qfunctions_win_p.h> + QT_BEGIN_NAMESPACE // Declared in qnetworklistmanagerevents.h @@ -82,12 +84,13 @@ private: void setConnectivity(NLM_CONNECTIVITY newConnectivity); void checkCaptivePortal(); + QComHelper comHelper; + ComPtr<QNetworkListManagerEvents> managerEvents; NLM_CONNECTIVITY connectivity = NLM_CONNECTIVITY_DISCONNECTED; bool monitoring = false; - bool comInitFailed = false; }; class QNetworkListManagerNetworkInformationBackendFactory : public QNetworkInformationBackendFactory @@ -121,12 +124,9 @@ public: QNetworkListManagerNetworkInformationBackend::QNetworkListManagerNetworkInformationBackend() { - auto hr = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); - if (FAILED(hr)) { - qCWarning(lcNetInfoNLM) << "Failed to initialize COM:" << errorStringFromHResult(hr); - comInitFailed = true; + if (!comHelper.isValid()) return; - } + managerEvents = new QNetworkListManagerEvents(); connect(managerEvents.Get(), &QNetworkListManagerEvents::connectivityChanged, this, &QNetworkListManagerNetworkInformationBackend::setConnectivity); @@ -140,10 +140,7 @@ QNetworkListManagerNetworkInformationBackend::QNetworkListManagerNetworkInformat QNetworkListManagerNetworkInformationBackend::~QNetworkListManagerNetworkInformationBackend() { - if (comInitFailed) - return; stop(); - CoUninitialize(); } void QNetworkListManagerNetworkInformationBackend::setConnectivity(NLM_CONNECTIVITY newConnectivity) @@ -175,7 +172,7 @@ bool QNetworkListManagerNetworkInformationBackend::start() { Q_ASSERT(!monitoring); - if (comInitFailed) + if (!comHelper.isValid()) return false; if (!managerEvents) diff --git a/src/plugins/platforms/windows/qwindowsservices.cpp b/src/plugins/platforms/windows/qwindowsservices.cpp index f2bd90f4bd..2c84204d2b 100644 --- a/src/plugins/platforms/windows/qwindowsservices.cpp +++ b/src/plugins/platforms/windows/qwindowsservices.cpp @@ -12,6 +12,7 @@ #include <QtCore/qthread.h> #include <QtCore/private/qwinregistry_p.h> +#include <QtCore/private/qfunctions_win_p.h> #include <shlobj.h> #include <shlwapi.h> @@ -34,11 +35,10 @@ public: void run() override { - if (SUCCEEDED(CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE))) { + QComHelper comHelper; + if (comHelper.isValid()) m_result = ShellExecute(nullptr, m_operation, m_file, m_parameters, nullptr, SW_SHOWNORMAL); - CoUninitialize(); - } } HINSTANCE result() const { return m_result; } diff --git a/src/plugins/platforms/windows/qwindowstheme.cpp b/src/plugins/platforms/windows/qwindowstheme.cpp index 38311363c5..61643e7700 100644 --- a/src/plugins/platforms/windows/qwindowstheme.cpp +++ b/src/plugins/platforms/windows/qwindowstheme.cpp @@ -43,6 +43,7 @@ #include <private/qhighdpiscaling_p.h> #include <private/qsystemlibrary_p.h> #include <private/qwinregistry_p.h> +#include <QtCore/private/qfunctions_win_p.h> #include <algorithm> @@ -145,7 +146,7 @@ public: void run() override { - m_init = CoInitializeEx(nullptr, COINIT_MULTITHREADED); + QComHelper comHelper(COINIT_MULTITHREADED); QMutexLocker readyLocker(&m_readyMutex); while (!m_cancelled.loadRelaxed()) { @@ -170,9 +171,6 @@ public: m_doneMutex.unlock(); } } - - if (m_init != S_FALSE) - CoUninitialize(); } bool runWithParams(QShGetFileInfoParams *params, qint64 timeOutMSecs) @@ -195,7 +193,6 @@ public: } private: - HRESULT m_init; QShGetFileInfoParams *m_params; QAtomicInt m_cancelled; QWaitCondition m_readyCondition; @@ -980,10 +977,7 @@ QString QWindowsFileIconEngine::cacheKey() const QPixmap QWindowsFileIconEngine::filePixmap(const QSize &size, QIcon::Mode, QIcon::State) { - /* We don't use the variable, but by storing it statically, we - * ensure CoInitialize is only called once. */ - static HRESULT comInit = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); - Q_UNUSED(comInit); + QComHelper comHelper; static QCache<QString, FakePointer<int> > dirIconEntryCache(1000); static QMutex mx; diff --git a/src/tools/bootstrap/CMakeLists.txt b/src/tools/bootstrap/CMakeLists.txt index c86a4bf896..8964e27fda 100644 --- a/src/tools/bootstrap/CMakeLists.txt +++ b/src/tools/bootstrap/CMakeLists.txt @@ -136,6 +136,7 @@ qt_internal_extend_target(Bootstrap CONDITION WIN32 ../../corelib/kernel/qcoreapplication_win.cpp ../../corelib/kernel/qwinregistry.cpp ../../corelib/plugin/qsystemlibrary.cpp + ../../corelib/kernel/qfunctions_win.cpp PUBLIC_LIBRARIES advapi32 netapi32 diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp index 41dfd12d4f..cbe46ca041 100644 --- a/tests/auto/corelib/io/qfile/tst_qfile.cpp +++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp @@ -22,6 +22,7 @@ #include <private/qabstractfileengine_p.h> #include <private/qfsfileengine_p.h> #include <private/qfilesystemengine_p.h> +#include <QtCore/private/qfunctions_win_p.h> #include <QtTest/private/qemulationdetector_p.h> @@ -1540,13 +1541,9 @@ static QString getWorkingDirectoryForLink(const QString &linkFileName) bool neededCoInit = false; QString ret; + QComHelper comHelper; IShellLink *psl; HRESULT hres = CoCreateInstance(CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, IID_IShellLink, (void **)&psl); - if (hres == CO_E_NOTINITIALIZED) { // COM was not initialized - neededCoInit = true; - CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); - hres = CoCreateInstance(CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, IID_IShellLink, (void **)&psl); - } if (SUCCEEDED(hres)) { // Get pointer to the IPersistFile interface. IPersistFile *ppf; @@ -1565,10 +1562,6 @@ static QString getWorkingDirectoryForLink(const QString &linkFileName) psl->Release(); } - if (neededCoInit) { - CoUninitialize(); - } - return ret; } #endif diff --git a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp index 90f1647567..97ca35453b 100644 --- a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp +++ b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp @@ -18,6 +18,7 @@ #include <qpa/qplatformnativeinterface.h> #include <qpa/qplatformintegration.h> #include <qpa/qplatformaccessibility.h> +#include <QtCore/private/qfunctions_win_p.h> #include <QtGui/private/qguiapplication_p.h> #include <QtGui/private/qhighdpiscaling_p.h> @@ -3846,14 +3847,14 @@ void tst_QAccessibility::bridgeTest() POINT pt{nativePos.x(), nativePos.y()}; // Initialize COM stuff. - HRESULT hr = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE); - QVERIFY(SUCCEEDED(hr)); + QComHelper comHelper; + QVERIFY(comHelper.isValid()); // Get UI Automation interface. const GUID CLSID_CUIAutomation_test{0xff48dba4, 0x60ef, 0x4201, {0xaa,0x87, 0x54,0x10,0x3e,0xef,0x59,0x4e}}; IUIAutomation *automation = nullptr; - hr = CoCreateInstance(CLSID_CUIAutomation_test, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&automation)); + HRESULT hr = CoCreateInstance(CLSID_CUIAutomation_test, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&automation)); QVERIFY(SUCCEEDED(hr)); // Get button element from UI Automation using point. @@ -3956,7 +3957,6 @@ void tst_QAccessibility::bridgeTest() controlWalker->Release(); windowElement->Release(); automation->Release(); - CoUninitialize(); QTestAccessibility::clearEvents(); #endif |