commit 388a2846886d87e1e2a9ec0cd19803a2490d3d2d
Author: Juergen Spitzmueller <sp...@lyx.org>
Date:   Thu Feb 8 11:31:23 2018 +0100

    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)
---
 src/Buffer.cpp            |   16 +++++++-------
 src/support/filetools.cpp |   49 ++++++++++++++++++++++++++++++++++++++++++++-
 src/support/filetools.h   |   23 +++++++++++++++++++++
 3 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index ffe522e..cd49a68 100644
--- 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 12457cb..bcfd1b6 100644
--- 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 0146474..3a4c2c5 100644
--- 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

Reply via email to