unix, windows: update uv_fs_poll API

* the callback gets called only once on error, not repeatedly... * ...unless the error reason changes from e.g. UV_ENOENT to UV_EACCES * the callback receives pointers to uv_statbuf_t objects so it can inspect what changed
joyent · Jun 16, 2012 · 6d67cf1 · 6d67cf1
1 parent 7ca524e
commit 6d67cf1
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 44 deletions.
diff --git a/include/uv-private/uv-unix.h b/include/uv-private/uv-unix.h
@@ -28,13 +28,17 @@
 #include "eio.h"
 
 #include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
 #include <netdb.h>
-#include <pwd.h>
+
 #include <termios.h>
+#include <pwd.h>
 
 #include <semaphore.h>
 #include <pthread.h>
@@ -55,6 +59,8 @@ typedef int uv_file;
 
 typedef int uv_os_sock_t;
 
+typedef struct stat uv_statbuf_t;
+
 #define UV_ONCE_INIT PTHREAD_ONCE_INIT
 
 typedef pthread_once_t uv_once_t;
@@ -249,9 +255,6 @@ struct uv__io_s {
   struct stat statbuf; \
   eio_req* eio;
 
-#define UV_FS_POLL_PRIVATE_FIELDS                                             \
-  struct stat statbuf;
-
 #define UV_WORK_PRIVATE_FIELDS \
   eio_req* eio;
 

diff --git a/include/uv-private/uv-win.h b/include/uv-private/uv-win.h
@@ -165,6 +165,8 @@ typedef struct uv_buf_t {
 
 typedef int uv_file;
 
+typedef struct _stati64 uv_statbuf_t;
+
 typedef SOCKET uv_os_sock_t;
 
 typedef HANDLE uv_thread_t;
@@ -487,9 +489,6 @@ RB_HEAD(uv_timer_tree_s, uv_timer_s);
     };                                    \
   };
 
-#define UV_FS_POLL_PRIVATE_FIELDS         \
-  struct _stati64 statbuf;
-
 #define UV_WORK_PRIVATE_FIELDS            \
 
 #define UV_FS_EVENT_PRIVATE_FIELDS        \

diff --git a/include/uv.h b/include/uv.h
@@ -297,7 +297,6 @@ typedef void (*uv_async_cb)(uv_async_t* handle, int status);
 typedef void (*uv_prepare_cb)(uv_prepare_t* handle, int status);
 typedef void (*uv_check_cb)(uv_check_t* handle, int status);
 typedef void (*uv_idle_cb)(uv_idle_t* handle, int status);
-typedef void (*uv_fs_poll_cb)(uv_fs_poll_t* handle, int status);
 typedef void (*uv_getaddrinfo_cb)(uv_getaddrinfo_t* handle, int status,
     struct addrinfo* res);
 typedef void (*uv_exit_cb)(uv_process_t*, int exit_status, int term_signal);
@@ -315,6 +314,11 @@ typedef void (*uv_walk_cb)(uv_handle_t* handle, void* arg);
 typedef void (*uv_fs_event_cb)(uv_fs_event_t* handle, const char* filename,
     int events, int status);
 
+typedef void (*uv_fs_poll_cb)(uv_fs_poll_t* handle,
+                              int status,
+                              uv_statbuf_t* prev,
+                              uv_statbuf_t* curr);
+
 typedef enum {
   UV_LEAVE_GROUP = 0,
   UV_JOIN_GROUP
@@ -1526,17 +1530,22 @@ struct uv_fs_poll_s {
   uv_fs_poll_cb poll_cb;
   uv_timer_t timer_handle;
   uv_fs_t* fs_req;
-  UV_FS_POLL_PRIVATE_FIELDS
+  uv_statbuf_t statbuf;
 };
 
 UV_EXTERN int uv_fs_poll_init(uv_loop_t* loop, uv_fs_poll_t* handle);
 
 /*
  * Check the file at `path` for changes every `interval` milliseconds.
  *
- * Your callback gets invoked repeatedly with `status == -1` if `path`
- * does not exist or is inaccessible. The watcher is *not* stopped. This
- * lets you monitor a path until the resource becomes available (again).
+ * Your callback i invoked with `status == -1` if `path` does not exist
+ * or is inaccessible. The watcher is *not* stopped but your callback is
+ * not called again until something changes (e.g. when the file is created
+ * or the error reason changes).
+ *
+ * When `status == 0`, your callback receives pointers to the old and new
+ * `uv_statbuf_t` structs. They are valid for the duration of the callback
+ * only!
  *
  * For maximum portability, use multi-second intervals. Sub-second intervals
  * will not detect all changes on many file systems.

diff --git a/src/fs-poll.c b/src/fs-poll.c
@@ -26,13 +26,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#ifdef _WIN32
-typedef struct _stati64 uv__statbuf_t;
-#else
-typedef struct stat uv__statbuf_t;
-#endif
-
-static int statbuf_eq(const uv__statbuf_t* a, const uv__statbuf_t* b);
+static int statbuf_eq(const uv_statbuf_t* a, const uv_statbuf_t* b);
 static void timer_cb(uv_timer_t* timer, int status);
 static void poll_cb(uv_fs_t* req);
 
@@ -75,6 +69,7 @@ int uv_fs_poll_start(uv_fs_poll_t* handle,
   handle->interval = interval ? interval : 1;
   handle->start_time = uv_now(handle->loop);
   handle->busy_polling = 0;
+  memset(&handle->statbuf, 0, sizeof(handle->statbuf));
 
   if (uv_fs_stat(handle->loop, handle->fs_req, handle->path, poll_cb))
     abort();
@@ -132,7 +127,7 @@ static void timer_cb(uv_timer_t* timer, int status) {
 
 
 static void poll_cb(uv_fs_t* req) {
-  uv__statbuf_t* statbuf;
+  uv_statbuf_t* statbuf;
   uv_fs_poll_t* handle;
   uint64_t interval;
 
@@ -144,30 +139,22 @@ static void poll_cb(uv_fs_t* req) {
   assert(req == handle->fs_req);
 
   if (req->result != 0) {
-    /* TODO(bnoordhuis) Only signal the error the first time? What if the
-     * error reason changes?
-     */
-    uv__set_artificial_error(handle->loop, req->errorno);
-    handle->poll_cb(handle, -1);
-    handle->busy_polling = -1;
+    if (handle->busy_polling != -req->errorno) {
+      uv__set_artificial_error(handle->loop, req->errorno);
+      handle->poll_cb(handle, -1, NULL, NULL);
+      handle->busy_polling = -req->errorno;
+    }
     goto out;
   }
 
   statbuf = req->ptr;
 
