fix potential issue during cluster index creation (#14692)

jsteemann · web-flow · commit 62872f20a5bc · 2021-08-25T15:51:34.000+02:00
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,10 @@
 devel
 -----
 
+* Fix a potential multi-threading issue in index creation on coordinators,
+  when an agency callback was triggered at the same time the method
+  `ensureIndexCoordinatorInner` was left.
+
 * Added protocol specific metrics: histogram about request body size, total
   number of HTTP/2 connections and total number of VST connections.
 
diff --git a/arangod/Cluster/ClusterInfo.cpp b/arangod/Cluster/ClusterInfo.cpp
@@ -4336,7 +4336,7 @@ Result ClusterInfo::ensureIndexCoordinatorInner(LogicalCollection const& collect
   // This object watches whether the collection is still present in Plan
   // It assumes that the collection *is* present and only changes state
   // if the collection disappears
-  CollectionWatcher collectionWatcher(_agencyCallbackRegistry, collection);
+  auto collectionWatcher = std::make_shared<CollectionWatcher>(_agencyCallbackRegistry, collection);
 
   if (!result.successful()) {
     if (result.httpCode() == rest::ResponseCode::PRECONDITION_FAILED) {
@@ -4437,7 +4437,7 @@ Result ClusterInfo::ensureIndexCoordinatorInner(LogicalCollection const& collect
           }
         }
 
-        if (!collectionWatcher.isPresent()) {
+        if (!collectionWatcher->isPresent()) {
           return Result(TRI_ERROR_ARANGO_INDEX_CREATION_FAILED,
                         "Collection " + collectionID +
                             " has gone from database " + databaseName +
@@ -4524,7 +4524,7 @@ Result ClusterInfo::ensureIndexCoordinatorInner(LogicalCollection const& collect
         // when we wanted to roll back the index creation.
       }
 
-      if (!collectionWatcher.isPresent()) {
+      if (!collectionWatcher->isPresent()) {
         return Result(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND,
                       "collection " + collectionID +
                           "appears to have been dropped from database " +
@@ -6164,9 +6164,10 @@ CollectionWatcher::CollectionWatcher(AgencyCallbackRegistry* agencyCallbackRegis
 
   _agencyCallback = std::make_shared<AgencyCallback>(
       collection.vocbase().server(), where,
-      [this](VPackSlice const& result) {
-        if (result.isNone()) {
-          _present.store(false);
+      [self = weak_from_this()](VPackSlice const& result) {
+        auto watcher = self.lock();
+        if (result.isNone() && watcher) {
+          watcher->_present.store(false);
         }
         return true;
       },
diff --git a/arangod/Cluster/ClusterInfo.h b/arangod/Cluster/ClusterInfo.h
@@ -72,7 +72,7 @@ struct ClusterCollectionCreationInfo;
 // make sure a collection is still in Plan
 // we are only going from *assuming* that it is present
 // to it being changed to not present.
-class CollectionWatcher {
+class CollectionWatcher : public std::enable_shared_from_this<CollectionWatcher> {
  public:
   CollectionWatcher(CollectionWatcher const&) = delete;
   CollectionWatcher(AgencyCallbackRegistry* agencyCallbackRegistry, LogicalCollection const& collection);
