check that we're removing the right root from watched_roots

Summary:
There was a race where:

1. `watch-del` is called on a root, which causes it to be removed from `watched_roots`
2. `watch` is called immediately afterwards on the same root, which causes it to be
re-added to `watched_roots`.
3. The notify thread for the first root completes, after which the *new* root gets
removed from the `watched_roots` list.

Fix this by checking before deleting.

Test Plan:
Added a `usleep(100000);` call right before the removal in
`root.c:run_notify_thread`, and a `usleep(200000);` right after the second `watch` in
`tests/integration/since.php:testReaddWatchFreshInstance`. This was enough to make the
bug trigger reliably while running the test. With the fix it doesn't.

Reviewers: wez

Reviewed By: wez

Differential Revision: https://phabricator.fb.com/D922818

--HG--
extra : rebase_source : 33ad24440d9b244f809ceddd746d6773352caa15
extra : amend_source : f218a450281b17eba03ac3788c3f35e51a902621

root.c

@@ -1980,6 +1980,21 @@ static const struct watchman_hash_funcs root_funcs = {
   root_del_val
 };
 
+static bool remove_root_from_watched(w_root_t *root)
+{
+  bool removed = false;
+  pthread_mutex_lock(&root_lock);
+  // it's possible that the root has already been removed and replaced with
+  // another, so make sure we're removing the right object
+  if (w_ht_val_ptr(w_ht_get(watched_roots, w_ht_ptr_val(root->root_path))) ==
+      root) {
+    w_ht_del(watched_roots, w_ht_ptr_val(root->root_path));
+    removed = true;
+  }
+  pthread_mutex_unlock(&root_lock);
+  return removed;
+}
+
 /* Returns true if the global config root_restrict_files is not defined or if
  * one of the files in root_restrict_files exists, false otherwise. */
 static bool root_check_restrict(const char *watch_path)
@@ -2137,9 +2152,7 @@ static void *run_notify_thread(void *arg)
 
   /* we'll remove it from watched roots if it isn't
    * already out of there */
-  pthread_mutex_lock(&root_lock);
-  w_ht_del(watched_roots, w_ht_ptr_val(root->root_path));
-  pthread_mutex_unlock(&root_lock);
+  remove_root_from_watched(root);
 
   w_root_delref(root);
   return 0;
@@ -2211,11 +2224,7 @@ bool w_root_cancel(w_root_t *root)
 
 bool w_root_stop_watch(w_root_t *root)
 {
-  bool stopped = false;
-
-  pthread_mutex_lock(&root_lock);
-  stopped = w_ht_del(watched_roots, w_ht_ptr_val(root->root_path));
-  pthread_mutex_unlock(&root_lock);
+  bool stopped = remove_root_from_watched(root);
 
   if (stopped) {
     w_root_cancel(root);