factor cookie sync out into its own class

Summary:
This will aid in subsequent refactoring.

One important difference here is that we no longer need to acquire a write lock
to the root; the cookie sync was abusing this to generate a serial number for a
unique cookie file name.  We now just use a local atomic integer for that
purpose instead.

As a result of that, we no longer need to record the root number in the
generated cookie name, so we're back to the original cookie format from before
adding root numbers, which is closer to those asserted in
`tests/integration/othercookies.php`.

Reviewed By: farnz

Differential Revision: D4045069

fbshipit-source-id: 84bf0589cb2a4a58f571847649897c2f8ad4f070

CookieSync.cpp

@@ -0,0 +1,114 @@
+/* Copyright 2012-present Facebook, Inc.
+ * Licensed under the Apache License, Version 2.0 */
+
+#include "watchman.h"
+
+namespace watchman {
+
+CookieSync::CookieSync(const w_string& dir) {
+  setCookieDir(dir);
+}
+
+void CookieSync::setCookieDir(const w_string& dir) {
+  cookieDir_ = dir;
+
+  char hostname[256];
+  gethostname(hostname, sizeof(hostname));
+  hostname[sizeof(hostname) - 1] = '\0';
+
+  cookiePrefix_ = w_string::printf(
+      "%.*s%c" WATCHMAN_COOKIE_PREFIX "%s-%d-",
+      int(cookieDir_.size()),
+      cookieDir_.data(),
+      WATCHMAN_DIR_SEP,
+      hostname,
+      int(getpid()));
+}
+
+bool CookieSync::syncToNow(std::chrono::milliseconds timeout) {
+  Cookie cookie;
+  w_stm_t file;
+  int errcode = 0;
+
+  auto cookie_lock = std::unique_lock<std::mutex>(cookie.mutex);
+
+  /* generate a cookie name: cookie prefix + id */
+  auto path_str = w_string::printf(
+      "%.*s%" PRIu32, cookiePrefix_.size(), cookiePrefix_.data(), serial_++);
+
+  /* insert our cookie in the map */
+  {
+    auto wlock = cookies_.wlock();
+    auto& map = *wlock;
+    map[path_str] = &cookie;
+  }
+
+  /* compute deadline */
+  auto deadline = std::chrono::system_clock::now() + timeout;
+
+  /* touch the file */
+  file = w_stm_open(
+      path_str.c_str(), O_CREAT | O_TRUNC | O_WRONLY | O_CLOEXEC, 0700);
+  if (!file) {
+    errcode = errno;
+    w_log(
+        W_LOG_ERR,
+        "sync_to_now: creat(%s) failed: %s\n",
+        path_str.c_str(),
+        strerror(errcode));
+    goto out;
+  }
+  w_stm_close(file);
+
+  w_log(W_LOG_DBG, "sync_to_now [%s] waiting\n", path_str.c_str());
+
+  /* timed cond wait (unlocks cookie lock, reacquires) */
+  if (!cookie.cond.wait_until(
+          cookie_lock, deadline, [&] { return cookie.seen; })) {
+    w_log(
+        W_LOG_ERR,
+        "sync_to_now: %s timedwait failed: %d: istimeout=%d %s\n",
+        path_str.c_str(),
+        errcode,
+        errcode == ETIMEDOUT,
+        strerror(errcode));
+    goto out;
+  }
+  w_log(W_LOG_DBG, "sync_to_now [%s] done\n", path_str.c_str());
+
+out:
+  cookie_lock.unlock();
+
+  // can't unlink the file until after the cookie has been observed because
+  // we don't know which file got changed until we look in the cookie dir
+  unlink(path_str.c_str());
+
+  {
+    auto map = cookies_.wlock();
+    map->erase(path_str);
+  }
+
+  if (!cookie.seen) {
+    errno = errcode;
+    return false;
+  }
+
+  return true;
+}
+
+void CookieSync::notifyCookie(const w_string& path) const {
+  auto map = cookies_.rlock();
+  auto cookie_iter = map->find(path);
+  w_log(
+      W_LOG_DBG,
+      "cookie for %s? %s\n",
+      path.c_str(),
+      cookie_iter != map->end() ? "yes" : "no");
+
+  if (cookie_iter != map->end()) {
+    auto cookie = cookie_iter->second;
+    cookie->seen = true;
+    cookie->cond.notify_one();
+  }
+}
+}

