QTemporaryFile: fix issues with removing a file twice

The assertion in isUnnamedFile() we had was incorrect after the file was
removed, since we cleared the name and possibly reset back to the
template. Since ~QTemporaryFile() calls remove(), this was easy to
trigger if you attempted to remove the temp file and leave
QTemporaryFile like that.

Take this opportunity to add to the docs of setAutoRemove() explaining
the possibility of unnamed files.

#7  0x00007f69bcc2b50e in qt_assert (
    assertion=assertion@entry=0x7f69bcf194a0 "unnamedFile == d_func()->fileEntry.isEmpty()",
    file=file@entry=0x7f69bcf19458 "io/qtemporaryfile.cpp",
    line=line@entry=514) at global/qglobal.cpp:3123
#8  0x00007f69bcd672cf in QTemporaryFileEngine::isUnnamedFile (this=this@entry=0x55cd60644df0)
    at io/qtemporaryfile.cpp:514
#9  0x00007f69bcd683f7 in QTemporaryFileEngine::remove (this=0x55cd60644df0)
    at io/qtemporaryfile.cpp:396
#10 0x00007f69bcd48654 in QFile::remove (this=this@entry=0x7fffb393f7e0)
    at io/qfile.cpp:513
#11 0x00007f69bcd6653b in QTemporaryFile::~QTemporaryFile (this=0x7fffb393f7e0, __in_chrg=<optimized out>)
    at io/qtemporaryfile.cpp:719

Change-Id: I57a1bd6e0c194530b732fffd14f4ed28ca8185b2
Reviewed-by: Andreas Hartmetz <ahartmetz@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

5 files changed, 43 insertions, 7 deletions

diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp
index bac995ff25..e4888e9523 100644
--- a/src/corelib/io/qfile.cpp
+++ b/src/corelib/io/qfile.cpp
@@ -503,7 +503,8 @@ bool
 QFile::remove()
 {
     Q_D(QFile);
-    if (d->fileName.isEmpty()) {
+    if (d->fileName.isEmpty() &&
+            !static_cast<QFSFileEngine *>(d->engine())->isUnnamedFile()) {
         qWarning("QFile::remove: Empty or null file name");
         return false;
     }
diff --git a/src/corelib/io/qfsfileengine_p.h b/src/corelib/io/qfsfileengine_p.h
index 1f34dfc2be..faef84cbe2 100644
--- a/src/corelib/io/qfsfileengine_p.h
+++ b/src/corelib/io/qfsfileengine_p.h
@@ -112,6 +112,9 @@ public:
     qint64 write(const char *data, qint64 len) Q_DECL_OVERRIDE;
     bool cloneTo(QAbstractFileEngine *target) override;
 
+    virtual bool isUnnamedFile() const
+    { return false; }
+
     bool extension(Extension extension, const ExtensionOption *option = 0, ExtensionReturn *output = 0) Q_DECL_OVERRIDE;
     bool supportsExtension(Extension extension) const Q_DECL_OVERRIDE;
 
diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp
index b8d3e859cf..35699d52df 100644
--- a/src/corelib/io/qtemporaryfile.cpp
+++ b/src/corelib/io/qtemporaryfile.cpp
@@ -393,7 +393,9 @@ bool QTemporaryFileEngine::remove()
     // we must explicitly call QFSFileEngine::close() before we remove it.
     d->unmapAll();
     QFSFileEngine::close();
-    if (isUnnamedFile() || QFSFileEngine::remove()) {
+    if (isUnnamedFile())
+        return true;
+    if (!filePathIsTemplate && QFSFileEngine::remove()) {
         d->fileEntry.clear();
         // If a QTemporaryFile is constructed using a template file path, the path
         // is generated in QTemporaryFileEngine::open() and then filePathIsTemplate
@@ -511,7 +513,10 @@ bool QTemporaryFileEngine::materializeUnnamedFile(const QString &newName, QTempo
 bool QTemporaryFileEngine::isUnnamedFile() const
 {
 #ifdef LINUX_UNNAMED_TMPFILE
-    Q_ASSERT(unnamedFile == d_func()->fileEntry.isEmpty());
+    if (unnamedFile) {
+        Q_ASSERT(d_func()->fileEntry.isEmpty());
+        Q_ASSERT(filePathIsTemplate);
+    }
     return unnamedFile;
 #else
     return false;
@@ -749,10 +754,21 @@ bool QTemporaryFile::autoRemove() const
 }
 
 /*!
-    Sets the QTemporaryFile into auto-remove mode if \a b is true.
+    Sets the QTemporaryFile into auto-remove mode if \a b is \c true.
 
     Auto-remove is on by default.
 
+    If you set this property to \c false, ensure the application provides a way
+    to remove the file once it is no longer needed, including passing the
+    responsibility on to another process. Always use the fileName() function to
+    obtain the name and never try to guess the name that QTemporaryFile has
+    generated.
+
+    On some systems, if fileName() is not called before closing the file, the
+    temporary file may be removed regardless of the state of this property.
+    This behavior should not be relied upon, so application code should either
+    call fileName() or leave the auto removal functionality enabled.
+
     \sa autoRemove(), remove()
 */
 void QTemporaryFile::setAutoRemove(bool b)
diff --git a/src/corelib/io/qtemporaryfile_p.h b/src/corelib/io/qtemporaryfile_p.h
index f74e5680b9..c3a2c01790 100644
--- a/src/corelib/io/qtemporaryfile_p.h
+++ b/src/corelib/io/qtemporaryfile_p.h
@@ -140,7 +140,7 @@ public:
 
     enum MaterializationMode { Overwrite, DontOverwrite, NameIsTemplate };
     bool materializeUnnamedFile(const QString &newName, MaterializationMode mode);
-    bool isUnnamedFile() const;
+    bool isUnnamedFile() const override final;
 
     const QString &templateName;
     quint32 fileMode;
diff --git a/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp b/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp
index f3ce902bbd..2d87c2193b 100644
--- a/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp
+++ b/tests/auto/corelib/io/qtemporaryfile/tst_qtemporaryfile.cpp
@@ -70,6 +70,7 @@ private slots:
     void io();
     void openCloseOpenClose();
     void removeAndReOpen();
+    void removeUnnamed();
     void size();
     void resize();
     void openOnRootDrives();
@@ -442,11 +443,13 @@ void tst_QTemporaryFile::removeAndReOpen()
     {
         QTemporaryFile file;
         file.open();
-        fileName = file.fileName();
+        fileName = file.fileName();     // materializes any unnamed file
         QVERIFY(QFile::exists(fileName));
 
-        file.remove();
+        QVERIFY(file.remove());
+        QVERIFY(file.fileName().isEmpty());
         QVERIFY(!QFile::exists(fileName));
+        QVERIFY(!file.remove());
 
         QVERIFY(file.open());
         QCOMPARE(QFileInfo(file.fileName()).path(), QFileInfo(fileName).path());
@@ -456,6 +459,19 @@ void tst_QTemporaryFile::removeAndReOpen()
     QVERIFY(!QFile::exists(fileName));
 }
 
+void tst_QTemporaryFile::removeUnnamed()
+{
+    QTemporaryFile file;
+    file.open();
+
+    // we did not call fileName(), so the file name may not have a name
+    QVERIFY(file.remove());
+    QVERIFY(file.fileName().isEmpty());
+
+    // if it was unnamed, this will succeed again, so we can't check the result
+    file.remove();
+}
+
 void tst_QTemporaryFile::size()
 {
     QTemporaryFile file;