Make wxFileSystemWatcher watch entries reference-counted.

This helps to avoid problems that arise from watching the same physical file
system path multiple times, which could happen when adding a watch for a path
already watched because of a recursive watch on a parent directory, for
example.

Closes #14490.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@72679 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/include/wx/fswatcher.h b/include/wx/fswatcher.h
index 7c6b63ef39db17584f47af9ff130482c239cfdc9..30656714ef6683b92d126f81106a436a8b6b8408 100644 (file)
--- a/include/wx/fswatcher.h
+++ b/include/wx/fswatcher.h
@@ -196,7 +196,7 @@ class wxFSWatchInfo
 {
 public:
     wxFSWatchInfo() :
-        m_events(-1), m_type(wxFSWPath_None)
+        m_events(-1), m_type(wxFSWPath_None), m_refcount(-1)
     {
     }
 
@@ -204,7 +204,8 @@ public:
                   int events,
                   wxFSWPathType type,
                   const wxString& filespec = wxString()) :
-        m_path(path), m_filespec(filespec), m_events(events), m_type(type)
+        m_path(path), m_filespec(filespec), m_events(events), m_type(type),
+        m_refcount(1)
     {
     }
 
@@ -225,11 +226,27 @@ public:
         return m_type;
     }
 
+    // Reference counting of watch entries is used to avoid watching the same
+    // file system path multiple times (this can happen even accidentally, e.g.
+    // when you have a recursive watch and then decide to watch some file or
+    // directory under it separately).
+    int IncRef()
+    {
+        return ++m_refcount;
+    }
+
+    int DecRef()
+    {
+        wxASSERT_MSG( m_refcount > 0, wxS("Trying to decrement a zero count") );
+        return --m_refcount;
+    }
+
 protected:
     wxString m_path;
     wxString m_filespec;      // For tree watches, holds any filespec to apply
     int m_events;
     wxFSWPathType m_type;
+    int m_refcount;
 };
 
 WX_DECLARE_STRING_HASH_MAP(wxFSWatchInfo, wxFSWatchInfoMap);

