- Remove provider, Handler,Waitable, PollAble, Interruptable as separate interface.

- Hold the lock when closing the ConfigHolder so that noone risk getting stuck inside the wait_until.
- Use wait_until instead of poll.

11 files changed, 22 insertions, 125 deletions

diff --git a/config/src/tests/configagent/configagent.cpp b/config/src/tests/configagent/configagent.cpp
index c2d68c5d9a0..81422d198e0 100644
--- a/config/src/tests/configagent/configagent.cpp
+++ b/config/src/tests/configagent/configagent.cpp
@@ -103,7 +103,7 @@ public:
     }
 
     bool poll() override { return true; }
-    void interrupt() override { }
+    void close() override { }
 private:
     std::unique_ptr<ConfigUpdate> _update;
 };
diff --git a/config/src/tests/configholder/configholder.cpp b/config/src/tests/configholder/configholder.cpp
index 5ff0efc92dc..e22ef55d747 100644
--- a/config/src/tests/configholder/configholder.cpp
+++ b/config/src/tests/configholder/configholder.cpp
@@ -65,7 +65,7 @@ TEST("Require that negative time does not mean forever.") {
     EXPECT_LESS(timer.elapsed(), ONE_MINUTE);
 }
 
-TEST_MT_F("Require that wait is interrupted", 2, ConfigHolder)
+TEST_MT_F("Require that wait is interrupted on close", 2, ConfigHolder)
 {
     if (thread_id == 0) {
         vespalib::Timer timer;
@@ -77,7 +77,7 @@ TEST_MT_F("Require that wait is interrupted", 2, ConfigHolder)
     } else {
         TEST_BARRIER();
         std::this_thread::sleep_for(500ms);
-        f.interrupt();
+        f.close();
         TEST_BARRIER();
     }
 }
diff --git a/config/src/vespa/config/common/configholder.cpp b/config/src/vespa/config/common/configholder.cpp
index d61e06ba0e6..e0f1795a1d4 100644
--- a/config/src/vespa/config/common/configholder.cpp
+++ b/config/src/vespa/config/common/configholder.cpp
@@ -46,8 +46,10 @@ ConfigHolder::poll()
 }
 
 void
-ConfigHolder::interrupt()
+ConfigHolder::close()
 {
+    std::lock_guard guard(_lock);
+    _current.reset();
     _cond.notify_all();
 }
 
diff --git a/config/src/vespa/config/common/configholder.h b/config/src/vespa/config/common/configholder.h
index c84e0718931..b3cb2d01abf 100644
--- a/config/src/vespa/config/common/configholder.h
+++ b/config/src/vespa/config/common/configholder.h
@@ -20,7 +20,7 @@ public:
     void handle(std::unique_ptr<ConfigUpdate> update) override;
     bool wait_until(vespalib::steady_time deadline) override;
     bool poll() override;
-    void interrupt() override;
+    void close() override;
 public:
     std::mutex                    _lock;
     std::condition_variable       _cond;
diff --git a/config/src/vespa/config/common/handler.h b/config/src/vespa/config/common/handler.h
deleted file mode 100644
index 5458f916357..00000000000
--- a/config/src/vespa/config/common/handler.h
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <memory>
-#include "configupdate.h"
-
-namespace config {
-
-/**
- * A Handler is a component to whom you can pass an object.
- **/
-template <typename T>
-struct Handler
-{
-    virtual void handle(std::unique_ptr<T> obj) = 0;
-    virtual ~Handler() {}
-};
-
-typedef Handler<ConfigUpdate> ConfigHandler;
-
-} // namespace config
-
diff --git a/config/src/vespa/config/common/iconfigholder.h b/config/src/vespa/config/common/iconfigholder.h
index 9b474ccf70b..5b7e8d6bbca 100644
--- a/config/src/vespa/config/common/iconfigholder.h
+++ b/config/src/vespa/config/common/iconfigholder.h
@@ -1,23 +1,23 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include <vespa/config/common/handler.h>
-#include <vespa/config/common/provider.h>
-#include <vespa/config/common/waitable.h>
-#include <vespa/config/common/pollable.h>
-#include <vespa/config/common/interruptable.h>
 #include <vespa/config/common/configupdate.h>
