Integer overflow in qdeclarativepixmapcache

Summary:

There is a signed integer (m_unreferencedCost) that counts the amount in bytes of unreferenced pixmap data in the qt declarative pixmap cache (which is separate from the regular qpixmapcache). This number is not decremented when image data is released, which causes it to eventually overflow when the amount (in bytes) of pixmap data over the total application lifetime exceeds +2,147,483,647 (INT_MAX, 32 bits for most systems including 64 bit systems as it is declared int not long).

When it overflows into the negative, the while loop in QDeclarativePixmapStore::shrinkCache() will never delete future pixmap data added to the cache due to a greater than symbol condition.

All pixmaps streamed to QML from that point forth are cached and never released even when unreferenced, causing memory usage to skyrocket.

Details:

For some background info, our application is similar to a streaming camera video application, so new QPixmaps are constantly sent to a QML Image element.

Before 4.7.0-RC1, we updated the QML Image's pixmap property using a QObject exposed to QML via QDeclarativeContext::setContextProperty(). This QObject derived class had a QPixmap property.

Since RC1, QML Image's pixmap property has been removed, and we are left with QDeclarativeImageProvider. This class is not well-suited to streaming camera images to the GUI, as it expects the QML Image's source to be fairly static, a network resource, or located on disk.

As a workaround, we have a QObject with a QUrl property that we set as the source for the QML Image. We update this QUrl and send a NOTIFY signal to QML that the property has changed. We then make sure that the image provider we are using is ready to serve this dynamic new url. As a side note, before we update the QUrl property, we update it once with an empty QUrl so that the resources allocated by qdeclarativepixmap are freed, then we update it a second time with the new url.

This new url is typically just the frame number of the camera image we want to stream to the GUI, since we can no longer update the pixmap directly and we constantly want the "source:" of the QML Image to change.

Back to the bug at hand, everything is in the file src/declarative/util/qdeclarativepixmapcache.cpp. Inside this file you'll find a class called QDeclarativePixmapStore.

This class has a member called m_unreferencedCost, which is a signed int. This number is incremented inside the QDeclarativePixmapStore::unreferencePixmap() function. This function is typically called from QDeclarativePixmapData::release() (more on this later).

unreferencePixmap() will call a function called QDeclarativePixmapStore::shrinkCache(), after adding the cost of the last unreferenced pixmap to m_unreferencedCost.

Inside shrinkCache(), you have a while loop with the following condition:

while ((remove > 0 || m_unreferencedCost > cache_limit) && m_lastUnreferencedPixmap) {

Inside this while loop, the following code needs to be added (1 line added, from 4.7.0-RC1) :

remove = data>cost();
m_unreferencedCost = data>cost(); // this is the line added
data->removeFromCache();
delete data;

Without decrementing m_unreferencedCost by the amount of data being removed from the cache, it will count all unreferenced data for the entire lifetime of the application, including data that has been already unreferenced and subsequently freed.

For our application, it only takes about 30 seconds before INT_MAX worth of bytes has been streamed to the GUI, and m_unreferencedCost overflows into the negative, causing the while loop to never execute and free future data that has been added to the cache yet is unreferenced. This is due to the greater than used in the while loop condition: (m_unreferencedCost > cache_limit) no longer holds true when m_unreferencedCost is negative.

I think this is the way you would prefer having it fixed. There are alternative ways of looking at it, including deleting the data directly inside QDeclarativePixmapData::release(), instead of just unreferencing it and waiting for QDeclarativePixmapStore::shrinkCache() to (possibly!) delete it later.

Also, I hope this fix does not break other applications of QML Image. I noticed that m_unreferencedCost is subtracted from in only one other place (besides my fix), QDeclarativePixmapStore::referencePixmap(), but this function is never called for our use case since we constantly stream new images and never reference old ones even in the cache.

Also, I've noticed on gitorious that QML Image exposes whether the image is inserted in the cache or not. This would be useful for our use case since we don't need to cache streaming images, but I think the integer overflow bug would persist even if we cache nothing. Basically, m_unreferencedCost would still increase indefinitely as long as you stream QPixmap's from C++.

I hope this helps. We could really use this fix in time for 4.7.0 rather than waiting for 4.7.1.