issue 485.1: reduce scope of lock in FlushFeature (arangodb#6943)

vasiliy-arangodb · Andrey Abramov · commit 46ee6d94b05f · 2018-10-17T14:33:48.000+03:00
diff --git a/arangod/IResearch/IResearchView.cpp b/arangod/IResearch/IResearchView.cpp
@@ -2037,11 +2037,33 @@ void IResearchView::registerFlushCallback() {
     return;
   }
 
-  flush->registerCallback(this, [this]() noexcept {
-    return IResearchView::FlushTransactionPtr(
-      this,
-      [](arangodb::FlushTransaction*){} // empty deleter
+  auto viewSelf = self();
+
+  flush->registerCallback(this, [viewSelf]() noexcept {
+    static struct NoopFlushTransaction: arangodb::FlushTransaction {
+      NoopFlushTransaction(): FlushTransaction("ArangoSearchNoop") {}
+      virtual arangodb::Result commit() override {
+        return arangodb::Result(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
+      }
+    } noopFlushTransaction;
+    SCOPED_LOCK_NAMED(viewSelf->mutex(), lock);
+
+    if (!*viewSelf) {
+      return arangodb::FlushFeature::FlushTransactionPtr(
+        &noopFlushTransaction, [](arangodb::FlushTransaction*)->void {}
+      );
+    }
+
+    auto trx = arangodb::FlushFeature::FlushTransactionPtr(
+      viewSelf->get(),
+      [](arangodb::FlushTransaction* trx)->void {
+        ADOPT_SCOPED_LOCK_NAMED(static_cast<IResearchView*>(trx)->self()->mutex(), lock);
+      }
     );
+
+    lock.release(); // unlocked in distructor above
+
+    return trx;
   });
 
   // noexcept
diff --git a/arangod/RestServer/FlushFeature.cpp b/arangod/RestServer/FlushFeature.cpp
@@ -154,15 +154,17 @@ bool FlushFeature::unregisterCallback(void* ptr) {
 void FlushFeature::executeCallbacks() {
   std::vector<FlushTransactionPtr> transactions;
 
-  READ_LOCKER(locker, _callbacksLock);
-  transactions.reserve(_callbacks.size());
-
-  // execute all callbacks. this will create as many transactions as
-  // there are callbacks
-  for (auto const& cb : _callbacks) {
-    // copy elision, std::move(..) not required
-    LOG_TOPIC(TRACE, arangodb::Logger::FLUSH) << "executing flush callback";
-    transactions.emplace_back(cb.second());
+  {
+    READ_LOCKER(locker, _callbacksLock);
+    transactions.reserve(_callbacks.size());
+
+    // execute all callbacks. this will create as many transactions as
+    // there are callbacks
+    for (auto const& cb: _callbacks) {
+      // copy elision, std::move(..) not required
+      LOG_TOPIC(TRACE, arangodb::Logger::FLUSH) << "executing flush callback";
+      transactions.emplace_back(cb.second());
+    }
   }
 
   // TODO: make sure all data is synced