-  if (handle->busy_polling == 0) {
-    handle->statbuf = *statbuf;
-    handle->busy_polling = 1;
-  }
-  else if (handle->busy_polling == -1) {
-    handle->statbuf = *statbuf;
-    handle->busy_polling = 1;
-    handle->poll_cb(handle, 0); /* Error went away. */
-  }
-  else if (!statbuf_eq(statbuf, &handle->statbuf)) {
-    handle->statbuf = *statbuf;
-    handle->poll_cb(handle, 0);
-  }
+  if (handle->busy_polling != 0)
+    if (handle->busy_polling < 0 || !statbuf_eq(&handle->statbuf, statbuf))
+      handle->poll_cb(handle, 0, &handle->statbuf, statbuf);
+
+  handle->statbuf = *statbuf;
+  handle->busy_polling = 1;
 
 out:
   uv_fs_req_cleanup(req);
@@ -188,7 +175,7 @@ static void poll_cb(uv_fs_t* req) {
 }
 
 
-static int statbuf_eq(const uv__statbuf_t* a, const uv__statbuf_t* b) {
+static int statbuf_eq(const uv_statbuf_t* a, const uv_statbuf_t* b) {
 #ifdef _WIN32
   return a->st_mtime == b->st_mtime
       && a->st_size == b->st_size

diff --git a/test/test-fs-poll.c b/test/test-fs-poll.c
@@ -22,14 +22,16 @@
 #include "uv.h"
 #include "task.h"
 
-#include <sys/stat.h>
-#include <fcntl.h>
+#include <string.h>
 
 #define FIXTURE "testfile"
 
-static void poll_cb(uv_fs_poll_t* handle, int status);
 static void timer_cb(uv_timer_t* handle, int status);
 static void close_cb(uv_handle_t* handle);
+static void poll_cb(uv_fs_poll_t* handle,
+                    int status,
+                    uv_statbuf_t* prev,
+                    uv_statbuf_t* curr);
 
 static uv_fs_poll_t poll_handle;
 static uv_timer_t timer_handle;
@@ -70,34 +72,52 @@ static void timer_cb(uv_timer_t* handle, int status) {
 }
 
 
-static void poll_cb(uv_fs_poll_t* handle, int status) {
+static void poll_cb(uv_fs_poll_t* handle,
+                    int status,
+                    uv_statbuf_t* prev,
+                    uv_statbuf_t* curr) {
   ASSERT(handle == &poll_handle);
   ASSERT(uv_is_active((uv_handle_t*)handle));
 
   switch (poll_cb_called++) {
   case 0:
     ASSERT(status == -1);
+    ASSERT(prev == NULL);
+    ASSERT(curr == NULL);
     ASSERT(uv_last_error(loop).code == UV_ENOENT);
     touch_file(FIXTURE);
     break;
 
   case 1:
     ASSERT(status == 0);
+    ASSERT(prev != NULL);
+    ASSERT(curr != NULL);
+    {
+      uv_statbuf_t buf;
+      memset(&buf, 0, sizeof(buf));
+      ASSERT(0 == memcmp(&buf, prev, sizeof(buf)));
+    }
     ASSERT(0 == uv_timer_start(&timer_handle, timer_cb, 20, 0));
     break;
 
   case 2:
     ASSERT(status == 0);
+    ASSERT(prev != NULL);
+    ASSERT(curr != NULL);
     ASSERT(0 == uv_timer_start(&timer_handle, timer_cb, 200, 0));
     break;
 
   case 3:
     ASSERT(status == 0);
+    ASSERT(prev != NULL);
+    ASSERT(curr != NULL);
     remove(FIXTURE);
     break;
 
   case 4:
     ASSERT(status == -1);
+    ASSERT(prev == NULL);
+    ASSERT(curr == NULL);
     ASSERT(uv_last_error(loop).code == UV_ENOENT);
     uv_close((uv_handle_t*)handle, close_cb);
     break;