From 79542a7196ebdef5f33b80f8619571f7aa3c6134 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 17 Feb 2014 23:55:22 +0000 Subject: [PATCH] Fix crash in wxMSW wxFileSystemWatcher when removing the same path twice. Starting to watch a path, stopping to watch it, starting to watch it again and stopping again resulted in a crash in wxMSW wxFileSystemWatcher implementation because the watcher object wasn't kept artificially kept alive when it was stopped for the second time. This happened because our way of keeping it alive was to store it in a hash map indexed by path, but if a watcher for the same path (added there when this path was first unwatched) was already present in the map, the watcher wasn't added to it and not kept alive. Fix this by using a vector instead of a map. We obviously sacrifice quick access to it by path but at least this doesn't crash any more. And we could actually still use a map, just indexed by the (unique) pointer to the object stored inside wxSharedPtr itself, and not its path. But a vector might be a more efficient data structure in practice, if we keep it from becoming too big as we should try to do by triggering artificial port completions when a watch is removed. At any rate, at least the crash is fixed for now. Closes #15995. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/branches/WX_3_0_BRANCH@75915 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- docs/changes.txt | 1 + include/wx/msw/private/fswatcher.h | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 5c10df8f30..a97a63b588 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -629,6 +629,7 @@ wxMSW: - Fix bug with multiple default buttons in a dialog (Artur Wieczorek). - Improve tooltips wrapping after updating their text (Artur Wieczorek). - Fix checking menu items before appending them to the menu. +- Fix crash when adding/removing the same path to/from wxFileSystemWatcher. wxOSX: diff --git a/include/wx/msw/private/fswatcher.h b/include/wx/msw/private/fswatcher.h index 86f91dc3d0..fdbeef68a3 100644 --- a/include/wx/msw/private/fswatcher.h +++ b/include/wx/msw/private/fswatcher.h @@ -173,7 +173,7 @@ public: // worse, reused to point to another object) pointer in ReadEvents() so // just remember that this one should be removed when CompleteRemoval() // is called later. - m_removedWatches.insert(wxFSWatchEntries::value_type(path, watch)); + m_removedWatches.push_back(watch); m_watches.erase(it); return true; @@ -185,15 +185,20 @@ public: // this case we'll just return false and do nothing. bool CompleteRemoval(wxFSWatchEntryMSW* watch) { - wxFSWatchEntries::iterator it = m_removedWatches.find(watch->GetPath()); - if ( it == m_removedWatches.end() ) - return false; + for ( Watches::iterator it = m_removedWatches.begin(); + it != m_removedWatches.end(); + ++it ) + { + if ( (*it).get() == watch ) + { + // Removing the object from here will result in deleting the + // watch itself as it's not referenced from anywhere else now. + m_removedWatches.erase(it); + return true; + } + } - // Removing the object from the map will result in deleting the watch - // itself as it's not referenced from anywhere else now. - m_removedWatches.erase(it); - - return true; + return false; } // post completion packet @@ -249,7 +254,8 @@ protected: wxFSWatchEntries m_watches; // Contains the watches which had been removed but are still pending. - wxFSWatchEntries m_removedWatches; + typedef wxVector< wxSharedPtr > Watches; + Watches m_removedWatches; };