Revert "Fix Slobrok race between completion callback and scheduled task [run-systemtest]"

1 files changed, 37 insertions, 37 deletions

diff --git a/slobrok/src/vespa/slobrok/sbregister.cpp b/slobrok/src/vespa/slobrok/sbregister.cpp
index f00da4a9515..cdab4d4009d 100644
--- a/slobrok/src/vespa/slobrok/sbregister.cpp
+++ b/slobrok/src/vespa/slobrok/sbregister.cpp
@@ -67,8 +67,8 @@ RegisterAPI::RegisterAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config
       _names(),
       _pending(),
       _unreg(),
-      _target(nullptr),
-      _req(nullptr)
+      _target(0),
+      _req(0)
 {
     _configurator->poll();
     if ( ! _slobrokSpecs.ok()) {
@@ -82,12 +82,12 @@ RegisterAPI::RegisterAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config
 RegisterAPI::~RegisterAPI()
 {
     Kill();
-    _configurator.reset();
-    if (_req != nullptr) {
+    _configurator.reset(0);
+    if (_req != 0) {
         _req->Abort();
         _req->SubRef();
     }
-    if (_target != nullptr) {
+    if (_target != 0) {
         _target->SubRef();
     }
 }
@@ -96,15 +96,15 @@ RegisterAPI::~RegisterAPI()
 void
 RegisterAPI::registerName(vespalib::stringref name)
 {
-    std::lock_guard guard(_lock);
-    for (const auto& existing_name : _names) {
-        if (existing_name == name) {
+    std::lock_guard<std::mutex> guard(_lock);
+    for (uint32_t i = 0; i < _names.size(); ++i) {
+        if (_names[i] == name) {
             return;
         }
     }
     _busy.store(true, std::memory_order_relaxed);
-    _names.emplace_back(name);
-    _pending.emplace_back(name);
+    _names.push_back(name);
+    _pending.push_back(name);
     discard(_unreg, name);
     ScheduleNow();
 }
@@ -113,11 +113,11 @@ RegisterAPI::registerName(vespalib::stringref name)
 void
 RegisterAPI::unregisterName(vespalib::stringref name)
 {
-    std::lock_guard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     _busy.store(true, std::memory_order_relaxed);
     discard(_names, name);
     discard(_pending, name);
-    _unreg.emplace_back(name);
+    _unreg.push_back(name);
     ScheduleNow();
 }
 
@@ -125,7 +125,6 @@ RegisterAPI::unregisterName(vespalib::stringref name)
 void
 RegisterAPI::handleReqDone()
 {
-    std::lock_guard guard(_lock);
     if (_reqDone) {
         _reqDone = false;
         if (_req->IsError()) {
@@ -134,10 +133,10 @@ RegisterAPI::handleReqDone()
                     _req->GetErrorMessage(), _req->GetErrorCode());
                 // unexpected error; close our connection to this
                 // slobrok server and try again with a fresh slate
-                if (_target != nullptr) {
+                if (_target != 0) {
                     _target->SubRef();
                 }
-                _target = nullptr;
+                _target = 0;
                 _busy.store(true, std::memory_order_relaxed);
             } else {
                 LOG(warning, "%s(%s -> %s) failed: %s",
@@ -147,7 +146,7 @@ RegisterAPI::handleReqDone()
                     _req->GetErrorMessage());
             }
         } else {
-            if (_logOnSuccess && _pending.empty() && !_names.empty()) {
+            if (_logOnSuccess && (_pending.size() == 0) && (_names.size() > 0)) {
                 LOG(info, "[RPC @ %s] registering %s with location broker %s completed successfully",
                     createSpec(_orb).c_str(), _names[0].c_str(), _currSlobrok.c_str());
                 _logOnSuccess = false;
@@ -156,7 +155,7 @@ RegisterAPI::handleReqDone()
             _backOff.reset();
         }
         _req->SubRef();
-        _req = nullptr;
+        _req = 0;
     }
 }
 
@@ -164,29 +163,29 @@ RegisterAPI::handleReqDone()
 void
 RegisterAPI::handleReconnect()
 {
-    if (_configurator->poll() && _target != nullptr) {
+    if (_configurator->poll() && _target != 0) {
         if (! _slobrokSpecs.contains(_currSlobrok)) {
             vespalib::string cps = _slobrokSpecs.logString();
             LOG(warning, "[RPC @ %s] location broker %s removed, will disconnect and use one of: %s",
                 createSpec(_orb).c_str(), _currSlobrok.c_str(), cps.c_str());
             _target->SubRef();
-            _target = nullptr;
+            _target = 0;
         }
     }
-    if (_target == nullptr) {
+    if (_target == 0) {
         _logOnSuccess = true;
         _currSlobrok = _slobrokSpecs.nextSlobrokSpec();
-        if (!_currSlobrok.empty()) {
+        if (_currSlobrok.size() > 0) {
             // try next possible server.
             _target = _orb.GetTarget(_currSlobrok.c_str());
         }
         {
-            std::lock_guard guard(_lock);
+            std::lock_guard<std::mutex> guard(_lock);
             // now that we have a new connection, we need to
             // immediately re-register everything.
             _pending = _names;
         }
-        if (_target == nullptr) {
+        if (_target == 0) {
             // we have tried all possible servers.
             // start from the top after a delay,
             // possibly with a warning.
@@ -212,13 +211,13 @@ RegisterAPI::handlePending()
     bool reg   = false;
     vespalib::string name;
     {
-        std::lock_guard guard(_lock);
+        std::lock_guard<std::mutex> guard(_lock);
         // pop off the todo stack, unregister has priority
-        if (!_unreg.empty()) {
+        if (_unreg.size() > 0) {
             name = _unreg.back();
             _unreg.pop_back();
             unreg = true;
-        } else if (!_pending.empty()) {
+        } else if (_pending.size() > 0) {
             name = _pending.back();
             _pending.pop_back();
             reg = true;
@@ -245,7 +244,7 @@ RegisterAPI::handlePending()
     } else {
         // nothing more to do right now; schedule to re-register all
         // names after a long delay.
-        std::lock_guard guard(_lock);
+        std::lock_guard<std::mutex> guard(_lock);
         _pending = _names;
         LOG(debug, "done, reschedule in 30s");
         _busy.store(false, std::memory_order_relaxed);
@@ -257,25 +256,23 @@ void
 RegisterAPI::PerformTask()
 {
     handleReqDone();
-    if (_req != nullptr) {
+    if (_req != 0) {
         LOG(debug, "req in progress");
         return; // current request still in progress, don't start anything new
     }
     handleReconnect();
     // still no connection?
-    if (_target == nullptr) return;
+    if (_target == 0) return;
     handlePending();
 }
 
 
 void
-RegisterAPI::RequestDone([[maybe_unused]] FRT_RPCRequest *req)
+RegisterAPI::RequestDone(FRT_RPCRequest *req)
 {
-    {
-        std::lock_guard guard(_lock);
-        LOG_ASSERT(req == _req && !_reqDone);
-        _reqDone = true;
-    }
+    LOG_ASSERT(req == _req && !_reqDone);
+    (void) req;
+    _reqDone = true;
     ScheduleNow();
 }
 
@@ -299,13 +296,16 @@ RegisterAPI::RPCHooks::RPCHooks(RegisterAPI &owner)
 }
 
 
-RegisterAPI::RPCHooks::~RPCHooks() = default;
+RegisterAPI::RPCHooks::~RPCHooks()
+{
+}
+
 
 void
 RegisterAPI::RPCHooks::rpc_listNamesServed(FRT_RPCRequest *req)
 {
     FRT_Values &dst = *req->GetReturn();
-    std::lock_guard guard(_owner._lock);
+    std::lock_guard<std::mutex> guard(_owner._lock);
     FRT_StringValue *names = dst.AddStringArray(_owner._names.size());
     for (uint32_t i = 0; i < _owner._names.size(); ++i) {
         dst.SetString(&names[i], _owner._names[i].c_str());