Commit 0d5712a4 authored by Greg Thompson's avatar Greg Thompson Committed by Chromium LUCI CQ

[Mac] Fix a crash following destruction of a FilePathWatcher.

FilePathWatcherKQueue::Watch uses a FileDescriptorWatcher, which
requires that its callers handle spurious runs of callbacks after the
watch is cancelled.

If this speculative fix resolves the crash, a follow-on CL will revise
the FileDescriptorWatcher documentation to make this case explicit.

BUG=1156603
R=fdoray@chromium.org

Change-Id: I7d5da3fb4e58cf30efab21963908b3ff41e9dc71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593379
Commit-Queue: Greg Thompson <grt@chromium.org>
Auto-Submit: Greg Thompson <grt@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837852}
parent 0cb92cb6
...@@ -299,12 +299,11 @@ bool FilePathWatcherKQueue::Watch(const FilePath& path, ...@@ -299,12 +299,11 @@ bool FilePathWatcherKQueue::Watch(const FilePath& path,
return false; return false;
} }
// It's safe to use Unretained() because the watch is cancelled and the // FileDescriptorWatcher may attempt to run its callback after the controller
// callback cannot be invoked after |kqueue_watch_controller_| (which is a // has been deleted, so bind to this via a WeakPtr.
// member of |this|) has been deleted.
kqueue_watch_controller_ = FileDescriptorWatcher::WatchReadable( kqueue_watch_controller_ = FileDescriptorWatcher::WatchReadable(
kqueue_, BindRepeating(&FilePathWatcherKQueue::OnKQueueReadable, kqueue_, BindRepeating(&FilePathWatcherKQueue::OnKQueueReadable,
Unretained(this))); weak_factory_.GetWeakPtr()));
return true; return true;
} }
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/files/file_path_watcher.h" #include "base/files/file_path_watcher.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h"
namespace base { namespace base {
...@@ -121,6 +122,8 @@ class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate { ...@@ -121,6 +122,8 @@ class FilePathWatcherKQueue : public FilePathWatcher::PlatformDelegate {
// data is available in |kqueue_|. // data is available in |kqueue_|.
std::unique_ptr<FileDescriptorWatcher::Controller> kqueue_watch_controller_; std::unique_ptr<FileDescriptorWatcher::Controller> kqueue_watch_controller_;
WeakPtrFactory<FilePathWatcherKQueue> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(FilePathWatcherKQueue); DISALLOW_COPY_AND_ASSIGN(FilePathWatcherKQueue);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment