AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QGradientCache: fix a new/delete mismatch

Browse Source 
Commit f839f536 fixed a data race in the gradient cache by
reference-counting the CacheInfo objects stored in the cache. To this
end, QSpanData gained a ref-counted pointer to the CacheInfo whose
members it references to keep the object alive for as long as the
QSpanData object needs it. However, since CacheInfo is only later
defined in qpaintengine_raster.cpp, the counted pointer's payload was
chosen as CacheInfo's base class, QSharedData.

As it turns out, e.g. in the QPainter test, the data race was real and
so QSpanData ends up being the entity that destroys (at least some)
CacheInfos, either in its destructor, or in the setup() method. Since
QSharedData's destructor is not virtual, and
QExplicitlySharedDataPointer<QSharedData> knows nothing of the
CacheInfo-ness of its payload, we end up calling the destructor of the
base class, and not the CacheInfo one.

Fix by using QSharedPointer instead, which stores the correct deleter
internally. Ideally, QSpanData would contain a QSharedPointer<const
void>, but QSharedPointer's implementation is deficient in that
respect and does not compile when instantiated with void, and we can't
use std::shared_ptr, yet, so introduce an arbitrary base class,
Pinnable, to be used instead.

Change-Id: I5573c599d5464278d3a8e4248d887ef9ffcd7b70
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
Marc Mutz 2017-01-10 12:49:11 +01:00
parent 610c7da075
commit 7d898ae38e
2 changed files with 16 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/gui/painting/qdrawhelper_p.h
View File

@ -64,6 +64,8 @@
#include "private/qrasterdefs_p.h"
#include <private/qsimd_p.h>
#include <QtCore/qsharedpointer.h>
QT_BEGIN_NAMESPACE
#if defined(Q_CC_GNU)
@ -335,7 +337,11 @@ struct QSpanData
QGradientData gradient;
QTextureData texture;
};
QExplicitlySharedDataPointer<const QSharedData> cachedGradient;
class Pinnable {
protected:
~Pinnable() {}
}; // QSharedPointer<const void> is not supported
QSharedPointer<const Pinnable> cachedGradient;
void init(QRasterBuffer *rb, const QRasterPaintEngine *pe);

18
src/gui/painting/qpaintengine_raster.cpp
View File

@ -4150,7 +4150,7 @@ void QRasterBuffer::flushToARGBImage(QImage *target) const
class QGradientCache
{
public:
struct CacheInfo : public QSharedData
struct CacheInfo : QSpanData::Pinnable
{
inline CacheInfo(QGradientStops s, int op, QGradient::InterpolationMode mode) :
stops(qMove(s)), opacity(op), interpolationMode(mode) {}
@ -4161,9 +4161,9 @@ public:
QGradient::InterpolationMode interpolationMode;
};
typedef QMultiHash<quint64, QExplicitlySharedDataPointer<const CacheInfo> > QGradientColorTableHash;
typedef QMultiHash<quint64, QSharedPointer<const CacheInfo>> QGradientColorTableHash;
inline QExplicitlySharedDataPointer<const CacheInfo> getBuffer(const QGradient &gradient, int opacity) {
inline QSharedPointer<const CacheInfo> getBuffer(const QGradient &gradient, int opacity) {
quint64 hash_val = 0;
const QGradientStops stops = gradient.stops();
@ -4177,7 +4177,7 @@ public:
return addCacheElement(hash_val, gradient, opacity);
else {
do {
const QExplicitlySharedDataPointer<const CacheInfo> &cache_info = it.value();
const auto &cache_info = it.value();
if (cache_info->stops == stops && cache_info->opacity == opacity && cache_info->interpolationMode == gradient.interpolationMode())
return cache_info;
++it;
@ -4193,12 +4193,12 @@ protected:
inline void generateGradientColorTable(const QGradient& g,
QRgba64 *colorTable,
int size, int opacity) const;
QExplicitlySharedDataPointer<const CacheInfo> addCacheElement(quint64 hash_val, const QGradient &gradient, int opacity) {
QSharedPointer<const CacheInfo> addCacheElement(quint64 hash_val, const QGradient &gradient, int opacity) {
if (cache.size() == maxCacheSize()) {
// may remove more than 1, but OK
cache.erase(cache.begin() + (qrand() % maxCacheSize()));
}
QExplicitlySharedDataPointer<CacheInfo> cache_entry(new CacheInfo (gradient.stops(), opacity, gradient.interpolationMode()));
auto cache_entry = QSharedPointer<CacheInfo>::create(gradient.stops(), opacity, gradient.interpolationMode());
generateGradientColorTable(gradient, cache_entry->buffer64, paletteSize(), opacity);
for (int i = 0; i < GRADIENT_STOPTABLE_SIZE; ++i)
cache_entry->buffer32[i] = cache_entry->buffer64[i].toArgb32();
@ -4438,7 +4438,7 @@ void QSpanData::setup(const QBrush &brush, int alpha, QPainter::CompositionMode
const QLinearGradient *g = static_cast<const QLinearGradient *>(brush.gradient());
gradient.alphaColor = !brush.isOpaque() || alpha != 256;
QExplicitlySharedDataPointer<const QGradientCache::CacheInfo> cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha);
auto cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha);
cachedGradient = cacheInfo;
gradient.colorTable32 = cacheInfo->buffer32;
gradient.colorTable64 = cacheInfo->buffer64;
@ -4460,7 +4460,7 @@ void QSpanData::setup(const QBrush &brush, int alpha, QPainter::CompositionMode
const QRadialGradient *g = static_cast<const QRadialGradient *>(brush.gradient());
gradient.alphaColor = !brush.isOpaque() || alpha != 256;
QExplicitlySharedDataPointer<const QGradientCache::CacheInfo> cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha);
auto cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha);
cachedGradient = cacheInfo;
gradient.colorTable32 = cacheInfo->buffer32;
gradient.colorTable64 = cacheInfo->buffer64;
@ -4486,7 +4486,7 @@ void QSpanData::setup(const QBrush &brush, int alpha, QPainter::CompositionMode
const QConicalGradient *g = static_cast<const QConicalGradient *>(brush.gradient());
gradient.alphaColor = !brush.isOpaque() || alpha != 256;
QExplicitlySharedDataPointer<const QGradientCache::CacheInfo> cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha);
auto cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha);
cachedGradient = cacheInfo;
gradient.colorTable32 = cacheInfo->buffer32;
gradient.colorTable64 = cacheInfo->buffer64;