From 8227103ceb8600a781ee7bb9c90ecc38f30cb18d Mon Sep 17 00:00:00 2001 From: "Zheng, Lei" Date: Wed, 10 Jul 2019 09:03:31 +0800 Subject: [PATCH] Gui: AutoSaver and recovery changes * Fix AutoSaver inconsistent BinaryBrep setting * Use temperary name when auto saving, so that it won't overwrite the original file, which may cause corruption when crashing in the middel of auto saving, especially if auto saving in a separate thread. * Support auto recovery document containing external links * Do not mark success after auto recover, in case the program crash again before the user remember to save the just recovered file. Only mark when user saves the document. --- src/Base/Writer.h | 1 + src/Gui/AutoSaver.cpp | 43 ++++++++++++--- src/Gui/AutoSaver.h | 3 ++ src/Gui/DocumentRecovery.cpp | 101 ++++++++++++++++++++++++----------- 4 files changed, 109 insertions(+), 39 deletions(-) diff --git a/src/Base/Writer.h b/src/Base/Writer.h index 1678512b67..5aebdf01ef 100644 --- a/src/Base/Writer.h +++ b/src/Base/Writer.h @@ -195,6 +195,7 @@ public: virtual void writeFiles(void); virtual std::ostream &Stream(void){return FileStream;} + void close() {FileStream.close();} /*! This method can be re-implemented in sub-classes to avoid to write out certain objects. The default implementation diff --git a/src/Gui/AutoSaver.cpp b/src/Gui/AutoSaver.cpp index 98f020ee69..4f074db891 100644 --- a/src/Gui/AutoSaver.cpp +++ b/src/Gui/AutoSaver.cpp @@ -26,6 +26,7 @@ #ifndef _PreComp_ # include # include +# include # include # include # include @@ -49,6 +50,8 @@ #include "MainWindow.h" #include "ViewProvider.h" +FC_LOG_LEVEL_INIT("App",true,true) + using namespace Gui; AutoSaver* AutoSaver::self = 0; @@ -71,6 +74,15 @@ AutoSaver* AutoSaver::instance() return self; } +void AutoSaver::renameFile(QString dirName, QString file, QString tmpFile) +{ + FC_LOG("auto saver rename " << tmpFile.toUtf8().constData() + << " -> " << file.toUtf8().constData()); + QDir dir(dirName); + dir.remove(file); + dir.rename(tmpFile,file); +} + void AutoSaver::setTimeout(int ms) { timeout = Base::clamp(ms, 0, 3600000); // between 0 and 60 min @@ -122,7 +134,7 @@ void AutoSaver::saveDocument(const std::string& name, AutoSaveProperty& saver) { Gui::WaitCursor wc; App::Document* doc = App::GetApplication().getDocument(name.c_str()); - if (doc) { + if (doc && !doc->testStatus(App::Document::PartialDoc)) { // Set the document's current transient directory std::string dirName = doc->TransientDir.getValue(); dirName += "/fc_recovery_files"; @@ -159,8 +171,11 @@ void AutoSaver::saveDocument(const std::string& name, AutoSaveProperty& saver) { if (!this->compressed) { RecoveryWriter writer(saver); - if (hGrp->GetBool("SaveBinaryBrep", true)) - writer.setMode("BinaryBrep"); + + // We will be using thread pool if not compressed. + // So, always force binary format because ASCII + // is not reentrant. See PropertyPartShape::SaveDocFile + writer.setMode("BinaryBrep"); writer.putNextEntry("Document.xml"); @@ -310,10 +325,11 @@ public: , writer(dir) { writer.setModes(modes); - // always force binary format because ASCII - // is not reentrant. See PropertyPartShape::SaveDocFile - writer.setMode("BinaryBrep"); - writer.putNextEntry(file); + + dirName = QString::fromUtf8(dir); + fileName = QString::fromUtf8(file); + tmpName = QString::fromLatin1("%1.tmp%2").arg(fileName).arg(rand()); + writer.putNextEntry(tmpName.toUtf8().constData()); } virtual ~RecoveryRunnable() { @@ -322,11 +338,24 @@ public: virtual void run() { prop->SaveDocFile(writer); + writer.close(); + + // We could have renamed the file in this thread. However, there is + // still chance of crash when we deleted the original and before rename + // the new file. So we ask the main thread to do it. There is still + // possibility of crash caused by thread other than the main, but + // that's the best we can do for now. + QMetaObject::invokeMethod(AutoSaver::instance(), "renameFile", + Qt::QueuedConnection, Q_ARG(QString,dirName) + ,Q_ARG(QString,fileName),Q_ARG(QString,tmpName)); } private: App::Property* prop; Base::FileWriter writer; + QString dirName; + QString fileName; + QString tmpName; }; } diff --git a/src/Gui/AutoSaver.h b/src/Gui/AutoSaver.h index 9e7d77d59a..6766f763b5 100644 --- a/src/Gui/AutoSaver.h +++ b/src/Gui/AutoSaver.h @@ -88,6 +88,9 @@ protected: void timerEvent(QTimerEvent * event); void saveDocument(const std::string&, AutoSaveProperty&); +public Q_SLOTS: + void renameFile(QString dirName, QString file, QString tmpFile); + private: int timeout; /*!< Timeout in milliseconds */ bool compressed; diff --git a/src/Gui/DocumentRecovery.cpp b/src/Gui/DocumentRecovery.cpp index f531b3d787..ffbfeff128 100644 --- a/src/Gui/DocumentRecovery.cpp +++ b/src/Gui/DocumentRecovery.cpp @@ -47,6 +47,7 @@ # include #endif +#include #include "DocumentRecovery.h" #include "ui_DocumentRecovery.h" #include "WaitCursor.h" @@ -63,6 +64,8 @@ #include #include +FC_LOG_LEVEL_INIT("Gui",true,true); + using namespace Gui; using namespace Gui::Dialog; @@ -81,8 +84,10 @@ std::string DocumentRecovery::doctools = " self.dirname = dirname\n" "\n" " def startElement(self, name, attributes):\n" +" if name == 'XLink':\n" +" return\n" " item=attributes.get(\"file\")\n" -" if item != None:\n" +" if item:\n" " self.files.append(os.path.join(self.dirname,str(item)))\n" "\n" " def characters(self, data):\n" @@ -237,33 +242,25 @@ void DocumentRecovery::accept() if (!d->recovered) { WaitCursor wc; - int index = 0; - for (QList::iterator it = d->recoveryInfo.begin(); it != d->recoveryInfo.end(); ++it, index++) { + int index = -1; + std::vector indices; + std::vector filenames, pathes, labels, errs; + for(auto &info : d->recoveryInfo) { + ++index; std::string documentName; QString errorInfo; QTreeWidgetItem* item = d_ptr->ui.treeWidget->topLevelItem(index); try { - QString file = it->projectFile; + QString file = info.projectFile; QFileInfo fi(file); if (fi.fileName() == QLatin1String("Document.xml")) - file = createProjectFile(it->projectFile); - App::Document* document = App::GetApplication().newDocument(); - documentName = document->getName(); - document->FileName.setValue(file.toUtf8().constData()); + file = createProjectFile(info.projectFile); - // If something goes wrong an exception will be thrown here - document->restore(); - - file = it->fileName; - document->FileName.setValue(file.toUtf8().constData()); - document->Label.setValue(it->label.toUtf8().constData()); - - // Set the modified flag so that the user cannot close by accident - Gui::Document* guidoc = Gui::Application::Instance->getDocument(document); - if (guidoc) { - guidoc->setModified(true); - } + pathes.emplace_back(file.toUtf8().constData()); + filenames.emplace_back(info.fileName.toUtf8().constData()); + labels.emplace_back(info.label.toUtf8().constData()); + indices.push_back(index); } catch (const std::exception& e) { errorInfo = QString::fromLatin1(e.what()); @@ -275,31 +272,71 @@ void DocumentRecovery::accept() errorInfo = tr("Unknown problem occurred"); } - // an error occurred so close the document again if (!errorInfo.isEmpty()) { - if (!documentName.empty()) - App::GetApplication().closeDocument(documentName.c_str()); - - it->status = DocumentRecoveryPrivate::Failure; - + info.status = DocumentRecoveryPrivate::Failure; if (item) { item->setText(1, tr("Failed to recover")); item->setToolTip(1, errorInfo); item->setForeground(1, QColor(170,0,0)); } + d->writeRecoveryInfo(info); } - // everything OK - else { - it->status = DocumentRecoveryPrivate::Success; + } + auto docs = App::GetApplication().openDocuments(filenames,&pathes,&labels,&errs); + + for(int i=0;i<(int)docs.size();++i) { + auto &info = d->recoveryInfo[indices[i]]; + QTreeWidgetItem* item = d_ptr->ui.treeWidget->topLevelItem(indices[i]); + if(!docs[i] || errs[i].size()) { + if(docs[i]) + App::GetApplication().closeDocument(docs[i]->getName()); + info.status = DocumentRecoveryPrivate::Failure; + if (item) { + item->setText(1, tr("Failed to recover")); + item->setToolTip(1, QString::fromUtf8(errs[index].c_str())); + item->setForeground(1, QColor(170,0,0)); + } + // write back current status + d->writeRecoveryInfo(info); + }else{ + auto gdoc = Application::Instance->getDocument(docs[i]); + if(gdoc) + gdoc->setModified(true); + + info.status = DocumentRecoveryPrivate::Success; if (item) { item->setText(1, tr("Successfully recovered")); item->setForeground(1, QColor(0,170,0)); } - } - // write back current status - d->writeRecoveryInfo(*it); + QDir transDir(QString::fromUtf8(docs[i]->TransientDir.getValue())); + + QFileInfo xfi(info.xmlFile); + QFileInfo fi(info.projectFile); + bool res = false; + if(fi.fileName() == QLatin1String("fc_recovery_file.fcstd")) { + transDir.remove(fi.fileName()); + res = transDir.rename(fi.absoluteFilePath(),fi.fileName()); + }else{ + transDir.rmdir(fi.dir().dirName()); + res = transDir.rename(fi.absolutePath(),fi.dir().dirName()); + } + if(res) { + transDir.remove(xfi.fileName()); + res = transDir.rename(xfi.absoluteFilePath(),xfi.fileName()); + } + if(!res) { + FC_WARN("Failed to move recovery file of document '" + << docs[i]->Label.getValue() << "'"); + }else{ + clearDirectory(xfi.absolutePath()); + QDir().rmdir(xfi.absolutePath()); + } + + // DO NOT write success into recovery info, in case the program + // crash again before the user save the just recovered file. + } } d->ui.buttonBox->button(QDialogButtonBox::Ok)->setText(tr("Finish"));