TempFile: clean-up interface by using internal ref-counted class member. Uncomp: add interface to clear cache

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 && 
+	if (temp.ok() && 
-	    (result = df->set_document_file(m_mimetype, temp->filename()))) {
+	    (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<RecollFilter*>::iterator it = m_handlers.begin();
+    for (auto& entry: m_handlers) {
-	 it != m_handlers.end(); it++) {
+        returnMimeHandler(entry);
-        returnMimeHandler(*it);
     }
+    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)));
+    TempFile temp(m_cfg->getSuffixFromMimeType(mt));
-    if (!temp->ok()) {
+    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: " <<reason << "\n");
 	return TempFile();
     }
@@ -723,8 +725,8 @@ int FileInterner::addHandler()
 	setres = newflt->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 && 
+	if (temp.ok() && 
-	    (setres = newflt->set_document_file(mimetype, temp->filename()))) {
+	    (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(
+    TempFile temp(cnf->getSuffixFromMimeType(mimetype));
-                      cnf->getSuffixFromMimeType(mimetype)));
+    if (!temp.ok()) {
-    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 = 
+    temp = TempFile(cnf->getSuffixFromMimeType(doc.mimetype));
-      TempFile(new TempFileInternal(cnf->getSuffixFromMimeType(doc.mimetype)));
+    if (!temp.ok()) {
-    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;

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

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<string>& 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<std::mutex> 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<std::mutex> 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();
+}

src/internfile/uncomp.h

@@ -27,10 +27,7 @@
 /// Uncompression script interface.
 class Uncomp {
 public:
-    Uncomp(bool docache = false)
+    explicit Uncomp(bool docache = false);
-	: m_dir(0), m_docache(docache)
-    {
-    }
     ~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<std::string>& 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()
+	UncompCache() {}
-	    : m_dir(0)
+	~UncompCache() {
-	{
-	}
-	~UncompCache()
-	{
 	    delete m_dir;
 	}
         std::mutex m_lock;
-	TempDir *m_dir;
+	TempDir *m_dir{0};
 	std::string   m_tfile;
 	std::string   m_srcpath;
     };

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<TempFile>  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<std::mutex> 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<std::mutex> locker(thetempfileslock);
-    for (vector<TempFile>::iterator it = o_tempfiles.begin();
+    for (auto& entry : o_tempfiles) {
-         it != o_tempfiles.end(); it++) {
+        if (entry.ok() && !fn.compare(entry.filename())) {
-        if ((*it) && !fn.compare((*it)->filename())) {
+            entry = TempFile();
-            it->reset();
         }
     }
     fn.erase();
@@ -76,11 +77,10 @@ void deleteAllTempFiles()
 {
     std::unique_lock<std::mutex> 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" );
 }
 

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();

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");
+        TempFile temp(".txt");
-        rememberTempFile(TempFile(m_idxreasontmp));
+        m_idxreasontmp = rememberTempFile(temp);
     }
     
     switch (m_indexerState) {
@@ -281,7 +281,7 @@ void RclMain::toggleIndexing()
             return;
         }
         vector<string> 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<string> 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<string> 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<Rcl::Doc>& docs)
     vector<string> paths;
     if (Rcl::docsToPaths(docs, paths)) {
 	vector<string> 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());
         }

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);
         }
     }

src/qtgui/rclmain_w.h

@@ -204,7 +204,7 @@ private:
     vector<ExecCmd*>  m_viewers;
     ExecCmd          *m_idxproc{0}; // Indexing process
     bool             m_idxkilled{false}; // Killed my process
-    TempFileInternal *m_idxreasontmp{nullptr};
+    TempFile        *m_idxreasontmp{nullptr};
     map<QString, QAction*> m_stemLangToId;
     vector<string>    m_catgbutvec;
     int               m_catgbutvecidx{0};

src/qtgui/recoll.h

@@ -36,7 +36,7 @@ bool getStemLangs(vector<string>& langs);
 
 extern RclConfig *theconfig;
 
-extern void rememberTempFile(TempFile);
+extern TempFile *rememberTempFile(TempFile);
 extern void forgetTempFile(string &fn);
 extern void deleteAllTempFiles();
 

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<Internal>();
+}
+
+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();
     }

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);
+    TempFile(const std::string& suffix);
-    ~TempFileInternal();
+    TempFile();
-    const char *filename() {
+    const char *filename() const;
-        return m_filename.c_str();
+    const std::string& getreason() const;
-    }
+    void setnoremove(bool onoff);
-    const std::string& getreason() {
+    bool ok() const;
-        return m_reason;
+    class Internal;
-    }
-    void setnoremove(bool onoff) {
-        m_noremove = onoff;
-    }
-    bool ok() {
-        return !m_filename.empty();
-    }
 private:
-    std::string m_filename;
+    std::shared_ptr<Internal> m;
-    std::string m_reason;
-    bool m_noremove;
 };
 
-typedef std::shared_ptr<TempFileInternal> TempFile;
-
 /// Temporary directory class. Recursively deleted by destructor.
 class TempDir {
 public: