From b6539254c4f7a804e79cd32fb0ae900e938e9e06 Mon Sep 17 00:00:00 2001
From: Arne Juul <arnej@verizonmedia.com>
Date: Thu, 24 Jun 2021 11:14:28 +0000
Subject: keep ownership in unique_ptr

* avoid doing release() in one place and then making a new
  unique_ptr later, hoping that will always be in sync
---
 .../vespa/slobrok/server/rpc_server_manager.cpp    | 38 +++++++++++-----------
 .../src/vespa/slobrok/server/rpc_server_manager.h  |  2 +-
 2 files changed, 20 insertions(+), 20 deletions(-)

(limited to 'slobrok')

diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
index 1b373284782..ead062e9783 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
@@ -283,7 +283,7 @@ RpcServerManager::~RpcServerManager()
 void
 RpcServerManager::PerformTask()
 {
-    std::vector<std::unique_ptr<ManagedRpcServer>> deleteAfterSwap;
+    std::vector<std::unique_ptr<NamedService>> deleteAfterSwap;
     std::swap(deleteAfterSwap, _deleteList);
 }
 
@@ -292,32 +292,32 @@ void
 RpcServerManager::notifyFailedRpcSrv(ManagedRpcServer *rpcsrv, std::string errmsg)
 {
     _env.countFailedHeartbeat();
-    bool logged = false;
+    const auto &name = rpcsrv->getName();
+    const auto &spec = rpcsrv->getSpec();
+    const char *namep = name.c_str();
+    const char *specp = spec.c_str();
+    std::unique_ptr<NamedService> toDelete;
     const NamedService *old = _rpcsrvmap.lookup(rpcsrv->getName());
     if (old == rpcsrv) {
-        old = _rpcsrvmap.remove(rpcsrv->getName()).release();
-        LOG_ASSERT(old == rpcsrv);
-        LOG(info, "managed server %s at %s failed: %s",
-            rpcsrv->getName().c_str(), rpcsrv->getSpec().c_str(), errmsg.c_str());
-        logged = true;
+        toDelete = _rpcsrvmap.remove(name);
+        LOG_ASSERT(toDelete.get() == rpcsrv);
+        LOG(info, "managed server %s at %s failed: %s", namep, specp, errmsg.c_str());
+    } else {
+        // only managed servers should exist, this is bad:
+        LOG(error, "unmanaged server %s at %s failed: %s", namep, specp, errmsg.c_str());
     }
-    _exchanger.forwardRemove(rpcsrv->getName(), rpcsrv->getSpec());
+    _exchanger.forwardRemove(name, spec);
     for (size_t i = 0; i < _addManageds.size(); ++i) {
         if (_addManageds[i].rpcsrv == rpcsrv) {
-            _addManageds[i].handler
-                .doneHandler(OkState(13, "failed check using listNames callback"));
-            _addManageds[i].rpcsrv = 0;
-            LOG(warning, "rpcserver %s at %s failed while trying to register",
-                rpcsrv->getName().c_str(), rpcsrv->getSpec().c_str());
-            logged = true;
+            LOG(warning, "rpcserver %s at %s failed while trying to register", namep, specp);
+            _addManageds[i].rpcsrv = nullptr;
+            _addManageds[i].handler.doneHandler(OkState(13, "failed check using listNames callback"));
         }
     }
-    if (!logged) {
-        LOG(warning, "unmanaged server %s at %s failed: %s",
-            rpcsrv->getName().c_str(), rpcsrv->getSpec().c_str(), errmsg.c_str());
+    if (toDelete) {
+        _deleteList.push_back(std::move(toDelete));
+        ScheduleNow();
     }
-    _deleteList.push_back(std::unique_ptr<ManagedRpcServer>(rpcsrv));
-    ScheduleNow();
 }
 
 void
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_manager.h b/slobrok/src/vespa/slobrok/server/rpc_server_manager.h
index 174afc1adcc..9c72fe91f18 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_manager.h
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_manager.h
@@ -43,7 +43,7 @@ private:
             : rpcsrv(d), handler(std::move(h)) {}
     };
     std::vector<MRSandRRSC>         _addManageds;
-    std::vector<std::unique_ptr<ManagedRpcServer>> _deleteList;
+    std::vector<std::unique_ptr<NamedService>> _deleteList;
 public:
     OkState checkPartner(const std::string & remslobrok);
 
-- 
cgit v1.2.3