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

6 files changed, 4 insertions, 411 deletions

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);
-};
-
-}