Simplify the supervisor responsibility

11 files changed, 98 insertions, 74 deletions

diff --git a/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp b/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp
index 3407a880ec7..ce0665eae4a 100644
--- a/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp
+++ b/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp
@@ -16,41 +16,41 @@ Flags::Flags()
       targethost("localhost"),
       portnumber(19090)
 { }
-Flags::~Flags() { }
+Flags::~Flags() = default;
 
 ProxyCmd::ProxyCmd(const Flags& flags)
-    : _supervisor(NULL),
-      _target(NULL),
-      _req(NULL),
+    : _server(),
+      _supervisor(nullptr),
+      _target(nullptr),
+      _req(nullptr),
       _flags(flags)
 { }
 
-ProxyCmd::~ProxyCmd() { }
+ProxyCmd::~ProxyCmd() = default;
 
 void ProxyCmd::initRPC() {
-    _supervisor = new FRT_Supervisor();
+    _server = std::make_unique<fnet::frt::StandaloneFRT>();
+    _supervisor = &_server->supervisor();
     _req = _supervisor->AllocRPCRequest();
-    _supervisor->Start();
 }
 
 void ProxyCmd::invokeRPC() {
-    if (_req == NULL) return;
+    if (_req == nullptr) return;
     _target->InvokeSync(_req, 65.0);
 }
 
 void ProxyCmd::finiRPC() {
-    if (_req != NULL) {
+    if (_req != nullptr) {
         _req->SubRef();
-        _req = NULL;
+        _req = nullptr;
     }
-    if (_target != NULL) {
+    if (_target != nullptr) {
         _target->SubRef();
         _target = NULL;
     }
-    if (_supervisor != NULL) {
-        _supervisor->ShutDown(true);
-        delete _supervisor;
-        _supervisor = NULL;
+    if (_server) {
+        _server.reset();
+        _supervisor = nullptr;
     }
 }
 
diff --git a/config/src/apps/vespa-configproxy-cmd/proxycmd.h b/config/src/apps/vespa-configproxy-cmd/proxycmd.h
index df8f429a3c9..49be0fff885 100644
--- a/config/src/apps/vespa-configproxy-cmd/proxycmd.h
+++ b/config/src/apps/vespa-configproxy-cmd/proxycmd.h
@@ -9,6 +9,8 @@ class FRT_Target;
 class FRT_RPCRequest;
 class FRT_Values;
 
+namespace fnet::frt { class StandaloneFRT; }
+
 struct Flags {
     vespalib::string method;
     std::vector<vespalib::string> args;
@@ -23,6 +25,7 @@ struct Flags {
 class ProxyCmd
 {
 private:
+    std::unique_ptr<fnet::frt::StandaloneFRT> _server;
     FRT_Supervisor *_supervisor;
     FRT_Target     *_target;
     FRT_RPCRequest *_req;
diff --git a/config/src/apps/vespa-get-config/getconfig.cpp b/config/src/apps/vespa-get-config/getconfig.cpp
index 290beacf5a4..f23f28b2734 100644
--- a/config/src/apps/vespa-get-config/getconfig.cpp
+++ b/config/src/apps/vespa-get-config/getconfig.cpp
@@ -20,6 +20,7 @@ using namespace config;
 class GetConfig : public FastOS_Application
 {
 private:
+    std::unique_ptr<fnet::frt::StandaloneFRT> _server;
     FRT_Supervisor *_supervisor;
     FRT_Target     *_target;
 
@@ -27,7 +28,7 @@ private:
     GetConfig &operator=(const GetConfig &);
 
 public:
-    GetConfig() : _supervisor(NULL), _target(NULL) {}
+    GetConfig() : _server(), _supervisor(nullptr), _target(nullptr) {}
     virtual ~GetConfig();
     int usage();
     void initRPC(const char *spec);
@@ -38,8 +39,8 @@ public:
 
 GetConfig::~GetConfig()
 {
-    LOG_ASSERT(_supervisor == NULL);
-    LOG_ASSERT(_target == NULL);
+    LOG_ASSERT(_supervisor == nullptr);
+    LOG_ASSERT(_target == nullptr);
 }
 
 
@@ -71,23 +72,22 @@ GetConfig::usage()
 void
 GetConfig::initRPC(const char *spec)
 {
-    _supervisor = new FRT_Supervisor();
+    _server = std::make_unique<fnet::frt::StandaloneFRT>();
+    _supervisor = &_server->supervisor();
     _target     = _supervisor->GetTarget(spec);
-    _supervisor->Start();
 }
 
 
 void
 GetConfig::finiRPC()
 {
-    if (_target != NULL) {
+    if (_target != nullptr) {
         _target->SubRef();
-        _target = NULL;
+        _target = nullptr;
     }
-    if (_supervisor != NULL) {
-        _supervisor->ShutDown(true);
-        delete _supervisor;
-        _supervisor = NULL;
+    if (_server) {
+        _server.reset();
+        _supervisor = nullptr;
     }
 }
 
@@ -99,8 +99,8 @@ GetConfig::Main()
     char c = -1;
 
     std::vector<vespalib::string> defSchema;
-    const char *schema = NULL;
-    const char *defName = NULL;
+    const char *schema = nullptr;
+    const char *defName = nullptr;
     const char *defMD5 = "";
     std::string defNamespace("config");
     const char *serverHost = "localhost";
@@ -110,7 +110,7 @@ GetConfig::Main()
     const char *vespaVersionString = nullptr;
     int64_t generation = 0;
 
-    if (configId == NULL) {
+    if (configId == nullptr) {
         configId = "";
     }
     const char *configMD5 = "";
@@ -119,7 +119,7 @@ GetConfig::Main()
 
     int serverPort = 19090;
 
-    const char *optArg = NULL;
+    const char *optArg = nullptr;
     int optInd = 0;
     while ((c = GetOpt("a:n:v:g:i:jlm:c:t:V:w:r:s:p:dh", optArg, optInd)) != -1) {
         int retval = 1;
@@ -181,19 +181,19 @@ GetConfig::Main()
         }
     }
 
-    if (defName == NULL || serverPort == 0) {
+    if (defName == nullptr || serverPort == 0) {
         usage();
         return 1;
     }
 
-    if (strchr(defName, '.') != NULL) {
+    if (strchr(defName, '.') != nullptr) {
         const char *tmp = defName;
         defName = strrchr(defName, '.');
         defName++;
         defNamespace = std::string(tmp, defName - tmp - 1);
     }
 
-    if (schema != NULL) {
+    if (schema != nullptr) {
         std::ifstream is;
         is.open(schema);
         std::string item;
diff --git a/config/src/apps/vespa-ping-configproxy/pingproxy.cpp b/config/src/apps/vespa-ping-configproxy/pingproxy.cpp
index a88d7deb79a..d681473ce34 100644
--- a/config/src/apps/vespa-ping-configproxy/pingproxy.cpp
+++ b/config/src/apps/vespa-ping-configproxy/pingproxy.cpp
@@ -12,6 +12,7 @@ LOG_SETUP("vespa-ping-configproxy");
 class PingProxy : public FastOS_Application
 {
 private:
+    std::unique_ptr<fnet::frt::StandaloneFRT> _server;
     FRT_Supervisor *_supervisor;
     FRT_Target     *_target;
 
@@ -19,7 +20,7 @@ private:
     PingProxy &operator=(const PingProxy &);
 
 public:
-    PingProxy() : _supervisor(NULL), _target(NULL) {}
+    PingProxy() : _server(), _supervisor(nullptr), _target(nullptr) {}
     virtual ~PingProxy();
     int usage();
     void initRPC(const char *spec);
@@ -30,8 +31,8 @@ public:
 
 PingProxy::~PingProxy()
 {
-    LOG_ASSERT(_supervisor == NULL);
-    LOG_ASSERT(_target == NULL);
+    LOG_ASSERT(_supervisor == nullptr);
+    LOG_ASSERT(_target == nullptr);
 }
 
 
@@ -48,23 +49,22 @@ PingProxy::usage()
 void
 PingProxy::initRPC(const char *spec)
 {
-    _supervisor = new FRT_Supervisor();
+    _server = std::make_unique<fnet::frt::StandaloneFRT>();
+    _supervisor = &_server->supervisor();
     _target     = _supervisor->GetTarget(spec);
-    _supervisor->Start();
 }
 
 
 void
 PingProxy::finiRPC()
 {
-    if (_target != NULL) {
+    if (_target != nullptr) {
         _target->SubRef();
-        _target = NULL;
+        _target = nullptr;
     }
-    if (_supervisor != NULL) {
-        _supervisor->ShutDown(true);
-        delete _supervisor;
-        _supervisor = NULL;
+    if (_server) {
+        _server.reset();
+        _supervisor = nullptr;
     }
 }
 
@@ -80,7 +80,7 @@ PingProxy::Main()
     int clientTimeout = 5;
     int serverPort = 19090;
 
-    const char *optArg = NULL;
+    const char *optArg = nullptr;
     int optInd = 0;
     while ((c = GetOpt("w:s:p:dh", optArg, optInd)) != -1) {
         switch (c) {
diff --git a/config/src/tests/failover/failover.cpp b/config/src/tests/failover/failover.cpp
index 990ca761e7e..7e7017ff057 100644
--- a/config/src/tests/failover/failover.cpp
+++ b/config/src/tests/failover/failover.cpp
@@ -34,7 +34,7 @@ struct RPCServer : public FRT_Invokable {
     vespalib::Barrier barrier;
     int64_t gen;
 
-    RPCServer() : supervisor(NULL), barrier(2), gen(1) { }
+    RPCServer() : supervisor(nullptr), barrier(2), gen(1) { }
 
     void init(FRT_Supervisor * s) {
         FRT_ReflectionBuilder rb(s);
@@ -81,18 +81,20 @@ struct RPCServer : public FRT_Invokable {
 
 void verifyConfig(std::unique_ptr<MyConfig> config)
 {
-    ASSERT_TRUE(config.get() != NULL);
+    ASSERT_TRUE(config);
     ASSERT_EQUAL("myval", config->myField);
 }
 
 struct ServerFixture {
     using UP = std::unique_ptr<ServerFixture>;
+    std::unique_ptr<fnet::frt::StandaloneFRT> frtServer;
     FRT_Supervisor * supervisor;
     RPCServer server;
     Barrier b;
     const vespalib::string listenSpec;
     ServerFixture(const vespalib::string & ls)
-        : supervisor(NULL),
+        : frtServer(),
+          supervisor(nullptr),
           server(),
           b(2),
           listenSpec(ls)
@@ -106,22 +108,21 @@ struct ServerFixture {
 
     void start()
     {
-        supervisor = new FRT_Supervisor();
+        frtServer = std::make_unique<fnet::frt::StandaloneFRT>();
+        supervisor = & frtServer->supervisor();
         server.init(supervisor);
         supervisor->Listen(get_port(listenSpec));
         wait(); // Wait until test runner signals we can start
-        supervisor->Main();
         wait(); // Signalling that we have shut down
         wait(); // Wait for signal saying that supervisor is deleted
     }
 
     void stop()
     {
-        if (supervisor != NULL) {
-            supervisor->ShutDown(true);
+        if (frtServer) {
             wait(); // Wait for supervisor to shut down
-            delete supervisor;
-            supervisor = NULL;
+            frtServer.reset();
+            supervisor = nullptr;
             wait(); // Signal that we are done and start can return.
         }
     }
diff --git a/config/src/tests/file_acquirer/file_acquirer_test.cpp b/config/src/tests/file_acquirer/file_acquirer_test.cpp
index 0453c6ddbd0..33bb8f47e09 100644
--- a/config/src/tests/file_acquirer/file_acquirer_test.cpp
+++ b/config/src/tests/file_acquirer/file_acquirer_test.cpp
@@ -7,8 +7,10 @@
 using namespace config;
 
 struct ServerFixture : FRT_Invokable {
-    FRT_Supervisor orb;
+    fnet::frt::StandaloneFRT server;
+    FRT_Supervisor &orb;
     vespalib::string spec;
+
     void init_rpc() {
         FRT_ReflectionBuilder rb(&orb);
         rb.DefineMethod("waitFor", "s", "s", FRT_METHOD(ServerFixture::RPC_waitFor), this);
@@ -16,24 +18,24 @@ struct ServerFixture : FRT_Invokable {
         rb.ParamDesc("file_ref", "file reference to wait for and resolve");
         rb.ReturnDesc("file_path", "actual path to the requested file");
     }
-    ServerFixture() : orb() {
+
+    ServerFixture() : server(), orb(server.supervisor()) {
         init_rpc();
         orb.Listen(0);
         spec = vespalib::make_string("tcp/localhost:%d", orb.GetListenPort());
-        orb.Start();
     }
+
     void RPC_waitFor(FRT_RPCRequest *req) {
         FRT_Values &params = *req->GetParams();
-        FRT_Values &ret    = *req->GetReturn();
+        FRT_Values &ret = *req->GetReturn();
         if (strcmp(params[0]._string._str, "my_ref") == 0) {
             ret.AddString("my_path");
         } else {
             req->SetError(FRTE_RPC_METHOD_FAILED, "invalid file reference");
         }
     }
-    ~ServerFixture() {
-        orb.ShutDown(true);
-    }
+
+    ~ServerFixture() = default;
 };
 
 TEST_FF("require that files can be acquired over rpc", ServerFixture(), RpcFileAcquirer(f1.spec)) {
diff --git a/config/src/tests/frt/frt.cpp b/config/src/tests/frt/frt.cpp
index c225dd9dcf3..f489ca4c7d9 100644
--- a/config/src/tests/frt/frt.cpp
+++ b/config/src/tests/frt/frt.cpp
@@ -119,17 +119,18 @@ namespace {
         int errorCode;
         int timeout;
         FRT_RPCRequest * ans;
-        FRT_Supervisor supervisor;
+        fnet::frt::StandaloneFRT server;
+        FRT_Supervisor & supervisor;
         FNET_Scheduler scheduler;
         vespalib::string address;
-        ConnectionMock(FRT_RPCRequest * answer = NULL);
+        ConnectionMock(FRT_RPCRequest * answer = nullptr);
         ~ConnectionMock();
         FRT_RPCRequest * allocRPCRequest() override { return supervisor.AllocRPCRequest(); }
         void setError(int ec) override { errorCode = ec; }
         void invoke(FRT_RPCRequest * req, double t, FRT_IRequestWait * waiter) override
         {
             timeout = static_cast<int>(t);
-            if (ans != NULL)
+            if (ans != nullptr)
                 waiter->RequestDone(ans);
             else
                 waiter->RequestDone(req);
@@ -142,10 +143,11 @@ namespace {
         : errorCode(0),
           timeout(0),
           ans(answer),
-          supervisor(),
+          server(),
+          supervisor(server.supervisor()),
           address()
     { }
-    ConnectionMock::~ConnectionMock() { }
+    ConnectionMock::~ConnectionMock() = default;
 
     struct FactoryMock : public ConnectionFactory {
         ConnectionMock * current;
@@ -284,7 +286,7 @@ TEST("require that v3 request is correctly initialized") {
     ConfigDefinition origDef(MyConfig::CONFIG_DEF_SCHEMA);
 
     FRT_RPCRequest * req = v3req.getRequest();
-    ASSERT_TRUE(req != NULL);
+    ASSERT_TRUE(req != nullptr);
     FRT_Values & params(*req->GetParams());
     std::string json(params[0]._string._str);
     Slime slime;
diff --git a/config/src/vespa/config/file_acquirer/file_acquirer.cpp b/config/src/vespa/config/file_acquirer/file_acquirer.cpp
index 140d63aa7e6..a61f480b33f 100644
--- a/config/src/vespa/config/file_acquirer/file_acquirer.cpp
+++ b/config/src/vespa/config/file_acquirer/file_acquirer.cpp
@@ -4,6 +4,8 @@
 #include <vespa/fnet/frt/supervisor.h>
 #include <vespa/fnet/frt/target.h>
 #include <vespa/fnet/frt/rpcrequest.h>
+#include <vespa/fnet/transport.h>
+#include <vespa/fastos/thread.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".config.file_acquirer");
@@ -11,10 +13,12 @@ LOG_SETUP(".config.file_acquirer");
 namespace config {
 
 RpcFileAcquirer::RpcFileAcquirer(const vespalib::string &spec)
-    : _orb(std::make_unique<FRT_Supervisor>()),
+    : _threadPool(std::make_unique<FastOS_ThreadPool>(1024*60)),
+      _transport(std::make_unique<FNET_Transport>()),
+      _orb(std::make_unique<FRT_Supervisor>(_transport.get())),
       _spec(spec)
 {
-    _orb->Start();
+    _transport->Start(_threadPool.get());
 }
 
 vespalib::string
@@ -39,7 +43,7 @@ RpcFileAcquirer::wait_for(const vespalib::string &file_ref, double timeout_s)
 
 RpcFileAcquirer::~RpcFileAcquirer()
 {
-    _orb->ShutDown(true);
+    _transport->ShutDown(true);
 }
 
 } // namespace config
diff --git a/config/src/vespa/config/file_acquirer/file_acquirer.h b/config/src/vespa/config/file_acquirer/file_acquirer.h
index 77b0cf9a821..844e277cbf9 100644
--- a/config/src/vespa/config/file_acquirer/file_acquirer.h
+++ b/config/src/vespa/config/file_acquirer/file_acquirer.h
@@ -4,6 +4,8 @@
 #include <vespa/vespalib/stllike/string.h>
 
 class FRT_Supervisor;
+class FNET_Transport;
+class FastOS_ThreadPool;
 
 namespace config {
 
@@ -23,12 +25,14 @@ struct FileAcquirer {
 class RpcFileAcquirer : public FileAcquirer
 {
 private:
+    std::unique_ptr<FastOS_ThreadPool> _threadPool;
+    std::unique_ptr<FNET_Transport> _transport;
     std::unique_ptr<FRT_Supervisor> _orb;
     vespalib::string _spec;
 public:
     RpcFileAcquirer(const vespalib::string &spec);
     vespalib::string wait_for(const vespalib::string &file_ref, double timeout_s) override;
-    ~RpcFileAcquirer();
+    ~RpcFileAcquirer() override;
 };
 
 } // namespace config
diff --git a/config/src/vespa/config/frt/frtconnectionpool.cpp b/config/src/vespa/config/frt/frtconnectionpool.cpp
index 4bea8062a03..b7440ceb7f0 100644
--- a/config/src/vespa/config/frt/frtconnectionpool.cpp
+++ b/config/src/vespa/config/frt/frtconnectionpool.cpp
@@ -4,6 +4,7 @@
 #include <vespa/vespalib/util/host_name.h>
 #include <vespa/fnet/frt/supervisor.h>
 #include <vespa/fnet/transport.h>
+#include <vespa/fastos/thread.h>
 
 namespace config {
 
@@ -26,7 +27,9 @@ FRTConnectionPool::FRTConnectionKey::operator==(const FRTConnectionKey& right) c
 }
 
 FRTConnectionPool::FRTConnectionPool(const ServerSpec & spec, const TimingValues & timingValues)
-    : _supervisor(std::make_unique<FRT_Supervisor>()),
+    :  _threadPool(std::make_unique<FastOS_ThreadPool>(1024*60)),
+       _transport(std::make_unique<FNET_Transport>()),
+       _supervisor(std::make_unique<FRT_Supervisor>(_transport.get())),
       _selectIdx(0),
       _hostname("")
 {
@@ -35,12 +38,12 @@ FRTConnectionPool::FRTConnectionPool(const ServerSpec & spec, const TimingValues
         _connections[key] = std::make_shared<FRTConnection>(spec.getHost(i), *_supervisor, timingValues);
     }
     setHostname();
-    _supervisor->Start();
+    _transport->Start(_threadPool.get());
 }
 
 FRTConnectionPool::~FRTConnectionPool()
 {
-    _supervisor->ShutDown(true);
+    _transport->ShutDown(true);
 }
 
 void
diff --git a/config/src/vespa/config/frt/frtconnectionpool.h b/config/src/vespa/config/frt/frtconnectionpool.h
index e671c756db5..0b30e723272 100644
--- a/config/src/vespa/config/frt/frtconnectionpool.h
+++ b/config/src/vespa/config/frt/frtconnectionpool.h
@@ -7,6 +7,9 @@
 #include <vector>
 #include <map>
 
+class FNET_Transport;
+class FastOS_ThreadPool;
+
 namespace config {
 
 class FRTConnectionPool : public ConnectionFactory {
@@ -29,6 +32,8 @@ private:
         int operator==(const FRTConnectionKey& right) const;
     };
 
+    std::unique_ptr<FastOS_ThreadPool> _threadPool;
+    std::unique_ptr<FNET_Transport> _transport;
     std::unique_ptr<FRT_Supervisor> _supervisor;
     int _selectIdx;
     vespalib::string _hostname;