From 57ab1694b78bceb66cc562b75ce453125217fa18 Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Wed, 11 Nov 2020 16:09:08 +0000
Subject: Improve protection of _parts map.

---
 .../src/vespa/searchlib/transactionlog/domain.cpp      | 18 ++++++++++++------
 searchlib/src/vespa/searchlib/transactionlog/domain.h  |  1 +
 2 files changed, 13 insertions(+), 6 deletions(-)

(limited to 'searchlib')

diff --git a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp
index 07da8087544..9f1e6bde06b 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp
@@ -215,7 +215,7 @@ Domain::triggerSyncNow()
     std::unique_lock guard(_syncMonitor);
     if (!_pendingSync) {
         _pendingSync = true;
-        _executor.execute(makeLambdaTask([this, domainPart=_parts.rbegin()->second]() {
+        _executor.execute(makeLambdaTask([this, domainPart= getActivePart()]() {
             domainPart->sync();
             std::lock_guard monitorGuard(_syncMonitor);
             _pendingSync = false;
@@ -242,6 +242,12 @@ Domain::findPart(SerialNum s)
     return DomainPart::SP();
 }
 
+DomainPart::SP
+Domain::getActivePart() {
+    std::lock_guard guard(_lock);
+    return _parts.rbegin()->second;
+}
+
 uint64_t
 Domain::size() const
 {
@@ -307,7 +313,7 @@ waitPendingSync(std::mutex &syncMonitor, std::condition_variable & syncCond, boo
 
 DomainPart::SP
 Domain::optionallyRotateFile(SerialNum serialNum) {
-    DomainPart::SP dp(_parts.rbegin()->second);
+    DomainPart::SP dp = getActivePart();
     if (dp->byteSize() > _config.getPartSizeLimit()) {
         waitPendingSync(_syncMonitor, _syncCond, _pendingSync);
         triggerSyncNow();
@@ -404,14 +410,14 @@ Domain::erase(SerialNum to)
 {
     bool retval(true);
     /// Do not erase the last element
+    UniqueLock guard(_lock);
     for (DomainPartList::iterator it(_parts.begin()); (_parts.size() > 1) && (it->second.get()->range().to() < to); it = _parts.begin()) {
         DomainPart::SP dp(it->second);
-        {
-            std::lock_guard guard(_lock);
-            _parts.erase(it);
-        }
+        _parts.erase(it);
+        guard.unlock();
         retval = retval && dp->erase(to);
         vespalib::File::sync(dir());
+        guard.lock();
     }
     if (_parts.begin()->second->range().to() >= to) {
         _parts.begin()->second->erase(to);
diff --git a/searchlib/src/vespa/searchlib/transactionlog/domain.h b/searchlib/src/vespa/searchlib/transactionlog/domain.h
index 432787f8619..c9eb6385b15 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/domain.h
+++ b/searchlib/src/vespa/searchlib/transactionlog/domain.h
@@ -58,6 +58,7 @@ public:
     Domain & setConfig(const DomainConfig & cfg);
 private:
     using UniqueLock = std::unique_lock<std::mutex>;
+    DomainPartSP getActivePart();
     void verifyLock(const UniqueLock & guard) const;
     void commitIfFull(const UniqueLock & guard);
     void commitAndTransferResponses(const UniqueLock & guard);
-- 
cgit v1.2.3