From 42e2070925d012845db9cf85e597a17851a0dcb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= <joao@abecasis.name>
Date: Wed, 6 May 2009 16:12:57 +0200
Subject: Fix leak of file descriptors in QTemporaryFile

Using setFileName in QFile::copy (introduced recently) has a nasty
side-effect of leaking file descriptors in QTemporaryFile. This happens
because the code assumes the file has been closed. In QTemporaryFile,
we need to explicitly call native file engine close.

Test case by Thiago. Bug report from Arora developers.

Reviewed-by: thiago
---
 src/corelib/io/qtemporaryfile.cpp                |  9 +++++
 tests/auto/qtemporaryfile/qtemporaryfile.pro     |  3 +-
 tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp | 45 ++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp
index 4856353..6a9125c 100644
--- a/src/corelib/io/qtemporaryfile.cpp
+++ b/src/corelib/io/qtemporaryfile.cpp
@@ -294,6 +294,8 @@ public:
     QTemporaryFileEngine(const QString &file) : QFSFileEngine(file) { }
     ~QTemporaryFileEngine();
 
+    void setFileName(const QString &file);
+
     bool open(QIODevice::OpenMode flags);
     bool remove();
     bool close();
@@ -304,6 +306,13 @@ QTemporaryFileEngine::~QTemporaryFileEngine()
     QFSFileEngine::close();
 }
 
+void QTemporaryFileEngine::setFileName(const QString &file)
+{
+    // Really close the file, so we don't leak
+    QFSFileEngine::close();
+    QFSFileEngine::setFileName(file);
+}
+
 bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)
 {
     Q_D(QFSFileEngine);
diff --git a/tests/auto/qtemporaryfile/qtemporaryfile.pro b/tests/auto/qtemporaryfile/qtemporaryfile.pro
index 0c90994..bde990a 100644
--- a/tests/auto/qtemporaryfile/qtemporaryfile.pro
+++ b/tests/auto/qtemporaryfile/qtemporaryfile.pro
@@ -2,5 +2,4 @@ load(qttest_p4)
 SOURCES       += tst_qtemporaryfile.cpp
 QT = core
 
-
-
+DEFINES += SRCDIR=\\\"$$PWD/\\\"
diff --git a/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp b/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp
index f5155ae..2daa0f6 100644
--- a/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp
+++ b/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp
@@ -51,6 +51,13 @@
 #if defined(Q_OS_WIN)
 # include <windows.h>
 #endif
+#if defined(Q_OS_UNIX)
+# include <sys/types.h>
+# include <sys/stat.h>
+# include <errno.h>
+# include <fcntl.h>             // open(2)
+# include <unistd.h>            // close(2)
+#endif
 
 //TESTED_CLASS=
 //TESTED_FILES=
@@ -78,6 +85,7 @@ private slots:
     void openOnRootDrives();
     void stressTest();
     void rename();
+    void renameFdLeak();
 public:
 };
 
@@ -356,5 +364,42 @@ void tst_QTemporaryFile::rename()
     QVERIFY(!dir.exists("temporary-file.txt"));
 }
 
+void tst_QTemporaryFile::renameFdLeak()
+{
+#ifdef Q_OS_UNIX
+    // Test this on Unix only
+
+    // Open a bunch of files to force the fd count to go up
+    static const int count = 10;
+    int bunch_of_files[count];
+    for (int i = 0; i < count; ++i) {
+        bunch_of_files[i] = ::open(SRCDIR "tst_qtemporaryfile.cpp", O_RDONLY);
+        QVERIFY(bunch_of_files[i] != -1);
+    }
+
+    int fd;
+    {
+        QTemporaryFile file;
+        file.setAutoRemove(false);
+        QVERIFY(file.open());
+
+        // close the bunch of files
+        for (int i = 0; i < count; ++i)
+            ::close(bunch_of_files[i]);
+
+        // save the file descriptor for later
+        fd = file.handle();
+
+        // rename the file to something
+        QString newPath = QDir::tempPath() + "/tst_qtemporaryfile-renameFdLeak-" + QString::number(getpid());
+        file.rename(newPath);
+        QFile::remove(newPath);
+    }
+
+    // check if QTemporaryFile closed the file
+    QVERIFY(::close(fd) == -1 && errno == EBADF);
+#endif
+}
+
 QTEST_MAIN(tst_QTemporaryFile)
 #include "tst_qtemporaryfile.moc"
-- 
cgit v0.12