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>

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

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);
     });

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

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
 

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 (!comHelper.isValid())
-    if (FAILED(hr)) {
-        qCDebug(lcNetMon) << "Failed to initialize COM:" << errorStringFromHResult(hr);
-        comInitFailed = true;
         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);

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 (!comHelper.isValid())
-    if (FAILED(hr)) {
-        qCWarning(lcNetInfoNLM) << "Failed to initialize COM:" << errorStringFromHResult(hr);
-        comInitFailed = true;
         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)

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; }

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
+    QComHelper comHelper;
-     * ensure CoInitialize is only called once. */
-    static HRESULT comInit = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
-    Q_UNUSED(comInit);
 
     static QCache<QString, FakePointer<int> > dirIconEntryCache(1000);
     static QMutex mx;

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

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

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);
+    QComHelper comHelper;
-    QVERIFY(SUCCEEDED(hr));
+    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