CookieSync.h

@@ -0,0 +1,66 @@
+/* Copyright 2012-present Facebook, Inc.
+ * Licensed under the Apache License, Version 2.0 */
+#pragma once
+#include "watchman_synchronized.h"
+#define WATCHMAN_COOKIE_PREFIX ".watchman-cookie-"
+
+namespace watchman {
+
+class CookieSync {
+ public:
+  explicit CookieSync(const w_string& dir);
+  void setCookieDir(const w_string& dir);
+
+  /* Ensure that we're synchronized with the state of the
+   * filesystem at the current time.
+   * We do this by touching a cookie file and waiting to
+   * observe it via inotify.  When we see it we know that
+   * we've seen everything up to the point in time at which
+   * we're asking questions.
+   * Returns true if we observe the change within the requested
+   * time, false otherwise. */
+  bool syncToNow(std::chrono::milliseconds timeout);
+
+  /* If path is a valid cookie in the map, notify the waiter.
+   * Returns true if the path matches the cookie prefix (not just
+   * whether the cookie is currently valid).
+   * Returns false if the path does not match our cookie prefix.
+   */
+  void notifyCookie(const w_string& path) const;
+
+  // We need to guarantee that we never collapse a cookie notification
+  // out of the pending list, because we absolutely must observe it coming
+  // in via the kernel notification mechanism in order for synchronization
+  // to be correct.
+  // Since we don't have a w_root_t available, we can't tell what the
+  // precise cookie prefix is for the current pending list here, so
+  // we do a substring match.  Not the most elegant thing in the world.
+  static inline bool isPossiblyACookie(const w_string_t* path) {
+    return w_string_contains_cstr_len(
+        path, WATCHMAN_COOKIE_PREFIX, sizeof(WATCHMAN_COOKIE_PREFIX) - 1);
+  }
+
+  const w_string& cookiePrefix() const {
+    return cookiePrefix_;
+  }
+
+  const w_string& cookieDir() const {
+    return cookieDir_;
+  }
+
+ private:
+  struct Cookie {
+    std::condition_variable cond;
+    std::mutex mutex;
+    bool seen{false};
+  };
+
+  // path to the query cookie dir
+  w_string cookieDir_;
+  // valid filename prefix for cookies we create
+  w_string cookiePrefix_;
+  // Serial number for cookie filename
+  std::atomic<uint32_t> serial_{0};
+  Synchronized<std::unordered_map<w_string, Cookie*>> cookies_;
+};
+}

Makefile.am

@@ -11,6 +11,7 @@ JSON_LIB = -L. libwmanjson.a
 watchman_CPPFLAGS = $(THIRDPARTY_CPPFLAGS) @IRONMANCFLAGS@
 watchman_LDADD = $(JSON_LIB) $(ART_LIB) libwildmatch.a
 watchman_SOURCES = \
+	CookieSync.cpp \
 	InMemoryView.cpp \
 	QueryableView.cpp \
 	argv.cpp       \

pending.cpp

@@ -162,18 +162,6 @@ struct kid_context {
   struct watchman_pending_collection *coll;
 };
 
-// We need to guarantee that we never collapse a cookie notification
-// out of the pending list, because we absolutely must observe it coming
-// in via the kernel notification mechanism in order for synchronization
-// to be correct.
-// Since we don't have a w_root_t available, we can't tell what the
-// precise cookie prefix is for the current pending list here, so
-// we do a substring match.  Not the most elegant thing in the world.
-static inline bool is_possibly_a_cookie(w_string_t *path) {
-  return w_string_contains_cstr_len(path, WATCHMAN_COOKIE_PREFIX,
-                                    sizeof(WATCHMAN_COOKIE_PREFIX) - 1);
-}
-
 // This is the iterator callback we use to prune out obsoleted leaves.
 // We need to compare the prefix to make sure that we don't delete
 // a sibling node by mistake (see commentary on the is_path_prefix
