Commit c2439f20 authored by Aurélien Gâteau's avatar Aurélien Gâteau
Browse files

Fix leaking documents

Documents were never released because DocumentInfoProvider kept loading them.
A big thank you to Benni Hill for discovering the bug and proposing the
initial patch.

BUG:296401
FIXED-IN:2.8.4
parent 2da5cb15
......@@ -61,14 +61,13 @@ void DocumentInfoProvider::thumbnailForDocument(const KUrl& url, ThumbnailGroup:
Q_ASSERT(outFullSize);
*outPix = QPixmap();
*outFullSize = QSize();
DocumentFactory* factory = DocumentFactory::instance();
const int pixelSize = ThumbnailGroup::pixelSize(group);
if (!factory->hasUrl(url)) {
Document::Ptr doc = DocumentFactory::instance()->getCachedDocument(url);
if (!doc) {
return;
}
Document::Ptr doc = factory->load(url);
if (doc->loadingState() != Document::Loaded) {
return;
}
......@@ -83,10 +82,8 @@ void DocumentInfoProvider::thumbnailForDocument(const KUrl& url, ThumbnailGroup:
bool DocumentInfoProvider::isModified(const KUrl& url)
{
DocumentFactory* factory = DocumentFactory::instance();
if (factory->hasUrl(url)) {
Document::Ptr doc = factory->load(url);
Document::Ptr doc = DocumentFactory::instance()->getCachedDocument(url);
if (doc) {
return doc->loadingState() == Document::Loaded && doc->isModified();
} else {
return false;
......@@ -95,10 +92,8 @@ bool DocumentInfoProvider::isModified(const KUrl& url)
bool DocumentInfoProvider::isBusy(const KUrl& url)
{
DocumentFactory* factory = DocumentFactory::instance();
if (factory->hasUrl(url)) {
Document::Ptr doc = factory->load(url);
Document::Ptr doc = DocumentFactory::instance()->getCachedDocument(url);
if (doc) {
return doc->isBusy();
} else {
return false;
......
......@@ -34,7 +34,7 @@ namespace Gwenview
#undef ENABLE_LOG
#undef LOG
//#define ENABLE_LOG
#define ENABLE_LOG
#ifdef ENABLE_LOG
#define LOG(x) kDebug() << x
#else
......@@ -84,8 +84,10 @@ struct DocumentFactoryPrivate
*/
void garbageCollect(DocumentMap& map)
{
// Build a map of all unreferenced images
typedef QMap<QDateTime, KUrl> UnreferencedImages;
// Build a map of all unreferenced images. We use a MultiMap because in
// rare cases documents may get accessed at the same millisecond.
// See https://bugs.kde.org/show_bug.cgi?id=296401
typedef QMultiMap<QDateTime, KUrl> UnreferencedImages;
UnreferencedImages unreferencedImages;
DocumentMap::Iterator
......@@ -94,7 +96,7 @@ struct DocumentFactoryPrivate
for (; it != end; ++it) {
DocumentInfo* info = it.value();
if (info->mDocument.count() == 1 && !info->mDocument->isModified()) {
unreferencedImages[info->mLastAccess] = it.key();
unreferencedImages.insert(info->mLastAccess, it.key());
}
}
......@@ -150,6 +152,12 @@ DocumentFactory* DocumentFactory::instance()
return &factory;
}
Document::Ptr DocumentFactory::getCachedDocument(const KUrl& url) const
{
const DocumentInfo* info = d->mDocumentMap.value(url);
return info ? info->mDocument : Document::Ptr();
}
Document::Ptr DocumentFactory::load(const KUrl& url)
{
DocumentInfo* info = 0;
......
......@@ -34,6 +34,13 @@ namespace Gwenview
struct DocumentFactoryPrivate;
/**
* This class holds all instances of Document.
*
* It keeps a cache of recently accessed documents to avoid reloading them.
* To do so it keeps a last-access timestamp, which is updated to the
* current time everytime DocumentFactory::load() is called.
*/
class GWENVIEWLIB_EXPORT DocumentFactory : public QObject
{
Q_OBJECT
......@@ -41,7 +48,18 @@ public:
static DocumentFactory* instance();
~DocumentFactory();
Document::Ptr load(const KUrl&);
/**
* Loads the document associated with url, or returns an already cached
* instance of Document::Ptr if there is any.
* This method updates the last-access timestamp.
*/
Document::Ptr load(const KUrl& url);
/**
* Returns a document if it has already been loaded once with load().
* This method does not update the last-access timestamp.
*/
Document::Ptr getCachedDocument(const KUrl&) const;
QList<KUrl> modifiedDocumentList() const;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment