Autogenerate a tag for cleanup when unregistering.

3 files changed, 43 insertions, 20 deletions

diff --git a/vespalib/src/tests/invokeservice/invokeservice_test.cpp b/vespalib/src/tests/invokeservice/invokeservice_test.cpp
index 88a7969e153..f0f8a21204d 100644
--- a/vespalib/src/tests/invokeservice/invokeservice_test.cpp
+++ b/vespalib/src/tests/invokeservice/invokeservice_test.cpp
@@ -7,6 +7,11 @@ using namespace vespalib;
 struct InvokeCounter {
     InvokeCounter() : _count(0) {}
     void inc() noexcept { _count++; }
+    void wait_for_atleast(uint64_t n) {
+        while (_count <= n) {
+            std::this_thread::sleep_for(1ms);
+        }
+    }
     std::atomic<uint64_t> _count;
 };
 
@@ -27,20 +32,35 @@ TEST("require that wakeup is called") {
 
 TEST("require that same wakeup can be registered multiple times.") {
     InvokeCounter a;
+    InvokeCounter b;
+    InvokeCounter c;
     InvokeServiceImpl service(1ms);
     EXPECT_EQUAL(0u, a._count);
     auto ra1 = service.registerInvoke([&a]() noexcept { a.inc(); });
     EXPECT_TRUE(ra1);
+    auto rb = service.registerInvoke([&b]() noexcept { b.inc(); });
+    EXPECT_TRUE(rb);
+    auto rc = service.registerInvoke([&c]() noexcept { c.inc(); });
+    EXPECT_TRUE(rc);
+    a.wait_for_atleast(1);
+    b.wait_for_atleast(1);
+    c.wait_for_atleast(1);
     auto ra2 = service.registerInvoke([&a]() noexcept { a.inc(); });
-    while (a._count == 0) {
-        std::this_thread::sleep_for(1ms);
-    }
-    ra1.reset();
-    uint64_t countAtStop = a._count;
-    ra2 = service.registerInvoke([&a]() noexcept { a.inc(); });
     EXPECT_TRUE(ra2);
+    a.wait_for_atleast(1);
+    b.wait_for_atleast(1);
+    c.wait_for_atleast(1);
+
+    rb.reset();
+    uint64_t countAtStop = b._count;
+    uint64_t a_count = a._count;
+    uint64_t c_count = c._count;
     std::this_thread::sleep_for(1s);
-    EXPECT_LESS(countAtStop, a._count);
+    EXPECT_EQUAL(countAtStop, b._count);
+
+    uint64_t diff_c = c._count - c_count;
+    uint64_t diff_a = a._count - a_count;
+    EXPECT_LESS((diff_c*3)/2, diff_a); // diff_c*3/2 should still be smaller than diff_a(2x)
 }
 
 
diff --git a/vespalib/src/vespa/vespalib/util/invokeserviceimpl.cpp b/vespalib/src/vespa/vespalib/util/invokeserviceimpl.cpp
index 1980e642b17..ad8d9973356 100644
--- a/vespalib/src/vespa/vespalib/util/invokeserviceimpl.cpp
+++ b/vespalib/src/vespa/vespalib/util/invokeserviceimpl.cpp
@@ -8,6 +8,7 @@ namespace vespalib {
 InvokeServiceImpl::InvokeServiceImpl(duration napTime)
     : _naptime(napTime),
       _lock(),
+      _currId(0),
       _closed(false),
       _toWakeup(),
       _thread()
@@ -28,35 +29,36 @@ InvokeServiceImpl::~InvokeServiceImpl()
 
 class InvokeServiceImpl::Registration : public IDestructorCallback {
 public:
-    Registration(InvokeServiceImpl * service, VoidFunc func) noexcept
+    Registration(InvokeServiceImpl * service, uint64_t id) noexcept
         : _service(service),
-          _func(func)
+          _id(id)
     { }
     Registration(const Registration &) = delete;
     Registration & operator=(const Registration &) = delete;
     ~Registration() override{
-        _service->unregister(_func);
+        _service->unregister(_id);
     }
 private:
     InvokeServiceImpl * _service;
-    VoidFunc        _func;
+    uint64_t        _id;
 };
 
 std::unique_ptr<IDestructorCallback>
 InvokeServiceImpl::registerInvoke(VoidFunc func) {
     std::lock_guard guard(_lock);
-    _toWakeup.push_back(func);
+    uint64_t id = _currId++;
+    _toWakeup.emplace_back(id, func);
     if ( ! _thread) {
         _thread = std::make_unique<std::thread>([this]() { runLoop(); });
     }
-    return std::make_unique<Registration>(this, func);
+    return std::make_unique<Registration>(this, id);
 }
 
 void
-InvokeServiceImpl::unregister(VoidFunc func) {
+InvokeServiceImpl::unregister(uint64_t id) {
     std::lock_guard guard(_lock);
-    auto found = std::find_if(_toWakeup.begin(), _toWakeup.end(), [&func](const VoidFunc & a) {
-        return func.target<VoidFunc>() == a.target<VoidFunc>();
+    auto found = std::find_if(_toWakeup.begin(), _toWakeup.end(), [id](const std::pair<uint64_t, VoidFunc> & a) {
+        return id == a.first;
     });
     assert (found != _toWakeup.end());
     _toWakeup.erase(found);
@@ -68,8 +70,8 @@ InvokeServiceImpl::runLoop() {
     while ( ! done ) {
         {
             std::lock_guard guard(_lock);
-            for (VoidFunc & func: _toWakeup) {
-                func();
+            for (auto & func: _toWakeup) {
+                func.second();
             }
             done = _closed;
         }
diff --git a/vespalib/src/vespa/vespalib/util/invokeserviceimpl.h b/vespalib/src/vespa/vespalib/util/invokeserviceimpl.h
index f92cb691780..36347e47d3e 100644
--- a/vespalib/src/vespa/vespalib/util/invokeserviceimpl.h
+++ b/vespalib/src/vespa/vespalib/util/invokeserviceimpl.h
@@ -23,12 +23,13 @@ public:
     std::unique_ptr<IDestructorCallback> registerInvoke(VoidFunc func) override;
 private:
     class Registration;
-    void unregister(VoidFunc func);
+    void unregister(uint64_t id);
     void runLoop();
     duration                       _naptime;
     std::mutex                     _lock;
+    uint64_t                       _currId;
     bool                           _closed;
-    std::vector<VoidFunc>          _toWakeup;
+    std::vector<std::pair<uint64_t, VoidFunc>> _toWakeup;
     std::unique_ptr<std::thread>   _thread;
 };