From 0f9e98a5861326dfe57fc42dd298ca66b183bf63 Mon Sep 17 00:00:00 2001 From: Robin Burchell Date: Sat, 17 Mar 2012 23:28:48 +0100 Subject: [PATCH] Fix directories never being unwatched after deletion on Windows. This causes all sorts of problems, but is also blocking the introduction of new, more detailed signals, because the backend never correctly identified the removal. The object handle appears to be woken up before the directory is actually deleted, thus causing QFileInfo::exists() to return true, and not doing the removal dance. This behaviour isn't exactly documented (as far as I was able to find out), but also seems to happen consistently, and Chromium also contains a comment noting a similar issue. Task-number: QTBUG-2331 Change-Id: Icfb6219b78e688852d7863a666a0ffc31bb4d573 Reviewed-by: Friedemann Kleint --- src/corelib/io/qfilesystemwatcher_win.cpp | 15 ++++++++++- .../tst_qfilesystemwatcher.cpp | 27 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/corelib/io/qfilesystemwatcher_win.cpp b/src/corelib/io/qfilesystemwatcher_win.cpp index 715e16bc7e..5dc72251ca 100644 --- a/src/corelib/io/qfilesystemwatcher_win.cpp +++ b/src/corelib/io/qfilesystemwatcher_win.cpp @@ -361,8 +361,18 @@ void QWindowsFileSystemWatcherEngineThread::run() if (handles.contains(handle)) { // qDebug()<<"thread"< &h = pathInfoForHandle[handle]; + bool fakeRemove = false; + if (!FindNextChangeNotification(handle)) { const DWORD error = GetLastError(); + + if (error == ERROR_ACCESS_DENIED) { + // for directories, our object's handle appears to be woken up when the target of a + // watch is deleted, before the watched thing is actually deleted... + // anyway.. we're given an error code of ERROR_ACCESS_DENIED in that case. + fakeRemove = true; + } + qErrnoWarning(error, "%s", qPrintable(msgFindNextFailed(h))); } QMutableHashIterator it(h); @@ -371,7 +381,10 @@ void QWindowsFileSystemWatcherEngineThread::run() QString absolutePath = x.value().absolutePath; QFileInfo fileInfo(x.value().path); // qDebug() << "checking" << x.key(); - if (!fileInfo.exists()) { + + // i'm not completely sure the fileInfo.exist() check will ever work... see QTBUG-2331 + // ..however, I'm not completely sure enough to remove it. + if (fakeRemove || !fileInfo.exists()) { // qDebug() << x.key() << "removed!"; if (x.value().isDir) emit directoryChanged(x.value().path, true); diff --git a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp index 8caccb6da9..78f229a740 100644 --- a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp +++ b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp @@ -84,6 +84,9 @@ private slots: void destroyAfterQCoreApplication(); + void QTBUG2331(); + void QTBUG2331_data() { basicTest_data(); } + private: QString m_tempDirPattern; }; @@ -556,5 +559,29 @@ void tst_QFileSystemWatcher::destroyAfterQCoreApplication() QTest::qWait(30); } +// regression test for QTBUG2331. +// essentially, on windows, directories were not unwatched after being deleted +// from the disk, causing all sorts of interesting problems. +void tst_QFileSystemWatcher::QTBUG2331() +{ + QFETCH(QString, backend); + + QTemporaryDir temporaryDirectory(m_tempDirPattern); + QVERIFY(temporaryDirectory.isValid()); + QFileSystemWatcher watcher; + watcher.setObjectName(QLatin1String("_qt_autotest_force_engine_") + backend); + QVERIFY(watcher.addPath(temporaryDirectory.path())); + + // watch signal + QSignalSpy changedSpy(&watcher, SIGNAL(directoryChanged(QString))); + QVERIFY(changedSpy.isValid()); + + // remove directory, we should get one change signal, and we should no longer + // be watching the directory. + QVERIFY(temporaryDirectory.remove()); + QTRY_COMPARE(changedSpy.count(), 1); + QCOMPARE(watcher.directories(), QStringList()); +} + QTEST_MAIN(tst_QFileSystemWatcher) #include "tst_qfilesystemwatcher.moc"