@@ -185,10 +173,9 @@ static int delete_kids(void *data, const unsigned char *key, uint32_t key_len,
   unused_parameter(value);
 
   if ((p->flags & W_PENDING_CRAWL_ONLY) == 0 && key_len > ctx->root->len &&
-      is_path_prefix((const char *)key, key_len, ctx->root->buf,
-                     ctx->root->len) &&
-      !is_possibly_a_cookie(p->path)) {
-
+      is_path_prefix(
+          (const char*)key, key_len, ctx->root->buf, ctx->root->len) &&
+      !watchman::CookieSync::isPossiblyACookie(p->path)) {
     w_log(W_LOG_DBG,
           "delete_kids: removing (%d) %.*s from pending because it is "
           "obsoleted by (%d) %.*s\n",
@@ -266,8 +253,7 @@ is_obsoleted_by_containing_dir(struct watchman_pending_collection *coll,
 
   if ((p->flags & W_PENDING_RECURSIVE) &&
       is_path_prefix(path->buf, path->len, (const char*)leaf->key, leaf->key_len)) {
-
-    if (is_possibly_a_cookie(path)) {
+    if (watchman::CookieSync::isPossiblyACookie(path)) {
       return false;
     }
 

root/init.cpp

@@ -166,7 +166,7 @@ void w_root_delref_raw(w_root_t *root) {
 }
 
 watchman_root::watchman_root(const w_string& root_path)
-    : root_path(root_path), inner(root_path) {}
+    : root_path(root_path), cookies(root_path), inner(root_path) {}
 
 watchman_root::~watchman_root() {
   w_log(W_LOG_DBG, "root: final ref on %s\n", root_path.c_str());

root/iothread.cpp

@@ -179,33 +179,16 @@ void w_root_process_path(
    *
    * The below condition is true for cases 1 and 2 and false for 3 and 4.
    */
-  if (w_string_startswith(full_path, lock->root->query_cookie_prefix)) {
+  if (w_string_startswith(full_path, lock->root->cookies.cookiePrefix())) {
     bool consider_cookie =
         (lock->root->inner.watcher->flags & WATCHER_HAS_PER_FILE_NOTIFICATIONS)
         ? ((flags & W_PENDING_VIA_NOTIFY) || !lock->root->inner.done_initial)
         : true;
 
-    if (!consider_cookie) {
-      // Never allow cookie files to show up in the tree
-      return;
+    if (consider_cookie) {
+      lock->root->cookies.notifyCookie(full_path);
     }
 
-    {
-      auto map = lock->root->query_cookies.rlock();
-      auto cookie_iter = map->find(full_path);
-      w_log(
-          W_LOG_DBG,
-          "cookie for %s? %s\n",
-          full_path.c_str(),
-          cookie_iter != map->end() ? "yes" : "no");
-
-      if (cookie_iter != map->end()) {
-        auto cookie = cookie_iter->second;
-        cookie->seen = true;
-        cookie->cond.notify_one();
-      }
-    };
-
     // Never allow cookie files to show up in the tree
     return;
   }

root/stat.cpp

@@ -192,7 +192,7 @@ void stat_path(
       if (!w_ht_get(root->ignore.ignore_vcs, w_ht_ptr_val(dir_name)) ||
           // but do if we're looking at the cookie dir (stat_path is never
           // called for the root itself)
-          w_string_equal(full_path, root->query_cookie_dir)) {
+          w_string_equal(full_path, root->cookies.cookieDir())) {
         if (!root->inner.watcher->flags & WATCHER_HAS_PER_FILE_NOTIFICATIONS) {
           /* we always need to crawl, but may not need to be fully recursive */
           w_pending_coll_add(coll, full_path, now,