repeated_timer: finish condition variable should release mutex when waiting for still running task finished.

src/braft/configuration_manager.cpp

@@ -63,7 +63,11 @@ void ConfigurationManager::set_snapshot(ConfigurationEntry&& entry) {
 
 void ConfigurationManager::get(int64_t last_included_index,
                                ConfigurationEntry* conf) {
-    if (_configurations.empty()) {
+    if (_configurations.empty() ||
+        _configurations.begin()->id.index > last_included_index) {
+        // configurations is empty or
+        // the smallest index configuration entry is greater than
+        // last_included_index.
         CHECK_GE(last_included_index, _snapshot.id.index);
         *conf = _snapshot;
         return;
@@ -80,8 +84,9 @@ void ConfigurationManager::get(int64_t last_included_index,
             return rhs.id.index > index;
         });
 
-    CHECK(it != _configurations.rend());
+    CHECK(it != _configurations.rend());  // must found.
     *conf = *it;
+    return;
 }
 
 const ConfigurationEntry& ConfigurationManager::last_configuration() const {

src/braft/configuration_manager.h

@@ -73,6 +73,8 @@ class ConfigurationManager {
 
     void set_snapshot(ConfigurationEntry&& snapshot);
 
+    // find the latest ConfigurationEntry which `last_included_index`
+    // log belongs to.
     void get(int64_t last_included_index, ConfigurationEntry* entry);
 
     const ConfigurationEntry& last_configuration() const;

src/braft/repeated_timer_task.cpp

@@ -27,7 +27,7 @@ RepeatedTimerTask::RepeatedTimerTask()
       _running(false),
       _destroyed(true),
       _invoking(false),
-      _finish_event(0) {}
+      _finish_cv(&_mutex) {}
 
 RepeatedTimerTask::~RepeatedTimerTask() {
     CHECK(!_running) << "Is still running";
@@ -72,7 +72,7 @@ void RepeatedTimerTask::on_timedout() {
             lck.unlock();
             on_destroy();
         }
-        _finish_event.signal();
+        _finish_cv.Signal();
         return;
     }
     return schedule(lck);
@@ -96,7 +96,6 @@ void RepeatedTimerTask::start() {
     // is still running, in which case on_timedout would invoke
     // schedule as it would not see _stopped
     _running = true;
-    _finish_event.reset(1);
     schedule(lck);
 }
 
@@ -179,8 +178,10 @@ void RepeatedTimerTask::destroy(bool wait_infight_task) {
         return;
     }
 
-    if (wait_infight_task) {
-        _finish_event.wait();
+    // `rc` == 1 means timer still running.
+    // if `wait_infight_task` is true, we should wait until task is finished.
+    if (rc == 1 && wait_infight_task) {
+        _finish_cv.Wait();
         CHECK(!_running);
         return;
     }

src/braft/repeated_timer_task.h

@@ -15,13 +15,13 @@
 // Authors: Zhangyi Chen([email protected])
 //          Ma,Jingwei([email protected])
 
-#ifndef BRAFT_REPEATED_TIMER_TASK_H
-#define BRAFT_REPEATED_TIMER_TASK_H
+#pragma once
 
 #include <bthread/unstable.h>
 
 #include "braft/macros.h"
 #include "bthread/countdown_event.h"
+#include "butil/synchronization/condition_variable.h"
 
 namespace braft {
 
@@ -79,9 +79,7 @@ class RepeatedTimerTask {
     bool _running;
     bool _destroyed;
     bool _invoking;
-    bthread::CountdownEvent _finish_event;
+    butil::ConditionVariable _finish_cv;
 };
 
 }  //  namespace braft
-
-#endif  // BRAFT_REPEATED_TIMER_TASK_H