Re-add method to get a temporary file name without persistent QTemporaryFile object

This is needed for cases where the temp file has to be manually removed
at some point (e.g., if temp files are used as conversion target, and
the initial file only serves as a placeholder), since QTemporaryFile
objects cannot be manually removed at least on Windows (they are always
kept open internally even after close()). See
​http://lists.qt-project.org/pipermail/interest/2013-August/008352.html

In order to avoid race conditions due to duplicate names (the issue why
the old method was removed), we record all used temp file names.

Fixes: #9139
(cherry picked from commit 9e2928be68992161a54287d153e1e9431e30bb4c)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index ffe522e0f9845a7838526037969df4aedc4856bf..cd49a683d79707f1785f86896ded7367d2bc0035 100644 (file)
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1090,8 +1090,7 @@ bool Buffer::importString(string const & format, docstring const & contents, Err
                return false;
        // It is important to use the correct extension here, since some
        // converters create a wrong output file otherwise (e.g. html2latex)
-       TempFile const tempfile("Buffer_importStringXXXXXX." + fmt->extension());
-       FileName const name(tempfile.name());
+       FileName const name = tempFileName("Buffer_importStringXXXXXX." + fmt->extension());
        ofdocstream os(name.toFilesystemEncoding().c_str());
        // Do not convert os implicitly to bool, since that is forbidden in C++11.
        bool const success = !(os << contents).fail();
@@ -1107,8 +1106,7 @@ bool Buffer::importString(string const & format, docstring const & contents, Err
                converted = importFile(format, name, errorList);
        }
 
-       if (name.exists())
-               name.removeFile();
+       removeTempFile(name);
        return converted;
 }
 
@@ -1118,10 +1116,12 @@ bool Buffer::importFile(string const & format, FileName const & name, ErrorList
        if (!theConverters().isReachable(format, "lyx"))
                return false;
 
-       TempFile const tempfile("Buffer_importFileXXXXXX.lyx");
-       FileName const lyx(tempfile.name());
-       if (theConverters().convert(0, name, lyx, name, format, "lyx", errorList))
-               return readFile(lyx) == ReadSuccess;
+       FileName const lyx = tempFileName("Buffer_importFileXXXXXX.lyx");
+       if (theConverters().convert(0, name, lyx, name, format, "lyx", errorList)) {
+               bool const success = readFile(lyx) == ReadSuccess;
+               removeTempFile(lyx);
+               return success;
+       }
 
        return false;
 }

diff --git a/src/support/filetools.cpp b/src/support/filetools.cpp
index 12457cb3ae328a4c2c679887b4313d1949356286..bcfd1b6c9ea741c41910c475a5213849eb505496 100644 (file)
--- a/src/support/filetools.cpp
+++ b/src/support/filetools.cpp
@@ -26,6 +26,7 @@
 
 #include "support/filetools.h"
 
+#include "support/convert.h"
 #include "support/debug.h"
 #include "support/environment.h"
 #include "support/gettext.h"
@@ -36,6 +37,7 @@
 #include "support/PathChanger.h"
 #include "support/Systemcall.h"
 #include "support/qstring_helpers.h"
+#include "support/TempFile.h"
 
 #include <QDir>
 #include <QTemporaryFile>
@@ -458,6 +460,51 @@ string const commandPrep(string const & command_in)
 }
 
 
+FileName const tempFileName(string const & mask)
+{
+       FileName tempfile = TempFile(mask).name();
+       // Since the QTemporaryFile object is destroyed at function return
+       // (which is what is intended here), the next call to this function
+       // may return the same file name again.
+       // Thus, in order to prevent race conditions, we track returned names
+       // and create our own unique names if QTemporaryFile returns a name again.
+       if (tmp_names_.find(tempfile.absFileName()) == tmp_names_.end()) {
+               tmp_names_.insert(tempfile.absFileName());
+               return tempfile;
+       }
+
+       // OK, we need another name. Simply append digits.
+       FileName tmp = tempfile;
+       tmp.changeExtension("");
+       for (int i = 1; i < INT_MAX ;++i) {
+               // Append digit to filename and re-add extension
+               string const new_fn = tmp.absFileName() + convert<string>(i)
+                               + "." + tempfile.extension();
+               if (tmp_names_.find(new_fn) == tmp_names_.end()) {
+                       tmp_names_.insert(new_fn);
+                       tempfile.set(new_fn);
+                       return tempfile;
+               }
+       }
+
+       // This should not happen!
+       LYXERR0("tempFileName(): Could not create unique temp file name!");
+       return tempfile;
+}
+
+
+void removeTempFile(FileName const & fn)
+{
+       if (!fn.exists())
+               return;
+
+       string const abs = fn.absFileName();
+       if (tmp_names_.find(abs) != tmp_names_.end())
+               tmp_names_.erase(abs);
+       fn.removeFile();
+}
+
+
 static string createTempFile(QString const & mask)
 {
        // FIXME: This is not safe. QTemporaryFile creates a file in open(),
@@ -466,7 +513,7 @@ static string createTempFile(QString const & mask)
        //        same file again. To make this safe the QTemporaryFile object
        //        needs to be kept for the whole life time of the temp file name.
        //        This could be achieved by creating a class TempDir (like
-       //        TempFile, but using a currentlky non-existing
+       //        TempFile, but using a currently non-existing
        //        QTemporaryDirectory object).
        QTemporaryFile qt_tmp(mask + ".XXXXXXXXXXXX");
        if (qt_tmp.open()) {

diff --git a/src/support/filetools.h b/src/support/filetools.h
index 0146474d0715d903e69c00fbffac1eb09bf8cae1..3a4c2c566d3031bd87a62517761b4b9c5d419636 100644 (file)
--- a/src/support/filetools.h
+++ b/src/support/filetools.h
@@ -16,12 +16,35 @@
 
 #include <utility>
 #include <string>
+#include <set>
 
 namespace lyx {
 namespace support {
 
 class FileName;
 
+/// Record used temp file names
+static std::set<std::string> tmp_names_;
+
+/// Get a temporary file name.
+/**
+* The actual temp file (QTemporaryFile object) is immediately
+* destroyed after the name has been generated, so a new file
+* has to be created manually from the name.
+* This is needed if the temp file has to be manually removed
+* (e.g., when temp files are used as conversion target, and the initial
+* file only serves as a placeholder), since QTemporaryFile objects
+* cannot be manually removed at least on Windows (they are always
+* kept open internally even after close()).
+* In order to avoid race conditions due to duplicate names, we record
+* all used temp file names.
+* If you don't have to remove the temp file manually, use TempFile instead!
+*/
+FileName const tempFileName(std::string const &);
+
+/// Remove and unregister a temporary file.
+void removeTempFile(FileName const &);
+
 /** Creates the global LyX temp dir.
   \p deflt can be an existing directory name. In this case a new directory
   inside \p deflt is created. If \p deflt does not exist yet, \p deflt is