+#include <vespa/vespalib/util/time.h>
 
 namespace config {
 
-class IConfigHolder : public ConfigHandler,
-                      public Provider<ConfigUpdate>,
-                      public Waitable,
-                      public Pollable,
-                      public Interruptable
+class IConfigHolder
 {
 public:
     virtual ~IConfigHolder() = default;
+    virtual std::unique_ptr<ConfigUpdate> provide() = 0;
+    virtual void handle(std::unique_ptr<ConfigUpdate> obj) = 0;
+    virtual void close() = 0;
+    virtual bool poll() = 0;
+    bool wait_for(vespalib::duration timeout) {
+        return wait_until(vespalib::steady_clock::now() + timeout);
+    }
+    virtual bool wait_until(vespalib::steady_time deadline) = 0;
 };
 
 } // namespace config
diff --git a/config/src/vespa/config/common/interruptable.h b/config/src/vespa/config/common/interruptable.h
deleted file mode 100644
index 843ce8bc838..00000000000
--- a/config/src/vespa/config/common/interruptable.h
+++ /dev/null
@@ -1,19 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <memory>
-
-namespace config {
-
-/**
- * An Interruptable is a component that can be notified that it should abort its current activities.
- */
-struct Interruptable
-{
-    virtual void interrupt() = 0;
-    virtual ~Interruptable() {}
-};
-
-} // namespace config
-
diff --git a/config/src/vespa/config/common/pollable.h b/config/src/vespa/config/common/pollable.h
deleted file mode 100644
index 6799f06cef5..00000000000
--- a/config/src/vespa/config/common/pollable.h
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <memory>
-
-namespace config {
-
-/**
- * A Pollable is a component that can be polled, and returns either true or
- * false.
- */
-struct Pollable
-{
-    virtual bool poll() = 0;
-    virtual ~Pollable() {}
-};
-
-} // namespace config
-
diff --git a/config/src/vespa/config/common/provider.h b/config/src/vespa/config/common/provider.h
deleted file mode 100644
index 923d440558a..00000000000
--- a/config/src/vespa/config/common/provider.h
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <memory>
-
-namespace config {
-
-/**
- * A Provider is a component from which you can request an object.
- **/
-template <typename T>
-struct Provider
-{
-    virtual std::unique_ptr<T> provide() = 0;
-    virtual ~Provider() {}
-};
-
-} // namespace config
-
diff --git a/config/src/vespa/config/common/waitable.h b/config/src/vespa/config/common/waitable.h
deleted file mode 100644
index 7c51dd6bbf6..00000000000
--- a/config/src/vespa/config/common/waitable.h
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <vespa/vespalib/util/time.h>
-
-namespace config {
-
-/**
- * A Waitable is a component that can be used to wait for some event to happen.
- */
-struct Waitable
-{
-    bool wait_for(vespalib::duration timeout) {
-        return wait_until(vespalib::steady_clock::now() + timeout);
-    }
-    virtual bool wait_until(vespalib::steady_time deadline) = 0;
-    virtual ~Waitable() = default;
-};
-
-} // namespace config
-
diff --git a/config/src/vespa/config/subscription/configsubscription.cpp b/config/src/vespa/config/subscription/configsubscription.cpp
index f145c26d551..5eea0895eed 100644
--- a/config/src/vespa/config/subscription/configsubscription.cpp
+++ b/config/src/vespa/config/subscription/configsubscription.cpp
@@ -31,11 +31,11 @@ ConfigSubscription::~ConfigSubscription()
 bool
 ConfigSubscription::nextUpdate(int64_t generation, vespalib::steady_time deadline)
 {
-    if (_closed || !_holder->poll()) {
-        return false;
-    }
+    if (_closed || !_holder->wait_until(deadline)) { return false; }
+    if (_closed) { return false; }  // The above wait_until can be interrupted
     auto old = std::move(_next);
     _next = _holder->provide();
+    if ( ! _next) { return false; }
     if (old) {
         _next->merge(*old);
     }
@@ -66,9 +66,8 @@ ConfigSubscription::getGeneration() const
 void
 ConfigSubscription::close()
 {
-    if (!_closed) {
-        _closed = true;
-        _holder->interrupt();
+    if (!_closed.exchange(true)) {
+        _holder->close();
         _source->close();
     }
 }