fsevents: FSEvents is most likely not thread-safe

Perform all operation with FSEventStream in the same thread, where it'll be used. Conflicts: src/unix/fsevents.c
nodejs · Aug 14, 2013 · f6308f3 · f6308f3
1 parent b1acb2e
commit f6308f3
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 26 deletions.
diff --git a/deps/uv/src/unix/fsevents.c b/deps/uv/src/unix/fsevents.c
@@ -192,41 +192,29 @@ void uv__fsevents_event_cb(ConstFSEventStreamRef streamRef,
 
 void uv__fsevents_schedule(void* arg) {
   uv_fs_event_t* handle;
-
-  handle = arg;
-  FSEventStreamScheduleWithRunLoop(handle->cf_eventstream,
-                                   handle->loop->cf_loop,
-                                   kCFRunLoopDefaultMode);
-  FSEventStreamStart(handle->cf_eventstream);
-  uv_sem_post(&handle->cf_sem);
-}
-
-
-int uv__fsevents_init(uv_fs_event_t* handle) {
   FSEventStreamContext ctx;
   FSEventStreamRef ref;
   CFStringRef path;
   CFArrayRef paths;
   CFAbsoluteTime latency;
   FSEventStreamCreateFlags flags;
 
+  handle = arg;
+
   /* Initialize context */
   ctx.version = 0;
   ctx.info = handle;
   ctx.retain = NULL;
   ctx.release = NULL;
   ctx.copyDescription = NULL;
 
-  /* Get absolute path to file */
-  handle->realpath = realpath(handle->filename, NULL);
-  if (handle->realpath != NULL)
-    handle->realpath_len = strlen(handle->realpath);
-
   /* Initialize paths array */
   path = CFStringCreateWithCString(NULL,
                                    handle->filename,
                                    CFStringGetSystemEncoding());
+  assert(path != NULL);
   paths = CFArrayCreate(NULL, (const void**)&path, 1, NULL);
+  assert(paths != NULL);
 
   latency = 0.15;
 
@@ -240,8 +228,42 @@ int uv__fsevents_init(uv_fs_event_t* handle) {
                             kFSEventStreamEventIdSinceNow,
                             latency,
                             flags);
+  assert(ref != NULL);
   handle->cf_eventstream = ref;
 
+  FSEventStreamScheduleWithRunLoop(handle->cf_eventstream,
+                                   handle->loop->cf_loop,
+                                   kCFRunLoopDefaultMode);
+  if (!FSEventStreamStart(handle->cf_eventstream))
+    abort();
+}
+
+
+static void uv__fsevents_unschedule(void* arg) {
+  uv_fs_event_t* handle;
+
+  handle = arg;
+
+  /* Stop emitting events */
+  FSEventStreamStop(handle->cf_eventstream);
+
+  /* Release stream */
+  FSEventStreamInvalidate(handle->cf_eventstream);
+  FSEventStreamRelease(handle->cf_eventstream);
+  handle->cf_eventstream = NULL;
+
+  /* Notify main thread that we're done here */
+  uv_sem_post(&handle->cf_sem);
+}
+
+
+int uv__fsevents_init(uv_fs_event_t* handle) {
+  /* Get absolute path to file */
+  handle->realpath = realpath(handle->filename, NULL);
+  if (handle->realpath != NULL)
+    handle->realpath_len = strlen(handle->realpath);
+
+  handle->cf_eventstream = NULL;
   /*
    * Events will occur in other thread.
    * Initialize callback for getting them back into event loop's thread
@@ -266,21 +288,16 @@ int uv__fsevents_init(uv_fs_event_t* handle) {
 
 
 int uv__fsevents_close(uv_fs_event_t* handle) {
-  if (handle->cf_eventstream == NULL)
+  if (handle->cf_cb == NULL)
     return -1;
 
-  /* Ensure that event stream was scheduled */
-  uv_sem_wait(&handle->cf_sem);
-
-  /* Stop emitting events */
-  FSEventStreamStop(handle->cf_eventstream);
+  uv__cf_loop_signal(handle->loop, uv__fsevents_unschedule, handle);
 
-  /* Release stream */
-  FSEventStreamInvalidate(handle->cf_eventstream);
-  FSEventStreamRelease(handle->cf_eventstream);
-  handle->cf_eventstream = NULL;
+  /* Wait for deinitialization */
+  uv_sem_wait(&handle->cf_sem);
 
   uv_close((uv_handle_t*) handle->cf_cb, (uv_close_cb) free);
+  handle->cf_cb = NULL;
 
   /* Free data in queue */
   UV__FSEVENTS_WALK(handle, {

diff --git a/deps/uv/src/unix/kqueue.c b/deps/uv/src/unix/kqueue.c
@@ -307,6 +307,7 @@ int uv_fs_event_init(uv_loop_t* loop,
 
 #if defined(__APPLE__)
   /* Nullify field to perform checks later */
+  handle->cf_cb = NULL;
   handle->cf_eventstream = NULL;
   handle->realpath = NULL;
   handle->realpath_len = 0;