commit 0d9a9c961ec44d054d4922a0355938265c73ae95
author Luigi Semenzato <semenzato@chromium.org> Thu Dec 05 15:55:12 2013 -0800
committer chrome-internal-fetch <chrome-internal-fetch@google.com> Fri Dec 06 22:44:22 2013 +0000
tree 8a1506af3a282b8e38057485f60024b03205d7e7
parent d399bed72c51c063525f7656e6506efe7743b2cf

Fix out-of-bound index and clean up code.

There was an out-of-bound index after something like 7.5 hours of
active use.  This probably caused the metrics daemon to restart
and pollute some of the memuse statistics, the very ones
responsible for the overflow.

BUG=chromium:326824
TEST=ran manually and verified memuse is still reported

Change-Id: I70709f8cbba63ea1ca5e277b2ab58729e072e127
Reviewed-on: https://chromium-review.googlesource.com/178976
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>

metrics/metrics_daemon.cc

diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc
index 1655fae..f3849ee 100644
--- a/metrics/metrics_daemon.cc
+++ b/metrics/metrics_daemon.cc
@@ -180,7 +180,7 @@
       user_active_(false),
       usemon_interval_(0),
       usemon_source_(NULL),
-      memuse_initial_time_(0),
+      memuse_final_time_(0),
       memuse_interval_index_(0),
       read_sectors_(0),
       write_sectors_(0),
@@ -305,7 +305,8 @@
 
   // Start collecting meminfo stats.
   ScheduleMeminfoCallback(kMetricMeminfoInterval);
-  ScheduleMemuseCallback(true, 0);
+  memuse_final_time_ = GetActiveTime() + kMemuseIntervals[0];
+  ScheduleMemuseCallback(kMemuseIntervals[0]);
 
   // Don't setup D-Bus and GLib in test mode.
   if (testing)
@@ -1012,20 +1013,11 @@
   return true;
 }
 
-void MetricsDaemon::ScheduleMemuseCallback(bool new_callback,
-                                           double time_elapsed) {
+void MetricsDaemon::ScheduleMemuseCallback(double interval) {
   if (testing_) {
     return;
   }
-  int interval = kMemuseIntervals[memuse_interval_index_];
-  int wait;
-  if (new_callback) {
-    memuse_initial_time_ = GetActiveTime();
-    wait = interval;
-  } else {
-    wait = ceil(interval - time_elapsed);  // round up
-  }
-  g_timeout_add_seconds(wait, MemuseCallbackStatic, this);
+  g_timeout_add_seconds(interval, MemuseCallbackStatic, this);
 }
 
 // static
@@ -1040,17 +1032,18 @@
   // the callbacks are driven by real time (uptime), we check if we should
   // reschedule this callback due to intervening sleep periods.
   double now = GetActiveTime();
-  double active_time = now - memuse_initial_time_;
-  if (active_time < kMemuseIntervals[memuse_interval_index_]) {
-    // Not enough active time has passed.  Reschedule the callback.
-    ScheduleMemuseCallback(false, active_time);
+  // Avoid intervals of less than one second.
+  double remaining_time = ceil(memuse_final_time_ - now);
+  if (remaining_time > 0) {
+    ScheduleMemuseCallback(remaining_time);
   } else {
-    // Enough active time has passed.  Do the work, and (if we succeed) see if
-    // we need to do more.
+    // Report stats and advance the measurement interval unless there are
+    // errors or we've completed the last interval.
     if (MemuseCallbackWork() &&
-        memuse_interval_index_ < arraysize(kMemuseIntervals) - 1) {
-      memuse_interval_index_++;
-      ScheduleMemuseCallback(true, 0);
+        memuse_interval_index_ < arraysize(kMemuseIntervals)) {
+      double interval = kMemuseIntervals[memuse_interval_index_++];
+      memuse_final_time_ = now + interval;
+      ScheduleMemuseCallback(interval);
     }
   }
 }

metrics/metrics_daemon.h

diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h
index d3a4f72..82372f8 100644
--- a/metrics/metrics_daemon.h
+++ b/metrics/metrics_daemon.h
@@ -329,12 +329,8 @@
   bool FillMeminfo(const std::string& meminfo_raw,
                    std::vector<MeminfoRecord>* fields);
 
-  // Schedule a memory use callback.  |new_callback| is true when this callback
-  // is scheduled for the first time.  When |new_callback| is false,
-  // |time_elapsed| is the active (non-sleep) time that has passed between now
-  // and the original callback scheduling time.  We use it to reschedule a
-  // callback that fired too early because we slept.
-  void ScheduleMemuseCallback(bool new_callback, double time_elapsed);
+  // Schedule a memory use callback in |interval| seconds.
+  void ScheduleMemuseCallback(double interval);
 
   // Static wrapper for MemuseCallback.  Always returns false.
   static gboolean MemuseCallbackStatic(void* handle);
@@ -405,8 +401,8 @@
   // Scheduled daily use monitor source (see ScheduleUseMonitor).
   GSource* usemon_source_;
 
-  // Time of initial scheduling of memuse callback
-  double memuse_initial_time_;
+  // End time of current memuse stat collection interval.
+  double memuse_final_time_;
 
   // Selects the wait time for the next memory use callback.
   unsigned int memuse_interval_index_;