Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1162)

Unified Diff: base/directory_watcher_inotify.cc

Issue 149630: Fix two races in DirectoryWatcherInotify: (Closed)
Patch Set: Created 11 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/directory_watcher_inotify.cc
diff --git a/base/directory_watcher_inotify.cc b/base/directory_watcher_inotify.cc
index 98475697f095e0f06f18a1c4f7041cd07b6c02e4..44e4dcb36087c02b5d08b61363ffa21df40cc26e 100644
--- a/base/directory_watcher_inotify.cc
+++ b/base/directory_watcher_inotify.cc
@@ -331,18 +331,17 @@ void InotifyReader::OnInotifyEvent(const inotify_event* event) {
if (event->mask & IN_IGNORED)
return;
- WatcherSet watchers_to_notify;
- FilePath changed_path;
-
- {
- AutoLock auto_lock(lock_);
- changed_path = paths_[event->wd];
- watchers_to_notify.insert(watchers_[event->wd].begin(),
- watchers_[event->wd].end());
- }
+ // In case you want to limit the scope of this lock, it's not sufficient
+ // to just copy things under the lock, and then run the notifications
+ // without holding the lock. DirectoryWatcherImpl's dtor removes its watches,
+ // and to do that obtains the lock. After it finishes removing watches,
+ // it's destroyed. So, if you copy under the lock and notify without the lock,
+ // it's possible you'll copy the DirectoryWatcherImpl which is being
+ // destroyed, then it will destroy itself, and then you'll try to notify it.
+ AutoLock auto_lock(lock_);
- for (WatcherSet::iterator watcher = watchers_to_notify.begin();
- watcher != watchers_to_notify.end();
+ for (WatcherSet::iterator watcher = watchers_[event->wd].begin();
+ watcher != watchers_[event->wd].end();
++watcher) {
(*watcher)->OnInotifyEvent(event);
}
@@ -438,16 +437,13 @@ bool DirectoryWatcherImpl::Watch(const FilePath& path,
if (!file_util::GetInode(path, &inode))
return false;
- InotifyReader::Watch watch =
- Singleton<InotifyReader>::get()->AddWatch(path, this);
- if (watch == InotifyReader::kInvalidWatch)
- return false;
-
delegate_ = delegate;
recursive_ = recursive;
root_path_ = path;
- watch_ = watch;
loop_ = MessageLoop::current();
+ watch_ = Singleton<InotifyReader>::get()->AddWatch(path, this);
+ if (watch_ == InotifyReader::kInvalidWatch)
+ return false;
{
AutoLock auto_lock(lock_);
@@ -473,4 +469,3 @@ bool DirectoryWatcherImpl::Watch(const FilePath& path,
DirectoryWatcher::DirectoryWatcher() {
impl_ = new DirectoryWatcherImpl();
}
-
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698