runtime/bin/file_system_watcher_macos.cc - Issue 140183004: Move all run-loop calls into the run-loop thread, and use correct memory handling (retain/release).

Unified Diff: runtime/bin/file_system_watcher_macos.cc

Issue 140183004: Move all run-loop calls into the run-loop thread, and use correct memory handling (retain/release). (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Patch Set: Created 6 years, 10 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

Index: runtime/bin/file_system_watcher_macos.cc

diff --git a/runtime/bin/file_system_watcher_macos.cc b/runtime/bin/file_system_watcher_macos.cc

index 2fa80a29ba9efd692fad312dbfbe731dcd7ac1fa..652e9325dd17d81391c15c4cda9387deb42b9e67 100644

--- a/runtime/bin/file_system_watcher_macos.cc

+++ b/runtime/bin/file_system_watcher_macos.cc

@@ -55,18 +55,46 @@ class FSEventsWatcher {

public:

class Node {

public:

- Node(FSEventsWatcher* watcher, intptr_t base_path_length, int read_fd,

+ Node(FSEventsWatcher* watcher, char* base_path, int read_fd,

int write_fd, bool recursive)

: watcher_(watcher),

ready_(false),

- base_path_length_(base_path_length),

+ base_path_length_(strlen(base_path)),

+ path_ref_(CFStringCreateWithCString(

+ NULL, base_path, kCFStringEncodingUTF8)),

read_fd_(read_fd),

write_fd_(write_fd),

recursive_(recursive),

- ref_(NULL) {}

+ ref_(NULL) {

Søren Gjesse 2014/02/07 10:20:02 Please keep this code in the Start method and call

Anders Johnsen 2014/02/07 12:49:24 Done.

+ CFRunLoopTimerContext context;

+ memset(&context, 0, sizeof(context));

+ context.info = this;

Søren Gjesse 2014/02/07 10:20:02 Please add a comment that this is scheduling Start

Anders Johnsen 2014/02/07 12:49:24 Done.

+ CFRunLoopTimerRef timer = CFRunLoopTimerCreate(

+ NULL, 0, 0, 0, 0, Node::StartCallback, &context);

+ CFRunLoopAddTimer(watcher_->run_loop_, timer, kCFRunLoopCommonModes);

+ CFRelease(timer);

+ watcher_->monitor_.Enter();

+ while (!ready_) {

+ watcher_->monitor_.Wait(Monitor::kNoTimeout);

+ }

+ watcher_->monitor_.Exit();

+ }

~Node() {

Søren Gjesse 2014/02/07 10:20:02 I am not a great fan of placing this code in the d

Anders Johnsen 2014/02/07 12:49:24 Done.

+ ASSERT(ready_);

+ CFRunLoopTimerContext context;

+ memset(&context, 0, sizeof(context));

+ context.info = this;

Søren Gjesse 2014/02/07 10:20:02 Please add a comment that this is scheduling StopC

Anders Johnsen 2014/02/07 12:49:24 Done.

+ CFRunLoopTimerRef timer = CFRunLoopTimerCreate(

+ NULL, 0, 0, 0, 0, StopCallback, &context);

+ CFRunLoopAddTimer(watcher_->run_loop_, timer, kCFRunLoopCommonModes);

+ watcher_->monitor_.Enter();

+ while (ready_) {

+ watcher_->monitor_.Wait(Monitor::kNoTimeout);

+ }

+ watcher_->monitor_.Exit();

close(write_fd_);

+ CFRelease(path_ref_);

}

void set_ref(FSEventStreamRef ref) {

@@ -75,33 +103,41 @@ class FSEventsWatcher {

static void StartCallback(CFRunLoopTimerRef timer, void* info) {

Node* node = reinterpret_cast<Node*>(info);

+ FSEventStreamContext context;

+ context.version = 0;

+ context.info = reinterpret_cast<void*>(node);

+ context.retain = NULL;

+ context.release = NULL;

+ context.copyDescription = NULL;

+ CFArrayRef array = CFArrayCreate(

+ NULL, reinterpret_cast<const void**>(&node->path_ref_), 1, NULL);

+ FSEventStreamRef ref = FSEventStreamCreate(

+ NULL,

+ Callback,

+ &context,

+ array,

+ kFSEventStreamEventIdSinceNow,

+ 0.10,

+ kFSEventStreamCreateFlagNoDefer | kFSEventStreamCreateFlagFileEvents);

+ CFRelease(array);

+ node->set_ref(ref);

FSEventStreamScheduleWithRunLoop(

node->ref_,

node->watcher_->run_loop_,

kCFRunLoopDefaultMode);

FSEventStreamStart(node->ref_);

FSEventStreamFlushSync(node->ref_);

node->watcher_->monitor_.Enter();

node->ready_ = true;

node->watcher_->monitor_.Notify();

node->watcher_->monitor_.Exit();

}

- void Start() {

- ASSERT(!ready_);

- CFRunLoopTimerContext context;

- memset(&context, 0, sizeof(context));

- context.info = this;

- CFRunLoopTimerRef timer = CFRunLoopTimerCreate(

- NULL, 0, 0, 0, 0, StartCallback, &context);

- CFRunLoopAddTimer(watcher_->run_loop_, timer, kCFRunLoopCommonModes);

- watcher_->monitor_.Enter();

- while (!ready_) {

- watcher_->monitor_.Wait(Monitor::kNoTimeout);

- }

- watcher_->monitor_.Exit();

- }

static void StopCallback(CFRunLoopTimerRef timer, void* info) {

Node* node = reinterpret_cast<Node*>(info);

FSEventStreamStop(node->ref_);

@@ -113,21 +149,6 @@ class FSEventsWatcher {

node->watcher_->monitor_.Exit();

}

- void Stop() {

- ASSERT(ready_);

- CFRunLoopTimerContext context;

- memset(&context, 0, sizeof(context));

- context.info = this;

- CFRunLoopTimerRef timer = CFRunLoopTimerCreate(

- NULL, 0, 0, 0, 0, StopCallback, &context);

- CFRunLoopAddTimer(watcher_->run_loop_, timer, kCFRunLoopCommonModes);

- watcher_->monitor_.Enter();

- while (ready_) {

- watcher_->monitor_.Wait(Monitor::kNoTimeout);

- }

- watcher_->monitor_.Exit();

- }

bool ready() const { return ready_; }

intptr_t base_path_length() const { return base_path_length_; }

int read_fd() const { return read_fd_; }

@@ -138,6 +159,7 @@ class FSEventsWatcher {

FSEventsWatcher* watcher_;

bool ready_;

intptr_t base_path_length_;

+ CFStringRef path_ref_;

int read_fd_;

int write_fd_;

bool recursive_;

@@ -148,8 +170,29 @@ class FSEventsWatcher {

Thread::Start(Run, reinterpret_cast<uword>(this));

}

+ static void StopCallback(CFRunLoopTimerRef timer, void* info) {

+ FSEventsWatcher* watcher = reinterpret_cast<FSEventsWatcher*>(info);

+ CFRunLoopStop(watcher->run_loop_);

+ CFRelease(watcher->run_loop_);

+ watcher->monitor_.Enter();

+ watcher->run_loop_ = NULL;

+ watcher->monitor_.Notify();

+ watcher->monitor_.Exit();

+ }

~FSEventsWatcher() {

Søren Gjesse 2014/02/07 10:20:02 Please move this to a Stop method and call that be

Anders Johnsen 2014/02/07 12:49:24 Done.

- CFRunLoopStop(run_loop_);

+ CFRunLoopTimerContext context;

+ memset(&context, 0, sizeof(context));

+ context.info = this;

+ CFRunLoopTimerRef timer = CFRunLoopTimerCreate(

+ NULL, 0, 0, 0, 0, StopCallback, &context);

+ CFRunLoopAddTimer(run_loop_, timer, kCFRunLoopCommonModes);

+ CFRelease(timer);

+ monitor_.Enter();

+ while (run_loop_ != NULL) {

+ monitor_.Wait(Monitor::kNoTimeout);

+ }

+ monitor_.Exit();

}

Monitor& monitor() { return monitor_; }

@@ -163,6 +206,7 @@ class FSEventsWatcher {

static void Run(uword arg) {

Søren Gjesse 2014/02/07 10:20:02 We should consider in debug mode to get the thread

Anders Johnsen 2014/02/07 12:49:24 Done.

FSEventsWatcher* watcher = reinterpret_cast<FSEventsWatcher*>(arg);

watcher->run_loop_ = CFRunLoopGetCurrent();

+ CFRetain(watcher->run_loop_);

// Notify, as the run-loop is set.

watcher->monitor().Enter();

@@ -177,8 +221,8 @@ class FSEventsWatcher {

TimerCallback,

NULL);

CFRunLoopAddTimer(watcher->run_loop_, timer, kCFRunLoopCommonModes);

+ CFRelease(timer);

CFRunLoopRun();

}

@@ -191,29 +235,8 @@ class FSEventsWatcher {

char base_path[PATH_MAX];

realpath(path, base_path);

- CFStringRef path_ref = CFStringCreateWithCString(

- NULL, base_path, kCFStringEncodingUTF8);

- Node* node = new Node(this, strlen(base_path), fds[0], fds[1], recursive);

- FSEventStreamContext context;

- context.version = 0;

- context.info = reinterpret_cast<void*>(node);

- context.retain = NULL;

- context.release = NULL;

- context.copyDescription = NULL;

- FSEventStreamRef ref = FSEventStreamCreate(

- NULL,

- Callback,

- &context,

- CFArrayCreate(NULL, reinterpret_cast<const void**>(&path_ref), 1, NULL),

- kFSEventStreamEventIdSinceNow,

- 0.10,

- kFSEventStreamCreateFlagNoDefer | kFSEventStreamCreateFlagFileEvents);

- node->set_ref(ref);

- return node;

+ return new Node(this, base_path, fds[0], fds[1], recursive);

}

private:

@@ -275,7 +298,6 @@ intptr_t FileSystemWatcher::WatchPath(intptr_t id,

bool recursive) {

FSEventsWatcher* watcher = reinterpret_cast<FSEventsWatcher*>(id);

FSEventsWatcher::Node* node = watcher->AddPath(path, events, recursive);

- node->Start();

return reinterpret_cast<intptr_t>(node);

}

@@ -284,7 +306,6 @@ void FileSystemWatcher::UnwatchPath(intptr_t id, intptr_t path_id) {

USE(id);

FSEventsWatcher::Node* node =

reinterpret_cast<FSEventsWatcher::Node*>(path_id);

- node->Stop();

delete node;

}

« no previous file with comments | « no previous file | no next file » | no next file with comments »