Fixed a resource (file descriptor) leak: If a QFile object was created from a C Runtime file descriptor, it will *not* close the file, when QFile::close() is called or when the QFile object is destroyed. Furthermore, if a C Runtime file descriptor is obtained via _open_osfhandle(), we *must* to close that file descriptor via _close() function. Closing the underlying system HANDLE via CloseHandle() is *not* sufficient, as the file descriptor is won't be freed! On the other hand, the _close() function *does* close the underlying system HANDLE for us. In order to avoid a resource (file descriptor) leak, we will now store the file descriptor and close it properly via _close() in the destructor.

Note: The resource leak probably did not cause any harm, simply because we did not create many LockedFile objects. But it *did* cause serious issues when running the benchmark. Anyway, it should be fixed now.

doc/Changelog.html

@@ -27,6 +27,7 @@ a:visited { color: #0000EE; }
 <li>Updated mpg123 decoder to v1.16.0 (2013-10-06), compiled with GCC 4.8.1
 <li>Updated GNU Wget binary to v1.14.0 (2012-08-05), compiled with GCC 4.8.1
 <li>Updated GnuPG to v1.4.15 (2013-10-05), compiled with GCC 4.8.1
+<li>Fixed a resource (file descriptor) leak in "static" builds, didn't cause much harm though
 <li>Various bugfixes and code improvements
 </ul><br>
 

src/LockedFile.cpp

@@ -44,9 +44,9 @@
 
 // WARNING: Passing file descriptors into Qt does NOT work with dynamically linked CRT!
 #ifdef QT_NODLL
-	static const bool g_useFileDescr = 1;
+	static const bool g_useFileDescrForQFile = 1;
 #else
-	static const bool g_useFileDescr = 0;
+	static const bool g_useFileDescrForQFile = 0;
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -86,9 +86,13 @@ QByteArray LockedFile::fileHash(QFile &file)
 
 ///////////////////////////////////////////////////////////////////////////////
 
-LockedFile::LockedFile(QResource *const resource, const QString &outPath, const QByteArray &expectedHash)
+LockedFile::LockedFile(QResource *const resource, const QString &outPath, const QByteArray &expectedHash, const bool bOwnsFile)
+:
+	m_bOwnsFile(bOwnsFile),
+	m_filePath(QFileInfo(outPath).absoluteFilePath())
 {
-	m_fileHandle = NULL;
+	m_fileDescriptor = -1;
+	HANDLE fileHandle = NULL;
 		
 	//Make sure the resource is valid
 	if(!(resource->isValid() && resource->data()))
@@ -97,7 +101,6 @@ LockedFile::LockedFile(QResource *const resource, const QString &outPath, const
 	}
 
 	QFile outFile(outPath);
-	m_filePath = QFileInfo(outFile).absoluteFilePath();
 	
 	//Open output file
 	for(int i = 0; i < 64; i++)
@@ -115,71 +118,84 @@ LockedFile::LockedFile(QResource *const resource, const QString &outPath, const
 			QFile::remove(QFileInfo(outFile).canonicalFilePath());
 			THROW_FMT("File '%s' could not be written!", QUTF8(QFileInfo(outFile).fileName()));
 		}
-		outFile.close();
 	}
 	else
 	{
 		THROW_FMT("File '%s' could not be created!", QUTF8(QFileInfo(outFile).fileName()));
 	}
 
+	//Close file after it has been written
+	outFile.close();
+
 	//Now lock the file!
 	for(int i = 0; i < 64; i++)
 	{
-		m_fileHandle = CreateFileW(QWCHAR(QDir::toNativeSeparators(m_filePath)), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, NULL, NULL);
+		fileHandle = CreateFileW(QWCHAR(QDir::toNativeSeparators(m_filePath)), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, NULL, NULL);
-		if((m_fileHandle != NULL) && (m_fileHandle != INVALID_HANDLE_VALUE)) break;
+		if((fileHandle != NULL) && (fileHandle != INVALID_HANDLE_VALUE)) break;
 		if(!i) qWarning("Failed to lock file on first attemp, retrying...");
 		Sleep(100);
 	}
 	
 	//Locked successfully?
-	if((m_fileHandle == NULL) || (m_fileHandle == INVALID_HANDLE_VALUE))
+	if((fileHandle == NULL) || (fileHandle == INVALID_HANDLE_VALUE))
 	{
 		QFile::remove(QFileInfo(outFile).canonicalFilePath());
 		THROW_FMT("File '%s' could not be locked!", QUTF8(QFileInfo(outFile).fileName()));
 	}
 
-	//Open file for reading
+	//Get file descriptor
-	if(g_useFileDescr)
+	m_fileDescriptor = _open_osfhandle(reinterpret_cast<intptr_t>(fileHandle), _O_RDONLY | _O_BINARY);
+	if(m_fileDescriptor < 0)
 	{
-		int fd = _open_osfhandle(reinterpret_cast<intptr_t>(m_fileHandle), _O_RDONLY | _O_BINARY);
+		THROW_FMT("Failed to obtain C Runtime file descriptor!");
-		if(fd >= 0) outFile.open(fd, QIODevice::ReadOnly);
 	}
-	else
+
+	QFile checkFile;
+
+	//Now re-open the file for reading
+	for(int i = 0; i < 64; i++)
 	{
-		for(int i = 0; i < 64; i++)
+		if(g_useFileDescrForQFile)
 		{
-			if(outFile.open(QIODevice::ReadOnly)) break;
+			if(checkFile.open(m_fileDescriptor, QIODevice::ReadOnly)) break;
-			if(!i) qWarning("Failed to re-open file on first attemp, retrying...");
-			Sleep(100);
 		}
+		else
+		{
+			if(checkFile.open(QIODevice::ReadOnly)) break;
+		}
+		if(!i) qWarning("Failed to re-open file on first attemp, retrying...");
+		Sleep(100);
+	}
+
+	//Opened successfully
+	if(!checkFile.isOpen())
+	{
+		QFile::remove(m_filePath);
+		THROW_FMT("File '%s' could not be read!", QUTF8(QFileInfo(checkFile).fileName()));
 	}
 
 	//Verify file contents
-	QByteArray hash;
+	const QByteArray hash = fileHash(checkFile);
-	if(outFile.isOpen())
+	checkFile.close();
-	{
-		hash = fileHash(outFile);
-		outFile.close();
-	}
-	else
-	{
-		QFile::remove(m_filePath);
-		THROW_FMT("File '%s' could not be read!", QUTF8(QFileInfo(outFile).fileName()));
-	}
 
 	//Compare hashes
 	if(hash.isNull() || _stricmp(hash.constData(), expectedHash.constData()))
 	{
 		qWarning("\nFile checksum error:\n A = %s\n B = %s\n", expectedHash.constData(), hash.constData());
-		LAMEXP_CLOSE(m_fileHandle);
+		LAMEXP_CLOSE(fileHandle);
 		QFile::remove(m_filePath);
-		THROW_FMT("File '%s' is corruputed, take care!", QUTF8(QFileInfo(outFile).fileName()));
+		THROW_FMT("File '%s' is corruputed, take care!", QUTF8(QFileInfo(checkFile).fileName()));
 	}
 }
 
-LockedFile::LockedFile(const QString &filePath)
+LockedFile::LockedFile(const QString &filePath, const bool bOwnsFile)
+:
+	m_bOwnsFile(bOwnsFile),
+	m_filePath(QFileInfo(filePath).canonicalPath())
 {
-	m_fileHandle = NULL;
+	m_fileDescriptor = -1;
+	HANDLE fileHandle = NULL;
+
 	QFileInfo existingFile(filePath);
 	existingFile.setCaching(false);
 	
@@ -189,28 +205,47 @@ LockedFile::LockedFile(const QString &filePath)
 		THROW_FMT("File '%s' does not exist!", QUTF8(existingFile.fileName()));
 	}
 	
-	//Remember file path
-	m_filePath = existingFile.canonicalFilePath();
-
 	//Now lock the file
 	for(int i = 0; i < 64; i++)
 	{
-		m_fileHandle = CreateFileW(QWCHAR(QDir::toNativeSeparators(filePath)), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, NULL, NULL);
+		fileHandle = CreateFileW(QWCHAR(QDir::toNativeSeparators(filePath)), GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, NULL, NULL);
-		if((m_fileHandle != NULL) && (m_fileHandle != INVALID_HANDLE_VALUE)) break;
+		if((fileHandle != NULL) && (fileHandle != INVALID_HANDLE_VALUE)) break;
 		if(!i) qWarning("Failed to lock file on first attemp, retrying...");
 		Sleep(100);
 	}
 
 	//Locked successfully?
-	if((m_fileHandle == NULL) || (m_fileHandle == INVALID_HANDLE_VALUE))
+	if((fileHandle == NULL) || (fileHandle == INVALID_HANDLE_VALUE))
 	{
 		THROW_FMT("File '%s' could not be locked!", QUTF8(existingFile.fileName()));
 	}
+
+	//Get file descriptor
+	m_fileDescriptor = _open_osfhandle(reinterpret_cast<intptr_t>(fileHandle), _O_RDONLY | _O_BINARY);
+	if(m_fileDescriptor < 0)
+	{
+		THROW_FMT("Failed to obtain C Runtime file descriptor!");
+	}
 }
 
 LockedFile::~LockedFile(void)
 {
-	LAMEXP_CLOSE(m_fileHandle);
+	if(m_fileDescriptor >= 0)
+	{
+		_close(m_fileDescriptor);
+		m_fileDescriptor = -1;
+	}
+	if(m_bOwnsFile)
+	{
+		if(QFileInfo(m_filePath).exists())
+		{
+			for(int i = 0; i < 64; i++)
+			{
+				if(QFile::remove(m_filePath)) break;
+				lamexp_sleep(1);
+			}
+		}
+	}
 }
 
 const QString &LockedFile::filePath()

src/LockedFile.h

@@ -30,8 +30,8 @@ class QFile;
 class LockedFile
 {
 public:
-	LockedFile(QResource *const resource, const QString &outPath, const QByteArray &expectedHash = QByteArray());
+	LockedFile(QResource *const resource, const QString &outPath, const QByteArray &expectedHash = QByteArray(), const bool bOwnsFile = true);
-	LockedFile(const QString &filePath);
+	LockedFile(const QString &filePath, const bool bOwnsFile = false);
 	~LockedFile(void);
 
 	const QString &filePath();
@@ -40,6 +40,7 @@ public:
 	static QByteArray fileHash(QFile &file);
 
 private:
-	QString m_filePath;
+	const bool m_bOwnsFile;
-	void *m_fileHandle;
+	const QString m_filePath;
+	int m_fileDescriptor;
 };