diff --git a/include/wx/private/fswatcher.h b/include/wx/private/fswatcher.h
index e106e6c926fcccc0f5ab4d9126ab837e4daa0750..abdc8e244c9ade20e01068480fac2297bd22761d 100644 (file)
--- a/include/wx/private/fswatcher.h
+++ b/include/wx/private/fswatcher.h
@@ -49,8 +49,13 @@ public:
 
     virtual bool Add(const wxFSWatchInfo& winfo)
     {
-        wxCHECK_MSG( m_watches.find(winfo.GetPath()) == m_watches.end(), false,
-                     "Path '%s' is already watched");
+        if ( m_watches.find(winfo.GetPath()) != m_watches.end() )
+        {
+            wxLogTrace(wxTRACE_FSWATCHER,
+                       "Path '%s' is already watched", winfo.GetPath());
+            // This can happen if a dir is watched, then a parent tree added
+            return true;
+        }
 
         // construct watch entry
         wxSharedPtr<wxFSWatchEntry> watch(new wxFSWatchEntry(winfo));
@@ -66,8 +71,13 @@ public:
     virtual bool Remove(const wxFSWatchInfo& winfo)
     {
         wxFSWatchEntries::iterator it = m_watches.find(winfo.GetPath());
-        wxCHECK_MSG( it != m_watches.end(), false, "Path '%s' is not watched");
-
+        if ( it == m_watches.end() )
+        {
+            wxLogTrace(wxTRACE_FSWATCHER,
+                       "Path '%s' is not watched", winfo.GetPath());
+            // This can happen if a dir is watched, then a parent tree added
+            return true;
+        }
         wxSharedPtr<wxFSWatchEntry> watch = it->second;
         m_watches.erase(it);
         return DoRemove(watch);

diff --git a/src/common/fswatchercmn.cpp b/src/common/fswatchercmn.cpp
index 28aeffdcb68dc9cb7c4988008b3fee75000cc631..b994758b60af80c9fc696bf063aa6ec0662aaf0c 100644 (file)
--- a/src/common/fswatchercmn.cpp
+++ b/src/common/fswatchercmn.cpp
@@ -108,17 +108,28 @@ wxFileSystemWatcherBase::AddAny(const wxFileName& path,
     if (canonical.IsEmpty())
         return false;
 
-    wxCHECK_MSG(m_watches.find(canonical) == m_watches.end(), false,
-                wxString::Format("Path '%s' is already watched", canonical));
-
     // adding a path in a platform specific way
     wxFSWatchInfo watch(canonical, events, type, filespec);
     if ( !m_service->Add(watch) )
         return false;
 
-    // on success, add path to our 'watch-list'
-    wxFSWatchInfoMap::value_type val(canonical, watch);
-    return m_watches.insert(val).second;
+    // on success, either add path to our 'watch-list'
+    // or, if already watched, inc the refcount. This may happen if
+    // a dir is Add()ed, then later AddTree() is called on a parent dir
+    wxFSWatchInfoMap::iterator it = m_watches.find(canonical);
+    if ( it == m_watches.end() )
+    {
+        wxFSWatchInfoMap::value_type val(canonical, watch);
+        m_watches.insert(val).second;
+    }
+    else
+    {
+        wxFSWatchInfo& watch = it->second;
+        int count = watch.IncRef();
+        wxLogTrace(wxTRACE_FSWATCHER,
+                   "'%s' is now watched %d times", canonical, count);
+    }
+    return true;
 }
 
 bool wxFileSystemWatcherBase::Remove(const wxFileName& path)
@@ -132,12 +143,17 @@ bool wxFileSystemWatcherBase::Remove(const wxFileName& path)
     wxCHECK_MSG(it != m_watches.end(), false,
                 wxString::Format("Path '%s' is not watched", canonical));
 
-    // remove from watch-list
-    wxFSWatchInfo watch = it->second;
-    m_watches.erase(it);
+    // Decrement the watch's refcount and remove from watch-list if 0
+    bool ret = true;
+    wxFSWatchInfo& watch = it->second;
+    if ( !watch.DecRef() )
+    {
+        // remove in a platform specific way
+        ret = m_service->Remove(watch);
 
-    // remove in a platform specific way
-    return m_service->Remove(watch);
+        m_watches.erase(it);
+    }
+    return ret;
 }
 
 bool wxFileSystemWatcherBase::AddTree(const wxFileName& path, int events,

diff --git a/tests/fswatcher/fswatchertest.cpp b/tests/fswatcher/fswatchertest.cpp
index 2cf5554dbbe4774ae7dd6a8a4acf312e203aa01c..59cbfb852e6ef33f9a2e21959939fbd98fc42256 100644 (file)
--- a/tests/fswatcher/fswatchertest.cpp
+++ b/tests/fswatcher/fswatchertest.cpp
@@ -653,6 +653,9 @@ void FileSystemWatcherTestCase::TestTrees()
         void GrowTree(wxFileName dir)
         {
             CPPUNIT_ASSERT(dir.Mkdir());
+            // Now add a subdir with an easy name to remember in WatchTree()
+            dir.AppendDir("child");
+            CPPUNIT_ASSERT(dir.Mkdir());
 
             // Create a branch of 5 numbered subdirs, each containing 3
             // numbered files
@@ -711,7 +714,7 @@ void FileSystemWatcherTestCase::TestTrees()
             // When there's no file mask, wxMSW sets a single watch
             // on the trunk which is implemented recursively.
             // wxGTK always sets an additional watch for each file/subdir
-            treeitems += (subdirs*files) + subdirs;
+            treeitems += (subdirs*files) + subdirs + 1; // +1 for 'child'
 #endif // __WINDOWS__
 
             // Store the initial count; there may already be some watches
@@ -726,6 +729,27 @@ void FileSystemWatcherTestCase::TestTrees()
 
             m_watcher->RemoveTree(dir);
             CPPUNIT_ASSERT_EQUAL(initial, m_watcher->GetWatchedPathsCount());
+
+            // Now test the refcount mechanism by watching items more than once
+            wxFileName child(dir);
+            child.AppendDir("child");
+            m_watcher->AddTree(child);
+            // Check some watches were added; we don't care about the number
+            CPPUNIT_ASSERT(initial < m_watcher->GetWatchedPathsCount());
+            // Now watch the whole tree and check that the count is the same
+            // as it was the first time, despite also adding 'child' separately
+            // Except that in wxMSW this isn't true: each watch will be a
+            // single, recursive dir; so fudge the count
+            size_t fudge = 0;
+#ifdef __WINDOWS__
+            fudge = 1;
+#endif // __WINDOWS__
+            m_watcher->AddTree(dir);
+            CPPUNIT_ASSERT_EQUAL(plustree + fudge, m_watcher->GetWatchedPathsCount());
+            m_watcher->RemoveTree(child);
+            CPPUNIT_ASSERT(initial < m_watcher->GetWatchedPathsCount());
+            m_watcher->RemoveTree(dir);
+            CPPUNIT_ASSERT_EQUAL(initial, m_watcher->GetWatchedPathsCount());
         }
 
         void WatchTreeWithFilespec(const wxFileName& dir)