Add class for threadsafe temp file handling

FileName::tempName() is not thread safe, since the QTemporaryFile object is
immediately deleted after creating it. Therefore, another thread could create
the same temporary file in the time span before the user of FileName::tempName()
recreates it. This is not as theoretical as it may look: I observed that
repeated creation and deletion of QTemporaryFile objects always use the same
name.
This problem is solved by the new class TempFile which should be used like
QTemporaryFile itself.

diff --git a/src/support/FileName.cpp b/src/support/FileName.cpp
index 3cc18bc9ae0b286716e32dd3e1bf30220462cf19..74b3c95316150c7ae2f4ca7e07dbacdeac442d62 100644 (file)
--- a/src/support/FileName.cpp
+++ b/src/support/FileName.cpp
@@ -442,6 +442,7 @@ static string createTempFile(QString const & mask)
        //        Therefore the next call to createTempFile() may create the
        //        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 can be achieved by using the TempFile class.
        QTemporaryFile qt_tmp(mask);
        if (qt_tmp.open()) {
                string const temp_file = fromqstr(qt_tmp.fileName());

diff --git a/src/support/FileName.h b/src/support/FileName.h
index e1bc0de83d45a3b9e06e57d114a4d4eef40ec951..83d9e946f26898c9d092e87a49471bdeb6f1c072 100644 (file)
--- a/src/support/FileName.h
+++ b/src/support/FileName.h
@@ -170,10 +170,12 @@ public:
        void changeExtension(std::string const & extension);
 
        static FileName fromFilesystemEncoding(std::string const & name);
-       /// (securely) create a temporary file with the given mask.
+       /// Create a temporary file with the given mask.
        /// \p mask must be in filesystem encoding, if it contains a
        /// relative path, the template file will be created in the global
        /// temporary directory as given by 'package().temp_dir()'.
+       /// CAUTION: This method may create race conditions.
+       ///          Do not use, use the TempFile class instead.
        static FileName tempName(std::string const & mask);
        static FileName tempName(FileName const & temp_dir,
                std::string const & mask);

diff --git a/src/support/Makefile.am b/src/support/Makefile.am
index 5ab3ef99eecb5c2204ab08099743ac19776fafdc..dfe4d0ec71d808e1b2420b2e433e3555d9068b90 100644 (file)
--- a/src/support/Makefile.am
+++ b/src/support/Makefile.am
@@ -94,6 +94,8 @@ liblyxsupport_a_SOURCES = \
        Systemcall.h \
        SystemcallPrivate.h \
        shared_ptr.h \
+       TempFile.cpp \
+       TempFile.h \
        textutils.h \
        Translator.h \
        Timeout.cpp \

diff --git a/src/support/TempFile.cpp b/src/support/TempFile.cpp
new file mode 100644 (file)
index 0000000..5c4b389
--- /dev/null
+++ b/src/support/TempFile.cpp
@@ -0,0 +1,78 @@
+/**
+ * \file TempFile.cpp
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Georg Baum
+ *
+ * Full author contact details are available in file CREDITS.
+ */
+
+#include <config.h>
+
+#include "support/TempFile.h"
+
+#include "support/debug.h"
+#include "support/FileName.h"
+#include "support/Package.h"
+#include "support/qstring_helpers.h"
+
+#include <QFileInfo>
+#include <QDir>
+#include <QTemporaryFile>
+
+using namespace std;
+
+namespace lyx {
+namespace support {
+
+struct TempFile::Private
+{
+       ///
+       Private(QString const & mask) : f(mask)
+       {
+               LYXERR(Debug::FILES, "Temporary file in " << fromqstr(mask));
+               if (f.open())
+                       LYXERR(Debug::FILES, "Temporary file `"
+                              << fromqstr(f.fileName()) << "' created.");
+               else
+                       LYXERR(Debug::FILES, "Unable to create temporary file with following template: "
+                              << f.fileTemplate());
+       }
+
+       ///
+       QTemporaryFile f;
+};
+
+
+TempFile::TempFile(FileName const & temp_dir, string const & mask)
+{
+       QFileInfo tmp_fi(QDir(toqstr(temp_dir.absoluteFilePath())), toqstr(mask));
+       d = new Private(tmp_fi.absoluteFilePath());
+}
+
+
+TempFile::TempFile(string const & mask)
+{
+       QFileInfo tmp_fi(QDir(toqstr(package().temp_dir().absoluteFilePath())), toqstr(mask));
+       d = new Private(tmp_fi.absoluteFilePath());
+}
+
+
+TempFile::~TempFile()
+{
+       delete d;
+}
+
+
+FileName TempFile::name() const
+{
+       QString const n = d->f.fileName();
+       if (n.isNull())
+               return FileName();
+       return FileName(fromqstr(n));
+}
+
+
+} // namespace support
+} // namespace lyx

diff --git a/src/support/TempFile.h b/src/support/TempFile.h
new file mode 100644 (file)
index 0000000..2adbb9f
--- /dev/null
+++ b/src/support/TempFile.h
@@ -0,0 +1,58 @@
+// -*- C++ -*-
+/**
+ * \file TempFile.h
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Georg Baum
+ *
+ * Full author contact details are available in file CREDITS.
+ */
+
+#ifndef TEMPFILE_H
+#define TEMPFILE_H
+
+#include <string>
+
+namespace lyx {
+namespace support {
+
+class FileName;
+
+/**
+ * Class for safely creating temporary files without race conditions.
+ * The file is created in the constructor, and deleted in the destructor.
+ * You may do anything with the file (including deletion), but the instance
+ * of this class must stay alive as long as the file is needed.
+ */
+class TempFile {
+public:
+       /**
+        *Create a temporary file with the given mask.
+        * \p mask must be in filesystem encoding, if it contains a
+        * relative path, the template file will be created in the global
+        * temporary directory as given by 'package().temp_dir()'.
+        * If the mask contains "XXXXXX" this portion will be replaced by
+        * a uniquely generetd string. If it does not contain this portion,
+        * it will be automatically appended using a dot. Therefore, please
+        * specify the "XXXXXX" portion if the extension of the generated
+        * name is important (e.g. for the converter machinery).
+        */
+       TempFile(std::string const & mask);
+       TempFile(FileName const & temp_dir, std::string const & mask);
+       ~TempFile();
+       /**
+        * Get the name of the temporary file.
+        * This is empty if the file could not be created.
+        */
+       FileName name() const;
+private:
+       ///
+       struct Private;
+       Private * d;
+};
+
+} // namespace support
+} // namespace lyx
+
+#endif