From d69d2abbde9649060bf28f3e48a208ca55074a34 Mon Sep 17 00:00:00 2001 From: Jean-Francois Dockes Date: Thu, 17 May 2018 10:24:01 +0200 Subject: [PATCH] TempFile: clean-up interface by using internal ref-counted class member. Uncomp: add interface to clear cache --- src/internfile/internfile.cpp | 50 ++++++++++++++++----------------- src/internfile/internfile.h | 5 ++-- src/internfile/uncomp.cpp | 36 +++++++++++++++++++----- src/internfile/uncomp.h | 20 +++++--------- src/qtgui/main.cpp | 15 +++++----- src/qtgui/preview_w.cpp | 8 +++--- src/qtgui/rclm_idx.cpp | 16 +++++------ src/qtgui/rclm_view.cpp | 8 +++--- src/qtgui/rclmain_w.h | 2 +- src/qtgui/recoll.h | 2 +- src/utils/rclutil.cpp | 52 +++++++++++++++++++++++++++++++++-- src/utils/rclutil.h | 29 ++++++------------- 12 files changed, 147 insertions(+), 96 deletions(-) diff --git a/src/internfile/internfile.cpp b/src/internfile/internfile.cpp index 1c4a07c4..a18ee471 100644 --- a/src/internfile/internfile.cpp +++ b/src/internfile/internfile.cpp @@ -47,6 +47,7 @@ using namespace std; #include "copyfile.h" #include "fetcher.h" #include "extrameta.h" +#include "uncomp.h" // The internal path element separator. This can't be the same as the rcldb // file to ipath separator : "|" @@ -188,7 +189,7 @@ void FileInterner::init(const string &f, const struct stat *stp, RclConfig *cnf, int maxkbs = -1; if (!m_cfg->getConfParam("compressedfilemaxkbs", &maxkbs) || maxkbs < 0 || !stp || int(stp->st_size / 1024) < maxkbs) { - if (!m_uncomp.uncompressfile(m_fn, ucmd, m_tfile)) { + if (!m_uncomp->uncompressfile(m_fn, ucmd, m_tfile)) { return; } LOGDEB1("FileInterner:: after ucomp: tfile " << m_tfile <<"\n"); @@ -293,8 +294,8 @@ void FileInterner::init(const string &data, RclConfig *cnf, result = df->set_document_data(m_mimetype, data.c_str(), data.length()); } else if (df->is_data_input_ok(Dijon::Filter::DOCUMENT_FILE_NAME)) { TempFile temp = dataToTempFile(data, m_mimetype); - if (temp && - (result = df->set_document_file(m_mimetype, temp->filename()))) { + if (temp.ok() && + (result = df->set_document_file(m_mimetype, temp.filename()))) { m_tmpflgs[m_handlers.size()] = true; m_tempfiles.push_back(temp); } @@ -312,7 +313,8 @@ void FileInterner::init(const string &data, RclConfig *cnf, void FileInterner::initcommon(RclConfig *cnf, int flags) { m_cfg = cnf; - m_uncomp = m_forPreview = ((flags & FIF_forPreview) != 0); + m_forPreview = ((flags & FIF_forPreview) != 0); + m_uncomp = new Uncomp(m_forPreview); // Initialize handler stack. m_handlers.reserve(MAXHANDLERS); for (unsigned int i = 0; i < MAXHANDLERS; i++) @@ -373,10 +375,10 @@ bool FileInterner::makesig(RclConfig *cnf, const Rcl::Doc& idoc, string& sig) FileInterner::~FileInterner() { - for (vector::iterator it = m_handlers.begin(); - it != m_handlers.end(); it++) { - returnMimeHandler(*it); + for (auto& entry: m_handlers) { + returnMimeHandler(entry); } + delete m_uncomp; // m_tempfiles will take care of itself } @@ -386,14 +388,14 @@ FileInterner::~FileInterner() TempFile FileInterner::dataToTempFile(const string& dt, const string& mt) { // Create temp file with appropriate suffix for mime type - TempFile temp(new TempFileInternal(m_cfg->getSuffixFromMimeType(mt))); - if (!temp->ok()) { + TempFile temp(m_cfg->getSuffixFromMimeType(mt)); + if (!temp.ok()) { LOGERR("FileInterner::dataToTempFile: cant create tempfile: " << - temp->getreason() << "\n"); + temp.getreason() << "\n"); return TempFile(); } string reason; - if (!stringtofile(dt, temp->filename(), reason)) { + if (!stringtofile(dt, temp.filename(), reason)) { LOGERR("FileInterner::dataToTempFile: stringtofile: " <set_document_data(mimetype,txt->c_str(),txt->length()); } else if (newflt->is_data_input_ok(Dijon::Filter::DOCUMENT_FILE_NAME)) { TempFile temp = dataToTempFile(*txt, mimetype); - if (temp && - (setres = newflt->set_document_file(mimetype, temp->filename()))) { + if (temp.ok() && + (setres = newflt->set_document_file(mimetype, temp.filename()))) { m_tmpflgs[m_handlers.size()] = true; m_tempfiles.push_back(temp); // Hack here, but really helps perfs: if we happen to @@ -765,7 +767,7 @@ FileInterner::Status FileInterner::internfile(Rcl::Doc& doc,const string& ipath) LOGDEB("FileInterner::internfile. ipath [" << ipath << "]\n"); // Get rid of possible image tempfile from older call - m_imgtmp.reset(); + m_imgtmp = TempFile(); if (m_handlers.size() < 1) { // Just means the constructor failed @@ -916,9 +918,8 @@ FileInterner::Status FileInterner::internfile(Rcl::Doc& doc,const string& ipath) bool FileInterner::tempFileForMT(TempFile& otemp, RclConfig* cnf, const string& mimetype) { - TempFile temp(new TempFileInternal( - cnf->getSuffixFromMimeType(mimetype))); - if (!temp->ok()) { + TempFile temp(cnf->getSuffixFromMimeType(mimetype)); + if (!temp.ok()) { LOGERR("FileInterner::tempFileForMT: can't create temp file\n"); return false; } @@ -970,7 +971,7 @@ bool FileInterner::topdocToFile( if (!tempFileForMT(temp, cnf, idoc.mimetype)) { return false; } - filename = temp->filename(); + filename = temp.filename(); } else { filename = tofile.c_str(); } @@ -985,7 +986,7 @@ bool FileInterner::topdocToFile( return false; } } - fn = temp ? temp->filename() : rawdoc.data; + fn = temp.ok() ? temp.filename() : rawdoc.data; if (!copyfile(fn.c_str(), filename, reason)) { LOGERR("FileInterner::idocToFile: copyfile: " << reason << "\n"); return false; @@ -1040,7 +1041,7 @@ bool FileInterner::interntofile(TempFile& otemp, const string& tofile, if (!tempFileForMT(temp, m_cfg, mimetype)) { return false; } - filename = temp->filename(); + filename = temp.filename(); } else { filename = tofile.c_str(); } @@ -1106,9 +1107,8 @@ bool FileInterner::maybeUncompressToTemp(TempFile& temp, const string& fn, " kbs\n"); return false; } - temp = - TempFile(new TempFileInternal(cnf->getSuffixFromMimeType(doc.mimetype))); - if (!temp->ok()) { + temp = TempFile(cnf->getSuffixFromMimeType(doc.mimetype)); + if (!temp.ok()) { LOGERR("FileInterner: cant create temporary file\n"); return false; } @@ -1123,9 +1123,9 @@ bool FileInterner::maybeUncompressToTemp(TempFile& temp, const string& fn, // reason for this, but it's not nice here. Have to move, the // uncompressed file, hopefully staying on the same dev. string reason; - if (!renameormove(uncomped.c_str(), temp->filename(), reason)) { + if (!renameormove(uncomped.c_str(), temp.filename(), reason)) { LOGERR("FileInterner::maybeUncompress: move [" << uncomped << - "] -> [" << temp->filename() << "] failed: " << reason << "\n"); + "] -> [" << temp.filename() << "] failed: " << reason << "\n"); return false; } return true; diff --git a/src/internfile/internfile.h b/src/internfile/internfile.h index 3d5345de..8dd0b967 100644 --- a/src/internfile/internfile.h +++ b/src/internfile/internfile.h @@ -28,14 +28,15 @@ using std::map; using std::set; #include "mimehandler.h" -#include "uncomp.h" #include "pathut.h" +#include "rclutil.h" class RclConfig; namespace Rcl { class Doc; } +class Uncomp; struct stat; /** Storage for missing helper program info. We want to keep this out of the @@ -277,7 +278,7 @@ class FileInterner { string m_reason; FIMissingStore *m_missingdatap{nullptr}; - Uncomp m_uncomp; + Uncomp *m_uncomp{nullptr}; bool m_noxattrs; // disable xattrs usage bool m_direct; // External app did the extraction diff --git a/src/internfile/uncomp.cpp b/src/internfile/uncomp.cpp index 5741c3fb..fe45490e 100644 --- a/src/internfile/uncomp.cpp +++ b/src/internfile/uncomp.cpp @@ -35,6 +35,12 @@ using std::vector; Uncomp::UncompCache Uncomp::o_cache; +Uncomp::Uncomp(bool docache) + : m_docache(docache) +{ + LOGDEB0("Uncomp::Uncomp: m_docache: " << m_docache << "\n"); +} + bool Uncomp::uncompressfile(const string& ifn, const vector& cmdv, string& tfile) { @@ -57,7 +63,8 @@ bool Uncomp::uncompressfile(const string& ifn, } // Make sure tmp dir is empty. we guarantee this to filters if (!m_dir || !m_dir->ok() || !m_dir->wipe()) { - LOGERR("uncompressfile: can't clear temp dir " << (m_dir->dirname()) << "\n" ); + LOGERR("uncompressfile: can't clear temp dir " << m_dir->dirname() << + "\n"); return false; } @@ -66,12 +73,14 @@ bool Uncomp::uncompressfile(const string& ifn, int pc; long long availmbs; if (!fsocc(m_dir->dirname(), &pc, &availmbs)) { - LOGERR("uncompressfile: can't retrieve avail space for " << (m_dir->dirname()) << "\n" ); + LOGERR("uncompressfile: can't retrieve avail space for " << + m_dir->dirname() << "\n"); // Hope for the best } else { long long fsize = path_filesize(ifn); if (fsize < 0) { - LOGERR("uncompressfile: stat input file " << (ifn) << " errno " << (errno) << "\n" ); + LOGERR("uncompressfile: stat input file " << ifn << " errno " << + errno << "\n"); return false; } // We need at least twice the file size for the uncompressed @@ -83,7 +92,9 @@ bool Uncomp::uncompressfile(const string& ifn, long long filembs = fsize / (1024 * 1024); if (availmbs < 2 * filembs + 1) { - LOGERR("uncompressfile. " << (lltodecstr(availmbs)) << " MBs available in " << (m_dir->dirname()) << " not enough to uncompress " << (ifn) << " of size " << (lltodecstr(filembs)) << " mbs\n" ); + LOGERR("uncompressfile. " << availmbs << " MBs available in " << + m_dir->dirname() << " not enough to uncompress " << + ifn << " of size " << filembs << " MBs\n"); return false; } } @@ -107,9 +118,10 @@ bool Uncomp::uncompressfile(const string& ifn, ExecCmd ex; int status = ex.doexec(cmd, args, 0, &tfile); if (status || tfile.empty()) { - LOGERR("uncompressfile: doexec: failed for [" << (ifn) << "] status 0x" << (status) << "\n" ); + LOGERR("uncompressfile: doexec: failed for [" << ifn << "] status 0x" << + status << "\n"); if (!m_dir->wipe()) { - LOGERR("uncompressfile: wipedir failed\n" ); + LOGERR("uncompressfile: wipedir failed\n"); } return false; } @@ -122,6 +134,8 @@ bool Uncomp::uncompressfile(const string& ifn, Uncomp::~Uncomp() { + LOGDEB0("Uncomp::~Uncomp: m_docache: " << m_docache << " m_dir " << + (m_dir?m_dir->dirname():"(null)") << "\n"); if (m_docache) { std::unique_lock lock(o_cache.m_lock); delete o_cache.m_dir; @@ -133,4 +147,12 @@ Uncomp::~Uncomp() } } - +void Uncomp::clearcache() +{ + LOGDEB0("Uncomp::clearcache\n"); + std::unique_lock lock(o_cache.m_lock); + delete o_cache.m_dir; + o_cache.m_dir = 0; + o_cache.m_tfile.clear(); + o_cache.m_srcpath.clear(); +} diff --git a/src/internfile/uncomp.h b/src/internfile/uncomp.h index 1d2d2754..e8661077 100644 --- a/src/internfile/uncomp.h +++ b/src/internfile/uncomp.h @@ -27,10 +27,7 @@ /// Uncompression script interface. class Uncomp { public: - Uncomp(bool docache = false) - : m_dir(0), m_docache(docache) - { - } + explicit Uncomp(bool docache = false); ~Uncomp(); /** Uncompress the input file into a temporary one, by executing the @@ -41,25 +38,22 @@ public: bool uncompressfile(const std::string& ifn, const std::vector& cmdv, std::string& tfile); - + static void clearcache(); + private: - TempDir *m_dir; + TempDir *m_dir{0}; std::string m_tfile; std::string m_srcpath; bool m_docache; class UncompCache { public: - UncompCache() - : m_dir(0) - { - } - ~UncompCache() - { + UncompCache() {} + ~UncompCache() { delete m_dir; } std::mutex m_lock; - TempDir *m_dir; + TempDir *m_dir{0}; std::string m_tfile; std::string m_srcpath; }; diff --git a/src/qtgui/main.cpp b/src/qtgui/main.cpp index 61302b7b..e30c3ac9 100644 --- a/src/qtgui/main.cpp +++ b/src/qtgui/main.cpp @@ -42,6 +42,7 @@ #include "guiutils.h" #include "smallut.h" #include "readfile.h" +#include "uncomp.h" #include "recollq.h" @@ -52,10 +53,11 @@ static vector o_tempfiles; /* Keep an array of temporary files for deletion at exit. It happens that we erase some of them before exiting (ie: when closing a preview tab), we don't reuse the array holes for now */ -void rememberTempFile(TempFile temp) +TempFile *rememberTempFile(TempFile temp) { std::unique_lock locker(thetempfileslock); o_tempfiles.push_back(temp); + return &o_tempfiles.back(); } void forgetTempFile(string &fn) @@ -63,10 +65,9 @@ void forgetTempFile(string &fn) if (fn.empty()) return; std::unique_lock locker(thetempfileslock); - for (vector::iterator it = o_tempfiles.begin(); - it != o_tempfiles.end(); it++) { - if ((*it) && !fn.compare((*it)->filename())) { - it->reset(); + for (auto& entry : o_tempfiles) { + if (entry.ok() && !fn.compare(entry.filename())) { + entry = TempFile(); } } fn.erase(); @@ -76,11 +77,10 @@ void deleteAllTempFiles() { std::unique_lock locker(thetempfileslock); o_tempfiles.clear(); + Uncomp::clearcache(); } Rcl::Db *rcldb; - - int recollNeedsExit; RclMain *mainWindow; @@ -151,7 +151,6 @@ static void recollCleanup() deleteZ(theconfig); deleteAllTempFiles(); - LOGDEB2("recollCleanup: done\n" ); } diff --git a/src/qtgui/preview_w.cpp b/src/qtgui/preview_w.cpp index c2727649..dea8d520 100644 --- a/src/qtgui/preview_w.cpp +++ b/src/qtgui/preview_w.cpp @@ -832,15 +832,15 @@ bool Preview::loadDocInCurrentTab(const Rcl::Doc &idoc, int docnum) // an ipath, create it. if (fn.empty() || !idoc.ipath.empty()) { TempFile temp = lthr.tmpimg; - if (temp) { + if (temp.ok()) { LOGDEB1("Preview: load: got temp file from internfile\n"); } else if (!FileInterner::idocToFile(temp, string(), theconfig, idoc)) { - temp.reset(); // just in case. + temp = TempFile(); // just in case. } - if (temp) { + if (temp.ok()) { rememberTempFile(temp); - fn = temp->filename(); + fn = temp.filename(); editor->m_tmpfilename = fn; } else { editor->m_tmpfilename.erase(); diff --git a/src/qtgui/rclm_idx.cpp b/src/qtgui/rclm_idx.cpp index e2742a8a..deba8944 100644 --- a/src/qtgui/rclm_idx.cpp +++ b/src/qtgui/rclm_idx.cpp @@ -86,7 +86,7 @@ void RclMain::periodic100() bool exited = m_idxproc->maybereap(&status); if (exited) { QString reasonmsg; - if (m_idxreasontmp) { + if (m_idxreasontmp && m_idxreasontmp->ok()) { string reasons; file_to_string(m_idxreasontmp->filename(), reasons); if (!reasons.empty()) { @@ -227,11 +227,11 @@ bool RclMain::checkIdxPaths() // re-enabled by the indexing status check void RclMain::toggleIndexing() { - if (!m_idxreasontmp) { + if (!m_idxreasontmp || !m_idxreasontmp->ok()) { // We just store the pointer and let the tempfile cleaner deal // with delete on exiting - m_idxreasontmp = new TempFileInternal(".txt"); - rememberTempFile(TempFile(m_idxreasontmp)); + TempFile temp(".txt"); + m_idxreasontmp = rememberTempFile(temp); } switch (m_indexerState) { @@ -281,7 +281,7 @@ void RclMain::toggleIndexing() return; } vector args{"-c", theconfig->getConfDir()}; - if (m_idxreasontmp) { + if (m_idxreasontmp && m_idxreasontmp->ok()) { args.push_back("-R"); args.push_back(m_idxreasontmp->filename()); } @@ -377,7 +377,7 @@ void RclMain::rebuildIndex() } vector args{"-c", theconfig->getConfDir(), "-z"}; - if (m_idxreasontmp) { + if (m_idxreasontmp && m_idxreasontmp->ok()) { args.push_back("-R"); args.push_back(m_idxreasontmp->filename()); } @@ -458,7 +458,7 @@ void RclMain::specialIndex() return; vector args{"-c", theconfig->getConfDir()}; - if (m_idxreasontmp) { + if (m_idxreasontmp && m_idxreasontmp->ok()) { args.push_back("-R"); args.push_back(m_idxreasontmp->filename()); } @@ -521,7 +521,7 @@ void RclMain::updateIdxForDocs(vector& docs) vector paths; if (Rcl::docsToPaths(docs, paths)) { vector args{"-c", theconfig->getConfDir(), "-e", "-i"}; - if (m_idxreasontmp) { + if (m_idxreasontmp && m_idxreasontmp->ok()) { args.push_back("-R"); args.push_back(m_idxreasontmp->filename()); } diff --git a/src/qtgui/rclm_view.cpp b/src/qtgui/rclm_view.cpp index d07587b5..cb0ff2ee 100644 --- a/src/qtgui/rclm_view.cpp +++ b/src/qtgui/rclm_view.cpp @@ -313,7 +313,7 @@ void RclMain::startNativeViewer(Rcl::Doc doc, int pagenum, QString term) bool enterHistory = false; bool istempfile = false; - LOGDEB("RclMain::startNV: groksipath " << groksipath << " wantsf " << + LOGDEB("StartNativeViewer: groksipath " << groksipath << " wantsf " << wantsfile << " wantsparentf " << wantsparentfile << "\n"); // If the command wants a file but this is not a file url, or @@ -332,7 +332,7 @@ void RclMain::startNativeViewer(Rcl::Doc doc, int pagenum, QString term) enterHistory = true; istempfile = true; rememberTempFile(temp); - fn = temp->filename(); + fn = temp.filename(); url = path_pathtofileurl(fn); } @@ -354,10 +354,10 @@ void RclMain::startNativeViewer(Rcl::Doc doc, int pagenum, QString term) return; } } - if (temp) { + if (temp.ok()) { istempfile = true; rememberTempFile(temp); - fn = temp->filename(); + fn = temp.filename(); url = path_pathtofileurl(fn); } } diff --git a/src/qtgui/rclmain_w.h b/src/qtgui/rclmain_w.h index 682b334f..10aafb1c 100644 --- a/src/qtgui/rclmain_w.h +++ b/src/qtgui/rclmain_w.h @@ -204,7 +204,7 @@ private: vector m_viewers; ExecCmd *m_idxproc{0}; // Indexing process bool m_idxkilled{false}; // Killed my process - TempFileInternal *m_idxreasontmp{nullptr}; + TempFile *m_idxreasontmp{nullptr}; map m_stemLangToId; vector m_catgbutvec; int m_catgbutvecidx{0}; diff --git a/src/qtgui/recoll.h b/src/qtgui/recoll.h index c3fb1bd3..b7f03c74 100644 --- a/src/qtgui/recoll.h +++ b/src/qtgui/recoll.h @@ -36,7 +36,7 @@ bool getStemLangs(vector& langs); extern RclConfig *theconfig; -extern void rememberTempFile(TempFile); +extern TempFile *rememberTempFile(TempFile); extern void forgetTempFile(string &fn); extern void deleteAllTempFiles(); diff --git a/src/utils/rclutil.cpp b/src/utils/rclutil.cpp index 13cc1e54..d83a8214 100644 --- a/src/utils/rclutil.cpp +++ b/src/utils/rclutil.cpp @@ -43,6 +43,7 @@ #include "wipedir.h" #include "transcode.h" #include "md5ut.h" +#include "log.h" using namespace std; @@ -281,8 +282,51 @@ bool maketmpdir(string& tdir, string& reason) return true; } -TempFileInternal::TempFileInternal(const string& suffix) - : m_noremove(false) + +class TempFile::Internal { +public: + Internal(const std::string& suffix); + ~Internal(); + friend class TempFile; +private: + std::string m_filename; + std::string m_reason; + bool m_noremove{false}; +}; + +TempFile::TempFile(const string& suffix) + : m(new Internal(suffix)) +{ +} + +TempFile::TempFile() +{ + m = std::shared_ptr(); +} + +const char *TempFile::filename() const +{ + return m ? m->m_filename.c_str() : ""; +} + +const std::string& TempFile::getreason() const +{ + static string fatal{"fatal error"}; + return m ? m->m_reason : fatal; +} + +void TempFile::setnoremove(bool onoff) +{ + if (m) + m->m_noremove = onoff; +} + +bool TempFile::ok() const +{ + return m ? !m->m_filename.empty() : false; +} + +TempFile::Internal::Internal(const string& suffix) { // Because we need a specific suffix, can't use mkstemp // well. There is a race condition between name computation and @@ -322,7 +366,7 @@ TempFileInternal::TempFileInternal(const string& suffix) } } -TempFileInternal::~TempFileInternal() +TempFile::Internal::~Internal() { if (!m_filename.empty() && !m_noremove) { unlink(m_filename.c_str()); @@ -335,11 +379,13 @@ TempDir::TempDir() m_dirname.erase(); return; } + LOGDEB("TempDir::TempDir: -> " << m_dirname << endl); } TempDir::~TempDir() { if (!m_dirname.empty()) { + LOGDEB("TempDir::~TempDir: erasing " << m_dirname << endl); (void)wipedir(m_dirname, true, true); m_dirname.erase(); } diff --git a/src/utils/rclutil.h b/src/utils/rclutil.h index 106d69e2..5c52077c 100644 --- a/src/utils/rclutil.h +++ b/src/utils/rclutil.h @@ -48,30 +48,19 @@ extern const std::string& tmplocation(); extern bool maketmpdir(std::string& tdir, std::string& reason); /// Temporary file class -class TempFileInternal { +class TempFile { public: - TempFileInternal(const std::string& suffix); - ~TempFileInternal(); - const char *filename() { - return m_filename.c_str(); - } - const std::string& getreason() { - return m_reason; - } - void setnoremove(bool onoff) { - m_noremove = onoff; - } - bool ok() { - return !m_filename.empty(); - } + TempFile(const std::string& suffix); + TempFile(); + const char *filename() const; + const std::string& getreason() const; + void setnoremove(bool onoff); + bool ok() const; + class Internal; private: - std::string m_filename; - std::string m_reason; - bool m_noremove; + std::shared_ptr m; }; -typedef std::shared_ptr TempFile; - /// Temporary directory class. Recursively deleted by destructor. class TempDir { public: