Commit 42e2070925d012845db9cf85e597a17851a0dcb6

João Abecasis
Fri May 08 11:28:34 GMT 2009

Tree SHA1: 5b1ab55
Parent SHA1: f64bcba (compile?)
raw diff | raw patch

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

Diff rendering mode:
inline
side by side


294	294	QTemporaryFileEngine(const QString &file) : QFSFileEngine(file) { }
295	295	~QTemporaryFileEngine();
296	296
	297	void setFileName(const QString &file);
	298
297	299	bool open(QIODevice::OpenMode flags);
298	300	bool remove();
299	301	bool close();
…	…
304	304	QTemporaryFileEngine::~QTemporaryFileEngine()
305	305	{
306	306	QFSFileEngine::close();
	307	}
	308
	309	void QTemporaryFileEngine::setFileName(const QString &file)
	310	{
	311	// Really close the file, so we don't leak
	312	QFSFileEngine::close();
	313	QFSFileEngine::setFileName(file);
307	314	}
308	315
309	316	bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)


1	1	load(qttest_p4)
2	2	SOURCES += tst_qtemporaryfile.cpp
3	3	QT = core
	4
	5	DEFINES += SRCDIR=\\\"$$PWD/\\\"


51	51	#if defined(Q_OS_WIN)
52	52	# include <windows.h>
53	53	#endif
	54	#if defined(Q_OS_UNIX)
	55	# include <sys/types.h>
	56	# include <sys/stat.h>
	57	# include <errno.h>
	58	# include <fcntl.h> // open(2)
	59	# include <unistd.h> // close(2)
	60	#endif
54	61
55	62	//TESTED_CLASS=
56	63	//TESTED_FILES=
…	…
85	85	void openOnRootDrives();
86	86	void stressTest();
87	87	void rename();
	88	void renameFdLeak();
88	89	public:
89	90	};
90	91
…	…
362	362
363	363	QVERIFY(!dir.exists(tempname));
364	364	QVERIFY(!dir.exists("temporary-file.txt"));
	365	}
	366
	367	void tst_QTemporaryFile::renameFdLeak()
	368	{
	369	#ifdef Q_OS_UNIX
	370	// Test this on Unix only
	371
	372	// Open a bunch of files to force the fd count to go up
	373	static const int count = 10;
	374	int bunch_of_files[count];
	375	for (int i = 0; i < count; ++i) {
	376	bunch_of_files[i] = ::open(SRCDIR "tst_qtemporaryfile.cpp", O_RDONLY);
	377	QVERIFY(bunch_of_files[i] != -1);
	378	}
	379
	380	int fd;
	381	{
	382	QTemporaryFile file;
	383	file.setAutoRemove(false);
	384	QVERIFY(file.open());
	385
	386	// close the bunch of files
	387	for (int i = 0; i < count; ++i)
	388	::close(bunch_of_files[i]);
	389
	390	// save the file descriptor for later
	391	fd = file.handle();
	392
	393	// rename the file to something
	394	QString newPath = QDir::tempPath() + "/tst_qtemporaryfile-renameFdLeak-" + QString::number(getpid());
	395	file.rename(newPath);
	396	QFile::remove(newPath);
	397	}
	398
	399	// check if QTemporaryFile closed the file
	400	QVERIFY(::close(fd) == -1 && errno == EBADF);
	401	#endif
365	402	}
366	403
367	404	QTEST_MAIN(tst_QTemporaryFile)

Commit 42e2070925d012845db9cf85e597a17851a0dcb6

Comments

Add a new comment:

Add your comment

Gitorious