[Groonga-commit] groonga/grnxx at e603869 [master] Fix a bug of grnxx::PeriodicClock.

Back to archive index

susumu.yata null+****@clear*****
Thu Jul 11 18:12:21 JST 2013


susumu.yata	2013-07-11 18:12:21 +0900 (Thu, 11 Jul 2013)

  New Revision: e603869b4b5020bb617e1f349ff84daef42afec3
  https://github.com/groonga/grnxx/commit/e603869b4b5020bb617e1f349ff84daef42afec3

  Message:
    Fix a bug of grnxx::PeriodicClock.

  Modified files:
    lib/grnxx/periodic_clock.cpp
    lib/grnxx/periodic_clock.hpp
    test/test_time.cpp

  Modified: lib/grnxx/periodic_clock.cpp (+22 -15)
===================================================================
--- lib/grnxx/periodic_clock.cpp    2013-07-11 16:37:31 +0900 (96a5615)
+++ lib/grnxx/periodic_clock.cpp    2013-07-11 18:12:21 +0900 (2f10d49)
@@ -17,10 +17,13 @@
 */
 #include "grnxx/periodic_clock.hpp"
 
-#include "grnxx/thread.hpp"
+#include <memory>
+
+#include "grnxx/intrinsic.hpp"
 #include "grnxx/lock.hpp"
 #include "grnxx/logger.hpp"
 #include "grnxx/mutex.hpp"
+#include "grnxx/thread.hpp"
 
 namespace grnxx {
 namespace {
@@ -29,8 +32,10 @@ namespace {
 // busy-wait loop, which exhausts CPU resources.
 constexpr Duration UPDATE_INTERVAL = Duration::milliseconds(100);
 
+// The number of PeriodicClock objects.
 volatile uint32_t ref_count = 0;
-grnxx::Thread *thread = nullptr;
+// The current thread ID.
+volatile uint32_t thread_id = 0;
 Mutex mutex;
 
 }  // namespace
@@ -38,10 +43,12 @@ Mutex mutex;
 Time PeriodicClock::now_ = Time::min();
 
 PeriodicClock::PeriodicClock() {
-  // Start the internal thread iff this is the first object.
   Lock lock(&mutex);
   if (++ref_count == 1) try {
-    thread = grnxx::Thread::create(routine);
+    // Start an internal thread that updates "now_" periodically.
+    std::unique_ptr<grnxx::Thread> thread(grnxx::Thread::create(routine));
+    thread->detach();
+    // Immediately update "now_".
     now_ = SystemClock::now();
   } catch (...) {
     GRNXX_WARNING() << "failed to create thread for PeriodicClock";
@@ -49,25 +56,25 @@ PeriodicClock::PeriodicClock() {
 }
 
 PeriodicClock::~PeriodicClock() {
-  // Stop the running thread iff this is the last object.
   Lock lock(&mutex);
   if (--ref_count == 0) {
-    try {
-      thread->detach();
-    } catch (...) {
-      GRNXX_WARNING() << "failed to detach thread for PeriodicClock";
-    }
-    delete thread;
-    thread = nullptr;
     now_ = Time::min();
+    // Increment "thread_id" so that an internal thread will stop.
+    atomic_fetch_and_add(1, &thread_id);
   }
 }
 
-// Periodically update the internal time variable.
 void PeriodicClock::routine() {
-  while (ref_count != 0) {
+  // Increment "thread_id" to generate the ID of this thread.
+  const uint64_t this_thread_id = atomic_fetch_and_add(1, &thread_id) + 1;
+  // This thread terminates if there are no PeriodicClock objects or another
+  // thread is running.
+  while ((ref_count != 0) && (this_thread_id == thread_id)) {
     Thread::sleep_for(UPDATE_INTERVAL);
-    now_ = SystemClock::now();
+    Lock lock(&mutex);
+    if ((ref_count != 0) && (this_thread_id == thread_id)) {
+      now_ = SystemClock::now();
+    }
   }
 }
 

  Modified: lib/grnxx/periodic_clock.hpp (+1 -0)
===================================================================
--- lib/grnxx/periodic_clock.hpp    2013-07-11 16:37:31 +0900 (68f9a95)
+++ lib/grnxx/periodic_clock.hpp    2013-07-11 18:12:21 +0900 (074fdf1)
@@ -42,6 +42,7 @@ class PeriodicClock {
  private:
   static Time now_;
 
+  // Update "now_" periodically.
   static void routine();
 };
 

  Modified: test/test_time.cpp (+12 -14)
===================================================================
--- test/test_time.cpp    2013-07-11 16:37:31 +0900 (555d04f)
+++ test/test_time.cpp    2013-07-11 18:12:21 +0900 (c6e9b45)
@@ -26,6 +26,9 @@
 #include "grnxx/time.hpp"
 
 void test_time() {
+  GRNXX_NOTICE() << "grnxx::SystemClock::now() = "
+                 << grnxx::SystemClock::now();
+
   assert(grnxx::Time::max().count() ==
          std::numeric_limits<std::int64_t>::max());
   assert(grnxx::Time::min().count() ==
@@ -33,9 +36,9 @@ void test_time() {
 }
 
 void test_broken_down_time() {
-  GRNXX_NOTICE() << "grnxx::SystemClock::now().universal_time(): "
+  GRNXX_NOTICE() << "grnxx::SystemClock::now().universal_time() = "
                  << grnxx::SystemClock::now().universal_time();
-  GRNXX_NOTICE() << "grnxx::SystemClock::now().local_time(): "
+  GRNXX_NOTICE() << "grnxx::SystemClock::now().local_time() = "
                  << grnxx::SystemClock::now().local_time();
 
   enum { LOOP_COUNT = 1 << 16 };
@@ -61,8 +64,7 @@ void test_broken_down_time() {
 
 void test_system_clock() {
   grnxx::Time time = grnxx::SystemClock::now();
-  GRNXX_NOTICE() << "grnxx::SystemClock::now(): " << time;
-  GRNXX_NOTICE() << "grnxx::SystemClock::now().local_time(): "
+  GRNXX_NOTICE() << "grnxx::SystemClock::now().local_time() = "
                  << time.local_time();
 
   enum { LOOP_COUNT = 1 << 16 };
@@ -80,27 +82,23 @@ void test_periodic_clock() {
   grnxx::PeriodicClock clock;
 
   grnxx::Time time = grnxx::PeriodicClock::now();
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now(): " << time;
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time(): "
+  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time() = "
                  << time.local_time();
 
   time = grnxx::PeriodicClock::now();
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now(): " << time;
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time(): "
+  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time() = "
                  << time.local_time();
 
-  grnxx::Thread::sleep_for(grnxx::Duration::milliseconds(310));
+  grnxx::Thread::sleep_for(grnxx::Duration::milliseconds(110));
 
   time = grnxx::PeriodicClock::now();
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now(): " << time;
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time(): "
+  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time() = "
                  << time.local_time();
 
-  grnxx::Thread::sleep_for(grnxx::Duration::milliseconds(310));
+  grnxx::Thread::sleep_for(grnxx::Duration::milliseconds(110));
 
   time = grnxx::PeriodicClock::now();
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now(): " << time;
-  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time(): "
+  GRNXX_NOTICE() << "grnxx::PeriodicClock::now().local_time() = "
                  << time.local_time();
 
   enum { LOOP_COUNT = 1 << 20 };
-------------- next part --------------
HTML����������������������������...
Télécharger 



More information about the Groonga-commit mailing list
Back to archive index