Fixed shutdown concurrency issues in FilePathWatcher.

content/common/file_path_watcher/file_path_watcher_inotify.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/file_path_watcher/file_path_watcher.h"
 
 #include <errno.h>
 #include <string.h>
 #include <sys/inotify.h>
 #include <sys/ioctl.h>
@@ skipping 62 matching lines @@
 
   // Use self-pipe trick to unblock select during shutdown.
   int shutdown_pipe_[2];
 
   // Flag set to true when startup was successful.
   bool valid_;
 
   DISALLOW_COPY_AND_ASSIGN(InotifyReader);
 };
 
-class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate {
+class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
+                            public MessageLoop::DestructionObserver {
  public:
   FilePathWatcherImpl();
 
   // Called for each event coming from the watch. |fired_watch| identifies the
   // watch that fired, |child| indicates what has changed, and is relative to
   // the currently watched path for |fired_watch|. The flag |created| is true if
   // the object appears, and |is_directory| is set when the event refers to a
   // directory.
   void OnFilePathChanged(InotifyReader::Watch fired_watch,
                          const FilePath::StringType& child,
                          bool created,
                          bool is_directory);
 
   // Start watching |path| for changes and notify |delegate| on each change.
   // Returns true if watch for |path| has been added successfully.
   virtual bool Watch(const FilePath& path,
                      FilePathWatcher::Delegate* delegate,
                      base::MessageLoopProxy* loop) OVERRIDE;
 
   // Cancel the watch. This unregisters the instance with InotifyReader.
   virtual void Cancel() OVERRIDE;
 
+  // Deletion of the FilePathWatcher will call Cancel() to dispose of this
+  // object in the right thread. This also observes destruction of the required
+  // cleanup thread, in case it quits before Cancel() is called.
+  virtual void WillDestroyCurrentMessageLoop() OVERRIDE;
+
  private:
   virtual ~FilePathWatcherImpl() {}
 
+  // Cleans up and stops observing the |message_loop_| thread.
+  void CancelOnMessageLoopThread() OVERRIDE;
+
   // Inotify watches are installed for all directory components of |target_|. A
   // WatchEntry instance holds the watch descriptor for a component and the
   // subdirectory for that identifies the next component.
   struct WatchEntry {
     WatchEntry(InotifyReader::Watch watch, const FilePath::StringType& subdir)
         : watch_(watch),
           subdir_(subdir) {}
 
     InotifyReader::Watch watch_;
     FilePath::StringType subdir_;
@@ skipping 237 matching lines @@
 
 bool FilePathWatcherImpl::Watch(const FilePath& path,
                                 FilePathWatcher::Delegate* delegate,
                                 base::MessageLoopProxy*) {
   DCHECK(target_.empty());
   DCHECK(MessageLoopForIO::current());
 
   set_message_loop(base::MessageLoopProxy::CreateForCurrentThread());
   delegate_ = delegate;
   target_ = path;
+  MessageLoop::current()->AddDestructionObserver(this);
+
   std::vector<FilePath::StringType> comps;
   target_.GetComponents(&comps);
   DCHECK(!comps.empty());
   for (std::vector<FilePath::StringType>::const_iterator comp(++comps.begin());
        comp != comps.end(); ++comp) {
     watches_.push_back(WatchEntry(InotifyReader::kInvalidWatch, *comp));
   }
   watches_.push_back(WatchEntry(InotifyReader::kInvalidWatch,
                                 FilePath::StringType()));
   return UpdateWatches();
 }
 
 void FilePathWatcherImpl::Cancel() {
-  if (!message_loop().get()) {
-    // Watch was never called, so exit.
+  if (!delegate_) {
+    // Watch was never called, or the |message_loop_| thread is already gone.
+    set_cancelled();
     return;
   }
 
   // Switch to the message_loop_ if necessary so we can access |watches_|.
   if (!message_loop()->BelongsToCurrentThread()) {
     message_loop()->PostTask(FROM_HERE,
-        NewRunnableMethod(this, &FilePathWatcherImpl::Cancel));
-    return;
+                             new FilePathWatcher::CancelTask(this));
+  } else {
+    CancelOnMessageLoopThread();
   }
+}
 
-  for (WatchVector::iterator watch_entry(watches_.begin());
-       watch_entry != watches_.end(); ++watch_entry) {
-    if (watch_entry->watch_ != InotifyReader::kInvalidWatch)
-      g_inotify_reader.Get().RemoveWatch(watch_entry->watch_, this);
+void FilePathWatcherImpl::CancelOnMessageLoopThread() {
+  if (!is_cancelled()) {
+    set_cancelled();
+    MessageLoop::current()->RemoveDestructionObserver(this);
+
+    for (WatchVector::iterator watch_entry(watches_.begin());
+         watch_entry != watches_.end(); ++watch_entry) {
+      if (watch_entry->watch_ != InotifyReader::kInvalidWatch)
+        g_inotify_reader.Get().RemoveWatch(watch_entry->watch_, this);
+    }
+    watches_.clear();
+    delegate_ = NULL;
+    target_.clear();
   }
-  watches_.clear();
-  delegate_ = NULL;
-  target_.clear();
+}
+
+void FilePathWatcherImpl::WillDestroyCurrentMessageLoop() {
+  CancelOnMessageLoopThread();
 }
 
 bool FilePathWatcherImpl::UpdateWatches() {
   // Ensure this runs on the message_loop_ exclusively in order to avoid
   // concurrency issues.
   DCHECK(message_loop()->BelongsToCurrentThread());
 
   // Walk the list of watches and update them as we go.
   FilePath path(FILE_PATH_LITERAL("/"));
   bool path_valid = true;
@@ skipping 16 matching lines @@
   }
 
   return true;
 }
 
 }  // namespace
 
 FilePathWatcher::FilePathWatcher() {
   impl_ = new FilePathWatcherImpl();
 }