Merge pull request #27112 from vespa-engine/balder/make-test-faster-and-deterministic

Ensure thread safety of handlA.

3 files changed, 21 insertions, 13 deletions

diff --git a/searchcore/src/tests/proton/common/timer/timer_test.cpp b/searchcore/src/tests/proton/common/timer/timer_test.cpp
index 4ff970df84e..56bad3981c9 100644
--- a/searchcore/src/tests/proton/common/timer/timer_test.cpp
+++ b/searchcore/src/tests/proton/common/timer/timer_test.cpp
@@ -5,9 +5,9 @@
 #include <vespa/searchcore/proton/common/scheduledexecutor.h>
 #include <vespa/vespalib/gtest/gtest.h>
 #include <vespa/vespalib/util/count_down_latch.h>
-#include <vespa/vespalib/util/size_literals.h>
 #include <vespa/vespalib/util/threadstackexecutor.h>
 #include <vespa/vespalib/util/lambdatask.h>
+#include <vespa/vespalib/testkit/time_bomb.h>
 #include <thread>
 
 using vespalib::Executor;
@@ -84,29 +84,36 @@ TYPED_TEST(ScheduledExecutorTest, test_drop_handle) {
 }
 
 TYPED_TEST(ScheduledExecutorTest, test_only_one_instance_running) {
+    vespalib::TimeBomb time_bomb(60s);
     vespalib::Gate latch;
     std::atomic<uint64_t> counter = 0;
     auto handleA = this->timer->scheduleAtFixedRate(makeLambdaTask([&]() { counter++; latch.await();}), 0ms, 1ms);
     std::this_thread::sleep_for(2s);
     EXPECT_EQ(1, counter);
     latch.countDown();
-    std::this_thread::sleep_for(2s);
+    while (counter <= 10) { std::this_thread::sleep_for(1ms); }
     EXPECT_GT(counter, 10);
 }
 
 TYPED_TEST(ScheduledExecutorTest, test_sync_delete) {
+    vespalib::TimeBomb time_bomb(60s);
     vespalib::Gate latch;
     std::atomic<uint64_t> counter = 0;
     std::atomic<uint64_t> reset_counter = 0;
+    std::mutex handleLock;
     auto handleA = this->timer->scheduleAtFixedRate(makeLambdaTask([&]() { counter++; latch.await();}), 0ms, 1ms);
-    auto handleB = this->timer->scheduleAtFixedRate(makeLambdaTask([&]() { handleA.reset(); reset_counter++; }), 0ms, 1ms);
-    std::this_thread::sleep_for(2s);
+    auto handleB = this->timer->scheduleAtFixedRate(makeLambdaTask([&]() {
+        std::lock_guard guard(handleLock);
+        handleA.reset();
+        reset_counter++;
+    }), 0ms, 1ms);
+    while (counter < 1) { std::this_thread::sleep_for(1ms); }
     EXPECT_EQ(1, counter);
     EXPECT_EQ(0, reset_counter);
     latch.countDown();
-    std::this_thread::sleep_for(2s);
+    while (reset_counter <= 10) { std::this_thread::sleep_for(1ms); }
     EXPECT_EQ(1, counter);
-    EXPECT_GT(reset_counter, 10);
+    std::lock_guard guard(handleLock);
     EXPECT_EQ(nullptr, handleA.get());
     EXPECT_FALSE(nullptr == handleB.get());
 }
diff --git a/vespalib/src/vespa/vespalib/testkit/time_bomb.cpp b/vespalib/src/vespa/vespalib/testkit/time_bomb.cpp
index 22cd535f434..9d4b2ea30c8 100644
--- a/vespalib/src/vespa/vespalib/testkit/time_bomb.cpp
+++ b/vespalib/src/vespa/vespalib/testkit/time_bomb.cpp
@@ -8,13 +8,13 @@ namespace vespalib {
 
 namespace {
 
-void bomb(Gate &gate, size_t seconds) {    
-    if (seconds > 5) {
-        if (gate.await(from_s(seconds - 5))) {
+void bomb(Gate &gate, vespalib::duration timeout) {
+    if (timeout > 5s) {
+        if (gate.await(timeout - 5s)) {
             return;
         }
     }
-    size_t countdown = std::min(seconds, size_t(5));
+    size_t countdown = std::min(count_s(timeout), 5l);
     while (countdown > 0) {
         fprintf(stderr, "...%zu...\n", countdown--);
         if (gate.await(1s)) {
@@ -27,9 +27,9 @@ void bomb(Gate &gate, size_t seconds) {
 
 } // namespace vespalib::<unnamed>
 
-TimeBomb::TimeBomb(size_t seconds)
+TimeBomb::TimeBomb(duration timeout)
     : _gate(),
-      _thread(bomb, std::ref(_gate), seconds)
+      _thread(bomb, std::ref(_gate), timeout)
 {
 }
 
diff --git a/vespalib/src/vespa/vespalib/testkit/time_bomb.h b/vespalib/src/vespa/vespalib/testkit/time_bomb.h
index a88aeeadd9a..ca02e7cda43 100644
--- a/vespalib/src/vespa/vespalib/testkit/time_bomb.h
+++ b/vespalib/src/vespa/vespalib/testkit/time_bomb.h
@@ -21,7 +21,8 @@ private:
     Gate _gate;
     std::thread _thread;
 public:
-    TimeBomb(size_t seconds);
+    TimeBomb(size_t seconds) : TimeBomb(from_s(seconds)) {}
+    TimeBomb(vespalib::duration duration);
     ~TimeBomb(); // defuse the bomb
 };