fix potential issue during cluster index creation (#14690)

jsteemann · KVS85 · web-flow · commit 5f0e28cfe98e · 2021-08-25T15:17:28.000+03:00
* fix potential issue during cluster index creation

* 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.

* Update CHANGELOG

Co-authored-by: Vadim &lt;vadim@arangodb.com&gt;
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,10 @@
 v3.7.15 (XXXX-XX-XX)
 --------------------
 
+* 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.
+
 * When creating Pregel memory-mapped files, create them with O_TMPFILE attribute
   on Linux so that files are guaranteed to vanish even if a process dies.
 
diff --git a/arangod/Cluster/ClusterInfo.cpp b/arangod/Cluster/ClusterInfo.cpp
@@ -4008,7 +4008,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() == (int)arangodb::rest::ResponseCode::PRECONDITION_FAILED) {
@@ -4111,7 +4111,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 +
@@ -4198,7 +4198,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 " +
diff --git a/arangod/Cluster/ClusterInfo.h b/arangod/Cluster/ClusterInfo.h
@@ -64,7 +64,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)
@@ -76,24 +76,27 @@ class CollectionWatcher {
 
     _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;
         },
         true, false);
+
     _agencyCallbackRegistry->registerCallback(_agencyCallback);
   }
+  
   ~CollectionWatcher();
 
   bool isPresent() {
     // Make sure we did not miss a callback
     _agencyCallback->refetchAndUpdate(true, false);
     return _present.load();
-  };
+  }
 
-private:
+ private:
   AgencyCallbackRegistry *_agencyCallbackRegistry;
   std::shared_ptr<AgencyCallback> _agencyCallback;
 
