whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QWindowsTheme: Run dedicated SHGetFileInfo() thread 

The changes are motivated by the following reasons:

1. SHGetFileInfo() needs to be COM-intialized per thread.
   Microsoft's documentation for CoInitalizeEx() is quite
   unambiguous about this.

2. Following point 1, using a thread from the global thread
   pool means we would taint every such thread with the
   COM-initialization state. This may result in unexpected
   behavior in other parts of the application. Moreover,
   systematic COM-uninitialization can be expensive and
   can't be recommended in this case.

3. Even though the timeout duration is pretty generous, the
   logic is wrong and could lead to serious errors should
   the call to SHGetFileInfo() actually take too long. This
   is because we let the thread run with references to the
   main thread's stack, namely a reference to the file name
   string, and pointers to the result variable and SHFILEINFO
   struct.

Running a dedicated thread allows us to ensure points 1
and 2. Point 3 is ensured by making a local copy of the
file name and using local instances for the info struct
and the result. Then, provided the thread has not been
cancelled, we can copy the info and result values back
into the main thread's stack referenced memory areas.

This also removes all need for QWindowsThreadPoolRunner
which will remain in the code base nonetheless.

Change-Id: Ic9c2d6204ac015aa409db2b57a09837361203291
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Reviewed-by: Andy Shaw <andy.shaw@qt.io>
bb10
Gabriel de Dietrich 2016-11-22 18:43:28 -08:00
parent 39e64e798f
commit e1113cae98
2 changed files with 100 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

131
src/plugins/platforms/windows/qwindowstheme.cpp
View File

