GC unuse code and use std::mutex/std:condition_variable over vespalib::Monitor

16 files changed, 71 insertions, 477 deletions

diff --git a/messagebus/src/tests/messageordering/messageordering.cpp b/messagebus/src/tests/messageordering/messageordering.cpp
index 481b8bbd270..fe523135364 100644
--- a/messagebus/src/tests/messageordering/messageordering.cpp
+++ b/messagebus/src/tests/messageordering/messageordering.cpp
@@ -29,25 +29,23 @@ getRouting()
 class MultiReceptor : public IMessageHandler
 {
 private:
-    vespalib::Monitor _mon;
+    std::mutex _mon;
     DestinationSession* _destinationSession;
     int _messageCounter;
 
-    MultiReceptor(const Receptor &);
-    MultiReceptor &operator=(const Receptor &);
 public:
     MultiReceptor()
         : _mon(),
-          _destinationSession(0),
+          _destinationSession(nullptr),
           _messageCounter(0)
     {}
     void handleMessage(Message::UP msg) override
      {
-        SimpleMessage& simpleMsg(dynamic_cast<SimpleMessage&>(*msg));
+        auto & simpleMsg(dynamic_cast<SimpleMessage&>(*msg));
         LOG(spam, "Attempting to acquire lock for %s",
             simpleMsg.getValue().c_str());
 
-        vespalib::MonitorGuard lock(_mon);
+        std::lock_guard guard(_mon);
 
         vespalib::string expected(vespalib::make_string("%d", _messageCounter));
         LOG(debug, "Got message %p with %s, expecting %s",
@@ -79,20 +77,22 @@ public:
 
 class VerifyReplyReceptor : public IReplyHandler
 {
-    vespalib::Monitor _mon;
+    mutable std::mutex              _mon;
+    mutable std::condition_variable _cond;
     std::string _failure;
     int _replyCount;
 public:
-    ~VerifyReplyReceptor();
+    ~VerifyReplyReceptor() override;
     VerifyReplyReceptor();
     void handleReply(Reply::UP reply) override;
     void waitUntilDone(int waitForCount) const;
     const std::string& getFailure() const { return _failure; }
 };
 
-VerifyReplyReceptor::~VerifyReplyReceptor() {}
+VerifyReplyReceptor::~VerifyReplyReceptor() = default;
 VerifyReplyReceptor::VerifyReplyReceptor()
     : _mon(),
+      _cond(),
       _failure(),
       _replyCount(0)
 {}
@@ -100,7 +100,7 @@ VerifyReplyReceptor::VerifyReplyReceptor()
 void
 VerifyReplyReceptor::handleReply(Reply::UP reply)
 {
-    vespalib::MonitorGuard lock(_mon);
+    std::lock_guard lock(_mon);
     if (reply->hasErrors()) {
         std::ostringstream ss;
         ss << "Reply failed with "
@@ -113,7 +113,7 @@ VerifyReplyReceptor::handleReply(Reply::UP reply)
         LOG(warning, "%s", ss.str().c_str());
     } else {
         vespalib::string expected(vespalib::make_string("%d", _replyCount));
-        SimpleReply& simpleReply(static_cast<SimpleReply&>(*reply));
+        auto & simpleReply(static_cast<SimpleReply&>(*reply));
         if (simpleReply.getValue() != expected) {
             std::stringstream ss;
             ss << "Received out-of-sequence reply! Expected "
@@ -127,14 +127,14 @@ VerifyReplyReceptor::handleReply(Reply::UP reply)
         }
     }
     ++_replyCount;
-    lock.broadcast();
+    _cond.notify_all();
 }
 void
 VerifyReplyReceptor::waitUntilDone(int waitForCount) const
 {
-    vespalib::MonitorGuard lock(_mon);
+    std::unique_lock guard(_mon);
     while (_replyCount < waitForCount) {
-        lock.wait(1000);
+        _cond.wait_for(guard, 1s);
     }
 }
 
diff --git a/messagebus/src/vespa/messagebus/testlib/slobrok.cpp b/messagebus/src/vespa/messagebus/testlib/slobrok.cpp
index 620a4615d08..944cd505957 100644
--- a/messagebus/src/vespa/messagebus/testlib/slobrok.cpp
+++ b/messagebus/src/vespa/messagebus/testlib/slobrok.cpp
@@ -13,21 +13,22 @@ namespace {
 class WaitTask : public FNET_Task
 {
 private:
-    bool              _done;
-    vespalib::Monitor _mon;
+    bool                    _done;
+    std::mutex              _mon;
+    std::condition_variable _cond;
 public:
-    WaitTask(FNET_Scheduler *s) : FNET_Task(s), _done(false), _mon() {}
+    explicit WaitTask(FNET_Scheduler *s) : FNET_Task(s), _done(false), _mon() {}
     void wait() {
-        vespalib::MonitorGuard guard(_mon);
+        std::unique_lock guard(_mon);
         while (!_done) {
-            guard.wait();
+            _cond.wait(guard);
         }
     }
 
     void PerformTask() override {
-        vespalib::MonitorGuard guard(_mon);
+        std::lock_guard guard(_mon);
         _done = true;
-        guard.signal();
+        _cond.notify_one();
     }
 };
 } // namespace <unnamed>
@@ -52,11 +53,11 @@ void
 Slobrok::init()
 {
     slobrok::ConfigShim shim(_port);
-    _env.reset(new slobrok::SBEnv(shim));
+    _env = std::make_unique<slobrok::SBEnv>(shim);
     _thread.setEnv(_env.get());
     WaitTask wt(_env->getTransport()->GetScheduler());
     wt.ScheduleNow();
-    if (_pool.NewThread(&_thread, 0) == 0) {
+    if (_pool.NewThread(&_thread, nullptr) == nullptr) {
         LOG_ABORT("Could not spawn thread");
     }
     wt.wait();
diff --git a/searchlib/src/tests/transactionlogstress/translogstress.cpp b/searchlib/src/tests/transactionlogstress/translogstress.cpp
index a516fb26604..5792da7aa18 100644
--- a/searchlib/src/tests/transactionlogstress/translogstress.cpp
+++ b/searchlib/src/tests/transactionlogstress/translogstress.cpp
@@ -18,8 +18,6 @@ LOG_SETUP("translogstress");
 
 using vespalib::nbostream;
 using search::Runnable;
-using vespalib::Monitor;
-using vespalib::MonitorGuard;
 using std::shared_ptr;
 using vespalib::make_string;
 using vespalib::ConstBufferRef;
@@ -116,7 +114,7 @@ Packet::Entry
 EntryGenerator::getRandomEntry(SerialNum num)
 {
     _rnd.srand48(_baseSeed + num);
-    if (_buffers != NULL) {
+    if (_buffers != nullptr) {
         size_t i = _rnd.lrand48() % _buffers->size();
         const nbostream& buffer = (*_buffers)[i];
         return Packet::Entry(num, 1024, ConstBufferRef(buffer.data(), buffer.size()));
@@ -209,7 +207,7 @@ private:
 public:
     FeederThread(const std::string & tlsSpec, const std::string & domain,
                  const EntryGenerator & generator, uint32_t feedRate, size_t packetSize);
-    ~FeederThread();
+    ~FeederThread() override;
     void doRun() override;
     SerialNumRange getRange() const { return SerialNumRange(1, _lastCommited); }
 };
@@ -247,7 +245,7 @@ void
 FeederThread::doRun()
 {
     _session = _client.open(_domain);
-    if (_session.get() == NULL) {
+    if ( ! _session) {
         throw std::runtime_error(vespalib::make_string("FeederThread: Could not open session to %s", _tlsSpec.c_str()));
     }
 
@@ -328,10 +326,10 @@ private:
     SerialNum _to;
     SerialNum _next;
     State _state;
-    Monitor _monitor;
+    std::mutex _monitor;
 
     void setState(State newState) {
-        MonitorGuard guard(_monitor);
+        std::lock_guard guard(_monitor);
         //LOG(info, "VisitorAgent[%u]: setState(%s)", _id, newState == IDLE ? "idle" :
         //    (newState == RUNNING ? "running" : "finished"));
         _state = newState;
@@ -343,23 +341,23 @@ public:
                  const EntryGenerator & generator, uint32_t id, bool validate) :
         Agent(tlsSpec, domain, generator, "VisitorAgent", id, validate),
         _visitor(), _from(0), _to(0), _next(0), _state(IDLE) {}
-    virtual ~VisitorAgent() {}
+    ~VisitorAgent() override = default;
     void start(SerialNum from, SerialNum to);
     void setIdle();
     bool idle() {
-        MonitorGuard guard(_monitor);
+        std::lock_guard guard(_monitor);
         return _state == IDLE;
     }
     bool running() {
-        MonitorGuard guard(_monitor);
+        std::lock_guard guard(_monitor);
         return _state == RUNNING;
     }
     bool finished() {
-        MonitorGuard guard(_monitor);
+        std::lock_guard guard(_monitor);
         return _state == FINISHED;
     }
     std::string getState() {
-        MonitorGuard guard(_monitor);
+        std::lock_guard guard(_monitor);
         if (_state == IDLE) {
             return std::string("idle");
         } else if (_state == FINISHED) {
@@ -516,7 +514,7 @@ void
 ControllerThread::doRun()
 {
     _session = _client.open(_domain);
-    if (_session.get() == NULL) {
+    if ( ! _session) {
         throw std::runtime_error(vespalib::make_string("ControllerThread: Could not open session to %s", _tlsSpec.c_str()));
     }
 
diff --git a/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp b/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp
index 40db92cbe78..11568ad2e36 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/client_session.cpp
@@ -9,7 +9,6 @@
 #include <vespa/log/log.h>
 LOG_SETUP(".translog.client_session");
 
-using vespalib::LockGuard;
 using namespace std::chrono_literals;
 
 namespace search::transactionlog::client {
diff --git a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp
index 9efc68bc8ec..07da8087544 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/domain.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/domain.cpp
@@ -22,8 +22,6 @@ using vespalib::make_string_short::fmt;
 using vespalib::makeTask;
 using vespalib::makeClosure;
 using vespalib::makeLambdaTask;
-using vespalib::Monitor;
-using vespalib::MonitorGuard;
 using std::runtime_error;
 using std::make_shared;
 
diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp b/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp
index 84919a59a97..1883943a53f 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/translogclient.cpp
@@ -45,8 +45,6 @@ struct RpcTask : public vespalib::Executor::Task {
 
 }
 
-using vespalib::LockGuard;
-
 TransLogClient::TransLogClient(const vespalib::string & rpcTarget) :
     _executor(std::make_unique<vespalib::ThreadStackExecutor>(1, 128 * 1024, translogclient_rpc_callback)),
     _rpcTarget(rpcTarget),
diff --git a/slobrok/src/vespa/slobrok/sbmirror.cpp b/slobrok/src/vespa/slobrok/sbmirror.cpp
index 855ed4dd82d..460d61cb2a8 100644
--- a/slobrok/src/vespa/slobrok/sbmirror.cpp
+++ b/slobrok/src/vespa/slobrok/sbmirror.cpp
@@ -7,10 +7,7 @@
 #include <vespa/log/log.h>
 LOG_SETUP(".slobrok.mirror");
 
-using vespalib::LockGuard;
-
-namespace slobrok {
-namespace api {
+namespace slobrok::api {
 
 MirrorAPI::MirrorAPI(FRT_Supervisor &orb, const ConfiguratorFactory & config)
     : FNET_Task(orb.GetScheduler()),
@@ -326,5 +323,4 @@ MirrorAPI::RequestDone(FRT_RPCRequest *req)
     ScheduleNow();
 }
 
-} // namespace api
-} // namespace slobrok
+}
diff --git a/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp b/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp
index 5dacaa5d204..622c9b9985f 100644
--- a/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp
+++ b/staging_vespalib/src/tests/singleexecutor/singleexecutor_test.cpp
@@ -28,7 +28,8 @@ TEST("test that all tasks are executed") {
 }
 
 void verifyResizeTaskLimit(bool up) {
-    Monitor lock;
+    std::mutex lock;
+    std::condition_variable cond;
     std::atomic<uint64_t> started(0);
     std::atomic<uint64_t> allowed(0);
     SingleExecutor executor(10);
@@ -38,11 +39,11 @@ void verifyResizeTaskLimit(bool up) {
     EXPECT_NOT_EQUAL(16u, roundedTaskLimit);
 
     for (uint64_t i(0); i < 10; i++) {
-        executor.execute(makeLambdaTask([&lock, &started, &allowed] {
+        executor.execute(makeLambdaTask([&lock, &cond, &started, &allowed] {
             started++;
-            MonitorGuard guard(lock);
+            std::unique_lock guard(lock);
             while (allowed < started) {
-                guard.wait(1ms);
+                cond.wait_for(guard, 1ms);
             }
         }));
     }
@@ -58,11 +59,11 @@ void verifyResizeTaskLimit(bool up) {
     while (started < 10);
     EXPECT_EQUAL(10u, started);
     EXPECT_EQUAL(16u, executor.getTaskLimit());
-    executor.execute(makeLambdaTask([&lock, &started, &allowed] {
+    executor.execute(makeLambdaTask([&lock, &cond, &started, &allowed] {
         started++;
-        MonitorGuard guard(lock);
+        std::unique_lock guard(lock);
         while (allowed < started) {
-            guard.wait(1ms);
+            cond.wait_for(guard, 1ms);
         }
     }));
     while (started < 11);
diff --git a/storage/src/tests/storageserver/dummystoragelink.cpp b/storage/src/tests/storageserver/dummystoragelink.cpp
deleted file mode 100644
index ab7c413f2ea..00000000000
--- a/storage/src/tests/storageserver/dummystoragelink.cpp
+++ /dev/null
@@ -1,181 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#include <vespa/storageframework/defaultimplementation/clock/realclock.h>
-#include <tests/common/dummystoragelink.h>
-#include <sys/time.h>
-
-namespace storage {
-
-DummyStorageLink* DummyStorageLink::_last(0);
-
-DummyStorageLink::DummyStorageLink()
-    : StorageLink("Dummy storage link"),
-      _commands(),
-      _replies(),
-      _injected(),
-      _autoReply(false),
-      _useDispatch(false),
-      _ignore(false),
-      _waitMonitor()
-{
-    _last = this;
-}
-
-DummyStorageLink::~DummyStorageLink()
-{
-        // Often a chain with dummy link on top is deleted in unit tests.
-        // If they haven't been closed already, close them for a cleaner
-        // shutdown
-    if (getState() == OPENED) {
-        close();
-        flush();
-    }
-    closeNextLink();
-    reset();
-}
-
-bool DummyStorageLink::onDown(const api::StorageMessage::SP& cmd)
-{
-    if (_ignore) {
-        return false;
-    }
-    if (_injected.size() > 0) {
-        vespalib::LockGuard guard(_lock);
-        sendUp(*_injected.begin());
-        _injected.pop_front();
-    } else if (_autoReply) {
-        if (!cmd->getType().isReply()) {
-            std::shared_ptr<api::StorageReply> reply(
-                    std::dynamic_pointer_cast<api::StorageCommand>(cmd)
-                    ->makeReply().release());
-            reply->setResult(api::ReturnCode(
-                    api::ReturnCode::OK, "Automatically generated reply"));
-            sendUp(reply);
-        }
-    }
-    if (isBottom()) {
-        vespalib::MonitorGuard lock(_waitMonitor);
-        {
-            vespalib::LockGuard guard(_lock);
-            _commands.push_back(cmd);
-        }
-        lock.broadcast();
-        return true;
-    }
-    return StorageLink::onDown(cmd);
-}
-
-bool DummyStorageLink::onUp(const api::StorageMessage::SP& reply) {
-    if (isTop()) {
-        vespalib::MonitorGuard lock(_waitMonitor);
-        {
-            vespalib::LockGuard guard(_lock);
-            _replies.push_back(reply);
-        }
-        lock.broadcast();
-        return true;
-    }
-    return StorageLink::onUp(reply);
-
-}
-
-void DummyStorageLink::injectReply(api::StorageReply* reply)
-{
-    assert(reply);
-    vespalib::LockGuard guard(_lock);
-    _injected.push_back(std::shared_ptr<api::StorageReply>(reply));
-}
-
-void DummyStorageLink::reset() {
-    vespalib::MonitorGuard lock(_waitMonitor);
-    vespalib::LockGuard guard(_lock);
-    _commands.clear();
-    _replies.clear();
-    _injected.clear();
-}
-
-void DummyStorageLink::waitForMessages(unsigned int msgCount, int timeout)
-{
-    framework::defaultimplementation::RealClock clock;
-    framework::MilliSecTime endTime(
-            clock.getTimeInMillis() + framework::MilliSecTime(timeout * 1000));
-    vespalib::MonitorGuard lock(_waitMonitor);
-    while (_commands.size() + _replies.size() < msgCount) {
-        if (timeout != 0 && clock.getTimeInMillis() > endTime) {
-            std::ostringstream ost;
-            ost << "Timed out waiting for " << msgCount << " messages to "
-                << "arrive in dummy storage link. Only "
-                << (_commands.size() + _replies.size()) << " messages seen "
-                << "after timout of " << timeout << " seconds was reached.";
-            throw vespalib::IllegalStateException(ost.str(), VESPA_STRLOC);
-        }
-        if (timeout >= 0) {
-            lock.wait((endTime - clock.getTimeInMillis()).getTime());
-        } else {
-            lock.wait();
-        }
-    }
-}
-
-void DummyStorageLink::waitForMessage(const api::MessageType& type, int timeout)
-{
-    framework::defaultimplementation::RealClock clock;
-    framework::MilliSecTime endTime(
-            clock.getTimeInMillis() + framework::MilliSecTime(timeout * 1000));
-    vespalib::MonitorGuard lock(_waitMonitor);
-    while (true) {
-        for (uint32_t i=0; i<_commands.size(); ++i) {
-            if (_commands[i]->getType() == type) return;
-        }
-        for (uint32_t i=0; i<_replies.size(); ++i) {
-            if (_replies[i]->getType() == type) return;
-        }
-        if (timeout != 0 && clock.getTimeInMillis() > endTime) {
-            std::ostringstream ost;
-            ost << "Timed out waiting for " << type << " message to "
-                << "arrive in dummy storage link. Only "
-                << (_commands.size() + _replies.size()) << " messages seen "
-                << "after timout of " << timeout << " seconds was reached.";
-            if (_commands.size() == 1) {
-                ost << " Found command of type " << _commands[0]->getType();
-            }
-            if (_replies.size() == 1) {
-                ost << " Found command of type " << _replies[0]->getType();
-            }
-            throw vespalib::IllegalStateException(ost.str(), VESPA_STRLOC);
-        }
-        if (timeout >= 0) {
-            lock.wait((endTime - clock.getTimeInMillis()).getTime());
-        } else {
-            lock.wait();
-        }
-    }
-}
-
-api::StorageMessage::SP
-DummyStorageLink::getAndRemoveMessage(const api::MessageType& type)
-{
-    vespalib::MonitorGuard lock(_waitMonitor);
-    for (std::vector<api::StorageMessage::SP>::iterator it = _commands.begin();
-         it != _commands.end(); ++it)
-    {
-        if ((*it)->getType() == type) {
-            api::StorageMessage::SP result(*it);
-            _commands.erase(it);
-            return result;
-        }
-    }
-    for (std::vector<api::StorageMessage::SP>::iterator it = _replies.begin();
-         it != _replies.end(); ++it)
-    {
-        if ((*it)->getType() == type) {
-            api::StorageMessage::SP result(*it);
-            _replies.erase(it);
-            return result;
-        }
-    }
-    std::ostringstream ost;
-    ost << "No message of type " << type << " found.";
-    throw vespalib::IllegalStateException(ost.str(), VESPA_STRLOC);
-}
-
-} // storage
diff --git a/storage/src/tests/storageserver/dummystoragelink.h b/storage/src/tests/storageserver/dummystoragelink.h
deleted file mode 100644
index 3902e1db19c..00000000000
--- a/storage/src/tests/storageserver/dummystoragelink.h
+++ /dev/null
@@ -1,115 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <vespa/vespalib/util/sync.h>
-#include <list>
-#include <sstream>
-#include <vespa/storageapi/messageapi/storagecommand.h>
-#include <string>
-#include <vector>
-#include <vespa/storage/common/storagelink.h>
-#include <vespa/storage/common/bucketmessages.h>
-#include <vespa/storageapi/message/internal.h>
-
-class FastOS_ThreadPool;
-
-namespace storage {
-
-class DummyStorageLink : public StorageLink {
-
-    mutable vespalib::Lock _lock; // to protect below containers:
-    std::vector<api::StorageMessage::SP> _commands;
-    std::vector<api::StorageMessage::SP> _replies;
-    std::list<api::StorageMessage::SP> _injected;
-
-    bool _autoReply;
-    bool _useDispatch;
-    bool _ignore;
-    static DummyStorageLink* _last;
-    vespalib::Monitor _waitMonitor;
-
-public:
-    DummyStorageLink();
-    ~DummyStorageLink();
-
-    bool onDown(const api::StorageMessage::SP&);
-    bool onUp(const api::StorageMessage::SP&);
-
-    void addOnTopOfChain(StorageLink& link) {
-        link.addTestLinkOnTop(this);
-    }
-
-    void print(std::ostream& ost, bool verbose, const std::string& indent) const
-    {
-        (void) verbose;
-        ost << indent << "DummyStorageLink("
-            << "autoreply = " << (_autoReply ? "on" : "off")
-            << ", dispatch = " << (_useDispatch ? "on" : "off")
-            << ", " << _commands.size() << " commands"
-            << ", " << _replies.size() << " replies";
-        if (_injected.size() > 0)
-            ost << ", " << _injected.size() << " injected";
-        ost << ")";
-    }
-
-    void injectReply(api::StorageReply* reply);
-    void reset();
-    void setAutoreply(bool autoReply) { _autoReply = autoReply; }
-    void setIgnore(bool ignore) { _ignore = ignore; }
-    // Timeout is given in seconds
-    void waitForMessages(unsigned int msgCount = 1, int timeout = -1);
-    // Wait for a single message of a given type
-    void waitForMessage(const api::MessageType&, int timeout = -1);
-
-    api::StorageMessage::SP getCommand(size_t i) const {
-        vespalib::LockGuard guard(_lock);
-        api::StorageMessage::SP ret = _commands[i];
-        return ret;
-    }
-    api::StorageMessage::SP getReply(size_t i) const {
-        vespalib::LockGuard guard(_lock);
-        api::StorageMessage::SP ret = _replies[i];
-        return ret;
-    }
-    size_t getNumCommands() const {
-        vespalib::LockGuard guard(_lock);
-        return _commands.size();
-    }
-    size_t getNumReplies() const {
-        vespalib::LockGuard guard(_lock);
-        return _replies.size();
-    }
-
-    const std::vector<api::StorageMessage::SP>& getCommands() const
-        { return _commands; }
-    const std::vector<api::StorageMessage::SP>& getReplies() const
-        { return _replies; }
-
-    std::vector<api::StorageMessage::SP> getCommandsOnce() {
-        vespalib::MonitorGuard lock(_waitMonitor);
-        std::vector<api::StorageMessage::SP> retval;
-        {
-            vespalib::LockGuard guard(_lock);
-            retval.swap(_commands);
-        }
-        return retval;
-    }
-
-    std::vector<api::StorageMessage::SP> getRepliesOnce() {
-        vespalib::MonitorGuard lock(_waitMonitor);
-        std::vector<api::StorageMessage::SP> retval;
-        {
-            vespalib::LockGuard guard(_lock);
-            retval.swap(_replies);
-        }
-        return retval;
-    }
-
-    api::StorageMessage::SP getAndRemoveMessage(const api::MessageType&);
-
-    static DummyStorageLink* getLast() { return _last; }
-};
-
-}
-
diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp
index 3c2d040010c..237acbb2d2c 100644
--- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp
+++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp
@@ -2,7 +2,6 @@
 
 #include "statuswebserver.h"
 #include <vespa/storageframework/storageframework.h>
-#include <vespa/storageapi/message/persistence.h>
 #include <vespa/vespalib/util/host_name.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/component/vtag.h>
@@ -19,7 +18,6 @@ StatusWebServer::StatusWebServer(
         framework::StatusReporterMap& reporterMap,
         const config::ConfigUri & configUri)
     : _reporterMap(reporterMap),
-      _workerMonitor(),
       _port(0),
       _httpServer(),
       _configFetcher(configUri.getContext()),
@@ -34,11 +32,11 @@ StatusWebServer::~StatusWebServer()
     // Avoid getting config during shutdown
     _configFetcher.close();
 
-    if (_httpServer.get() != 0) {
+    if (_httpServer) {
         LOG(debug, "Shutting down status web server on port %u", _httpServer->getListenPort());
     }
     // Delete http server to ensure that no more incoming requests reach us.
-    _httpServer.reset(0);
+    _httpServer.reset();
 }
 
 void StatusWebServer::configure(std::unique_ptr<vespa::config::content::core::StorStatusConfig> config)
@@ -55,7 +53,7 @@ void StatusWebServer::configure(std::unique_ptr<vespa::config::content::core::St
         // Negative port number means don't run the web server
     if (newPort >= 0) {
         try {
-            server.reset(new WebServer(*this, newPort));
+            server = std::make_unique<WebServer>(*this, newPort);
         } catch (const vespalib::PortListenException & e) {
             LOG(error, "Failed listening to network port(%d) with protocol(%s): '%s', giving up and restarting.",
                 e.get_port(), e.get_protocol().c_str(), e.what());
@@ -148,7 +146,7 @@ void
 StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, vespalib::Portal::GetRequest request)
 {
     vespalib::string link(urlpath.getPath());
-    if (link.size() > 0 && link[0] == '/') link = link.substr(1);
+    if (!link.empty() && link[0] == '/') link = link.substr(1);
 
     size_t slashPos = link.find('/');
     if (slashPos != std::string::npos) link = link.substr(0, slashPos);
diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h
index e94af34346a..5955339d43b 100644
--- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h
+++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h
@@ -62,7 +62,6 @@ class StatusWebServer : private config::IFetcherCallback<vespa::config::content:
     };
 
     framework::StatusReporterMap&          _reporterMap;
-    vespalib::Monitor                      _workerMonitor;
     uint16_t                               _port;
     std::unique_ptr<WebServer>             _httpServer;
     config::ConfigFetcher                  _configFetcher;
diff --git a/storage/src/vespa/storage/visiting/visitorlibraries.cpp b/storage/src/vespa/storage/visiting/visitorlibraries.cpp
deleted file mode 100644
index 39dc49ddf10..00000000000
--- a/storage/src/vespa/storage/visiting/visitorlibraries.cpp
+++ /dev/null
@@ -1,70 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#include "visitorlibraries.h"
-#include <vespa/defaults.h>
-
-#include <vespa/log/log.h>
-
-LOG_SETUP(".visiting.libraryloader");
-
-namespace storage {
-
-VisitorLibraries::LibMap VisitorLibraries::_libs;
-vespalib::Lock VisitorLibraries::_libLock;
-
-/**
- * Utility function to get a dynamic library.
- * Assumes _libLock has been grabbed before calling.
- */
-VisitorLibraries::LibraryRef
-VisitorLibraries::getLibrary(StorageServerInterface& storageServer, const std::string& libName, const std::string& libraryPath)
-{
-    vespalib::LockGuard guard(_libLock);
-
-    LibMap::iterator it = _libs.find(libName);
-    if (it != _libs.end()) {
-        return LibraryRef(it->second.factory, it->second.environment.get());
-    }
-
-    std::shared_ptr<FastOS_DynamicLibrary> lib(new FastOS_DynamicLibrary);
-    std::string file = libraryPath + "lib" + libName + ".so";
-    if (!lib->Open(file.c_str())) {
-        std::string error = lib->GetLastErrorString();
-        std::string absfile = vespa::Defaults::underVespaHome("libexec/vespa/storage/");
-        absfile.append("lib" + libName + ".so");
-        if (!lib->Open(absfile.c_str())) {
-            LOG(error, "Could not load library %s: %s",
-                       file.c_str(), error.c_str());
-            return LibraryRef();
-        }
-    }
-    std::shared_ptr<VisitorEnvironment> env(
-            getVisitorEnvironment(storageServer, *lib, libName));
-
-    LibMapEntry entry;
-    entry.library = lib;
-    entry.environment = env;
-    entry.factory = lib.get() ? (VisitorFactoryFuncT) lib->GetSymbol("makeVisitor") : 0;
-    _libs[libName] = entry;
-
-    return LibraryRef(entry.factory, env.get());
-}
-
-std::shared_ptr<VisitorEnvironment>
-VisitorLibraries::getVisitorEnvironment(StorageServerInterface& storageServer, FastOS_DynamicLibrary& lib,
-                                     const std::string& libName)
-{
-    typedef VisitorEnvironment::UP
-            (*VisitorEnvFuncT)(StorageServerInterface& server);
-    VisitorEnvFuncT factoryFunc
-        = (VisitorEnvFuncT) lib.GetSymbol("makeVisitorEnvironment");
-    if (factoryFunc == 0) {
-        std::string err = lib.GetLastErrorString();
-        LOG(error, "Unable to load symbol 'makeVisitorEnvironment' from "
-                   "'%s': %s", libName.c_str(), err.c_str());
-        return std::shared_ptr<VisitorEnvironment>();
-    }
-    return std::shared_ptr<VisitorEnvironment>(
-            factoryFunc(storageServer).release());
-}
-
-}
diff --git a/storage/src/vespa/storage/visiting/visitorlibraries.h b/storage/src/vespa/storage/visiting/visitorlibraries.h
deleted file mode 100644
index 74778b0483d..00000000000
--- a/storage/src/vespa/storage/visiting/visitorlibraries.h
+++ /dev/null
@@ -1,38 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#pragma once
-
-/**
-   This class handles ownership and creation of dynamic visitor libraries.
-*/
-
-#include "visitor.h"
-
-namespace storage {
-
-class VisitorLibraries {
-public:
-    typedef Visitor* (*VisitorFactoryFuncT)(StorageServerInterface& server,
-                                            VisitorEnvironment& env,
-                                            const vdslib::Parameters& params);
-
-    struct LibMapEntry {
-        std::shared_ptr<FastOS_DynamicLibrary> library;
-        std::shared_ptr<VisitorEnvironment> environment;
-        VisitorFactoryFuncT factory;
-    };
-
-    typedef std::map<std::string, LibMapEntry> LibMap;
-    typedef std::pair<VisitorFactoryFuncT, VisitorEnvironment*> LibraryRef;
-
-    static LibraryRef getLibrary(StorageServerInterface& storageServer, const std::string& libName, const std::string& libraryPath);
-
-private:
-    static LibMap _libs;
-    static vespalib::Lock _libLock;
-
-    static std::shared_ptr<VisitorEnvironment> getVisitorEnvironment(StorageServerInterface& storageServer,
-                                          FastOS_DynamicLibrary& lib,
-                                          const std::string& libName);
-};
-
-}
diff --git a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp
index e0d78c5d1b7..4f93f891cf8 100644
--- a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp
+++ b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.cpp
@@ -50,6 +50,14 @@ Runnable::UP chain(Runnable::UP first, Runnable::UP second) {
 
 //-----------------------------------------------------------------------------
 
+Signal::Signal() noexcept
+    : valid(true),
+      generation(0),
+      monitor(std::make_unique<std::mutex>()),
+      cond(std::make_unique<std::condition_variable>())
+{}
+Signal::~Signal() = default;
+
 SimpleThreadBundle::Pool::Pool(size_t bundleSize)
     : _lock(),
       _bundleSize(bundleSize),
diff --git a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h
index 135fc2d7562..40844b277f1 100644
--- a/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h
+++ b/vespalib/src/vespa/vespalib/util/simple_thread_bundle.h
@@ -6,7 +6,6 @@
 #include "thread.h"
 #include "runnable.h"
 #include "thread_bundle.h"
-#include "noncopyable.hpp"
 
 namespace vespalib {
 
@@ -46,32 +45,35 @@ struct Part {
 struct Signal {
     bool valid;
     size_t generation;
-    Monitor monitor;
-    Signal() noexcept : valid(true), generation(0), monitor() {}
+    std::unique_ptr<std::mutex> monitor;
+    std::unique_ptr<std::condition_variable> cond;
+    Signal() noexcept;
+    Signal(Signal &&) noexcept = default;
+    ~Signal();
     size_t wait(size_t &localGen) {
-        MonitorGuard guard(monitor);
+        std::unique_lock guard(*monitor);
         while (localGen == generation) {
-            guard.wait();
+            cond->wait(guard);
         }
         size_t diff = (generation - localGen);
         localGen = generation;
         return (valid ? diff : 0);
     }
     void send() {
-        MonitorGuard guard(monitor);
+        std::lock_guard guard(*monitor);
         ++generation;
-        guard.signal();
+        cond->notify_one();
     }
     void broadcast() {
-        MonitorGuard guard(monitor);
+        std::lock_guard guard(*monitor);
         ++generation;
-        guard.broadcast();
+        cond->notify_all();
     }
     void cancel() {
-        MonitorGuard guard(monitor);
+        std::lock_guard guard(*monitor);
         ++generation;
         valid = false;
-        guard.broadcast();
+        cond->notify_all();
     }
 };
 
@@ -105,7 +107,7 @@ public:
     };
 
 private:
-    struct Worker : Runnable, noncopyable {
+    struct Worker : Runnable {
         using UP = std::unique_ptr<Worker>;
         Thread thread;
         Signal &signal;