From e8d9feed70d74ed3ac06a7b8d5b25eaac2d2b018 Mon Sep 17 00:00:00 2001 From: Albert Astals Cid Date: Wed, 14 Feb 2018 17:48:10 +0100 Subject: [PATCH] Fix potential crash in document saving Probably doesn't happen very often since most of the times people don't save while rendering is still happening but if that is the case we need to wait for all the rendering to finish otherwise we remove the document from under the render thread feet and bad things happen --- core/document.cpp | 53 +++++++++++++++++++++++++++-------------------- core/document_p.h | 2 ++ 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/core/document.cpp b/core/document.cpp index 4bba49288..dc4f430f3 100644 --- a/core/document.cpp +++ b/core/document.cpp @@ -2118,6 +2118,33 @@ void DocumentPrivate::loadSyncFile( const QString & filePath ) m_pagesVector[i]->setSourceReferences( refRects.at(i) ); } +void DocumentPrivate::clearAndWaitForRequests() +{ + m_pixmapRequestsMutex.lock(); + QLinkedList< PixmapRequest * >::const_iterator sIt = m_pixmapRequestsStack.constBegin(); + QLinkedList< PixmapRequest * >::const_iterator sEnd = m_pixmapRequestsStack.constEnd(); + for ( ; sIt != sEnd; ++sIt ) + delete *sIt; + m_pixmapRequestsStack.clear(); + m_pixmapRequestsMutex.unlock(); + + QEventLoop loop; + bool startEventLoop = false; + do + { + m_pixmapRequestsMutex.lock(); + startEventLoop = !m_executingPixmapRequests.isEmpty(); + m_pixmapRequestsMutex.unlock(); + if ( startEventLoop ) + { + m_closingLoop = &loop; + loop.exec(); + m_closingLoop = nullptr; + } + } + while ( startEventLoop ); +} + Document::Document( QWidget *widget ) : QObject( nullptr ), d( new DocumentPrivate( this ) ) { @@ -2533,29 +2560,7 @@ void Document::closeDocument() d->m_scripter = nullptr; // remove requests left in queue - d->m_pixmapRequestsMutex.lock(); - QLinkedList< PixmapRequest * >::const_iterator sIt = d->m_pixmapRequestsStack.constBegin(); - QLinkedList< PixmapRequest * >::const_iterator sEnd = d->m_pixmapRequestsStack.constEnd(); - for ( ; sIt != sEnd; ++sIt ) - delete *sIt; - d->m_pixmapRequestsStack.clear(); - d->m_pixmapRequestsMutex.unlock(); - - QEventLoop loop; - bool startEventLoop = false; - do - { - d->m_pixmapRequestsMutex.lock(); - startEventLoop = !d->m_executingPixmapRequests.isEmpty(); - d->m_pixmapRequestsMutex.unlock(); - if ( startEventLoop ) - { - d->m_closingLoop = &loop; - loop.exec(); - d->m_closingLoop = nullptr; - } - } - while ( startEventLoop ); + d->clearAndWaitForRequests(); if ( d->m_fontThread ) { @@ -4356,6 +4361,8 @@ bool Document::swapBackingFile( const QString &newFileName, const QUrl &url ) // Save metadata about the file we're about to close d->saveDocumentInfo(); + d->clearAndWaitForRequests(); + qCDebug(OkularCoreDebug) << "Swapping backing file to" << newFileName; QVector< Page * > newPagesVector; Generator::SwapBackingFileResult result = genIt->generator->swapBackingFile( newFileName, newPagesVector ); diff --git a/core/document_p.h b/core/document_p.h index 644a3c77b..6efe1b7fa 100644 --- a/core/document_p.h +++ b/core/document_p.h @@ -202,6 +202,8 @@ class DocumentPrivate // For sync files void loadSyncFile( const QString & filePath ); + void clearAndWaitForRequests(); + // member variables Document *m_parent; QPointer m_widget;