@ -61,6 +61,9 @@
#include <QtCore/QTextStream>
#include <QtCore/QSysInfo>
#include <QtCore/QCache>
#include <QtCore/QThread>
#include <QtCore/QMutex>
#include <QtCore/QWaitCondition>
#include <QtGui/QColor>
#include <QtGui/QPalette>
#include <QtGui/QGuiApplication>
@ -127,40 +130,114 @@ static inline QColor getSysColor(int index)
// QTBUG-48823/Windows 10: SHGetFileInfo() (as called by item views on file system
// models has been observed to trigger a WM_PAINT on the mainwindow. Suppress the
// behavior by running it in a thread.
class ShGetFileInfoFunction
struct QShGetFileInfoParams
{
QShGetFileInfoParams(const QString &fn, DWORD a, SHFILEINFO *i, UINT f, bool *r)
: fileName(fn), attributes(a), flags(f), info(i), result(r)
{ }
const QString &fileName;
const DWORD attributes;
const UINT flags;
SHFILEINFO *const info;
bool *const result;
};
class QShGetFileInfoThread : public QThread
{
public:
explicit ShGetFileInfoFunction(const wchar_t *fn, DWORD a, SHFILEINFO *i, UINT f, bool *r) :
m_fileName(fn), m_attributes(a), m_flags(f), m_info(i), m_result(r) {}
void operator()() const
explicit QShGetFileInfoThread()
: QThread(), m_params(nullptr)
{
connect(this, &QThread::finished, this, &QObject::deleteLater);
}
void run() override
{
m_init = CoInitializeEx(NULL, COINIT_MULTITHREADED);
QMutexLocker readyLocker(&m_readyMutex);
while (!m_cancelled.load()) {
if (!m_params && !m_cancelled.load()
&& !m_readyCondition.wait(&m_readyMutex, 1000))
continue;
if (m_params) {
const QString fileName = m_params->fileName;
SHFILEINFO info;
#ifndef Q_OS_WINCE
const UINT oldErrorMode = SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX);
const UINT oldErrorMode = SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX);
#endif
*m_result = SHGetFileInfo(m_fileName, m_attributes, m_info, sizeof(SHFILEINFO), m_flags);
const bool result = SHGetFileInfo(reinterpret_cast<const wchar_t *>(fileName.utf16()),
m_params->attributes, &info, sizeof(SHFILEINFO),
m_params->flags);
#ifndef Q_OS_WINCE
SetErrorMode(oldErrorMode);
SetErrorMode(oldErrorMode);
#endif
m_doneMutex.lock();
if (!m_cancelled.load()) {
*m_params->result = result;
memcpy(m_params->info, &info, sizeof(SHFILEINFO));
}
m_params = nullptr;
m_doneCondition.wakeAll();
m_doneMutex.unlock();
}
}
if (m_init != S_FALSE)
CoUninitialize();
}
bool runWithParams(QShGetFileInfoParams *params, unsigned long timeOutMSecs)
{
QMutexLocker doneLocker(&m_doneMutex);
m_readyMutex.lock();
m_params = params;
m_readyCondition.wakeAll();
m_readyMutex.unlock();
return m_doneCondition.wait(&m_doneMutex, timeOutMSecs);
}
void cancel()
{
QMutexLocker doneLocker(&m_doneMutex);
m_cancelled.store(1);
m_readyCondition.wakeAll();
}
private:
const wchar_t *m_fileName;
const DWORD m_attributes;
const UINT m_flags;
SHFILEINFO *const m_info;
bool *m_result;
HRESULT m_init;
QShGetFileInfoParams *m_params;
QAtomicInt m_cancelled;
QWaitCondition m_readyCondition;
QWaitCondition m_doneCondition;
QMutex m_readyMutex;
QMutex m_doneMutex;
};
static bool shGetFileInfoBackground(QWindowsThreadPoolRunner &r,
const wchar_t *fileName, DWORD attributes,
static bool shGetFileInfoBackground(const QString &fileName, DWORD attributes,
SHFILEINFO *info, UINT flags,
unsigned long timeOutMSecs = 5000)
{
static QShGetFileInfoThread *getFileInfoThread = nullptr;
if (!getFileInfoThread) {
getFileInfoThread = new QShGetFileInfoThread;
getFileInfoThread->start();
}
bool result = false;
if (!r.run(ShGetFileInfoFunction(fileName, attributes, info, flags, &result), timeOutMSecs)) {
qWarning().noquote() << "ShGetFileInfoBackground() timed out for "
<< QString::fromWCharArray(fileName);
QShGetFileInfoParams params(fileName, attributes, info, flags, &result);
if (!getFileInfoThread->runWithParams(&params, timeOutMSecs)) {
// Cancel and reset getFileInfoThread. It'll
// be reinitialized the next time we get called.
getFileInfoThread->cancel();
getFileInfoThread = nullptr;
qWarning().noquote() << "SHGetFileInfo() timed out for " << fileName;
return false;
}
return result;
@ -315,7 +392,6 @@ const char *QWindowsTheme::name = "windows";
QWindowsTheme *QWindowsTheme::m_instance = 0;
QWindowsTheme::QWindowsTheme()
: m_threadPoolRunner(new QWindowsThreadPoolRunner)
{
m_instance = this;
std::fill(m_fonts, m_fonts + NFonts, static_cast<QFont *>(0));
@ -718,10 +794,8 @@ static QPixmap pixmapFromShellImageList(int iImageList, const SHFILEINFO &info)
class QWindowsFileIconEngine : public QAbstractFileIconEngine
{
public:
explicit QWindowsFileIconEngine(const QFileInfo &info,
QPlatformTheme::IconOptions opts,
const QSharedPointer<QWindowsThreadPoolRunner> &runner) :
QAbstractFileIconEngine(info, opts), m_threadPoolRunner(runner) {}
explicit QWindowsFileIconEngine(const QFileInfo &info, QPlatformTheme::IconOptions opts) :
QAbstractFileIconEngine(info, opts) {}
QList<QSize> availableSizes(QIcon::Mode = QIcon::Normal, QIcon::State = QIcon::Off) const override
{ return QWindowsTheme::instance()->availableFileIconSizes(); }
@ -729,9 +803,6 @@ public:
protected:
QString cacheKey() const override;
QPixmap filePixmap(const QSize &size, QIcon::Mode mode, QIcon::State) override;
private:
const QSharedPointer<QWindowsThreadPoolRunner> m_threadPoolRunner;
};
QString QWindowsFileIconEngine::cacheKey() const
@ -797,10 +868,8 @@ QPixmap QWindowsFileIconEngine::filePixmap(const QSize &size, QIcon::Mode, QIcon
SHGFI_ICON|iconSize|SHGFI_SYSICONINDEX|SHGFI_ADDOVERLAYS|SHGFI_OVERLAYINDEX;
const bool val = cacheableDirIcon && useDefaultFolderIcon
? shGetFileInfoBackground(*m_threadPoolRunner.data(), L"dummy", FILE_ATTRIBUTE_DIRECTORY,
&info, flags | SHGFI_USEFILEATTRIBUTES)
: shGetFileInfoBackground(*m_threadPoolRunner.data(), reinterpret_cast<const wchar_t *>(filePath.utf16()), 0,
&info, flags);
? shGetFileInfoBackground(QString::fromWCharArray(L"dummy"), FILE_ATTRIBUTE_DIRECTORY, &info, flags | SHGFI_USEFILEATTRIBUTES)
: shGetFileInfoBackground(filePath, 0, &info, flags);
// Even if GetFileInfo returns a valid result, hIcon can be empty in some cases
if (val && info.hIcon) {
@ -844,7 +913,7 @@ QPixmap QWindowsFileIconEngine::filePixmap(const QSize &size, QIcon::Mode, QIcon
QIcon QWindowsTheme::fileIcon(const QFileInfo &fileInfo, QPlatformTheme::IconOptions iconOptions) const
{
return QIcon(new QWindowsFileIconEngine(fileInfo, iconOptions, m_threadPoolRunner));
return QIcon(new QWindowsFileIconEngine(fileInfo, iconOptions));
}
QT_END_NAMESPACE

2
src/plugins/platforms/windows/qwindowstheme.h
View File

@ -40,7 +40,6 @@
#ifndef QWINDOWSTHEME_H
#define QWINDOWSTHEME_H
#include "qwindowsthreadpoolrunner.h"
#include <qpa/qplatformtheme.h>
#include <QtCore/QSharedPointer>
@ -88,7 +87,6 @@ private:
static QWindowsTheme *m_instance;
QPalette *m_palettes[NPalettes];
QFont *m_fonts[NFonts];
const QSharedPointer<QWindowsThreadPoolRunner> m_threadPoolRunner;
QList<QSize> m_fileIconSizes;
};