Fix race condition in CpuInfoProvider::QueryCpuTimePerProcessor

chrome/browser/extensions/api/system_cpu/cpu_info_provider_linux.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/system_cpu/cpu_info_provider.h"
 
 #include <cstdio>
 #include <sstream>
 
 #include "base/file_util.h"
@@ skipping 19 matching lines @@
   std::string line;
 
   // Skip the first line because it is just an aggregated number of
   // all cpuN lines.
   std::getline(iss, line);
   size_t i = 0;
   while (std::getline(iss, line)) {
     if (line.compare(0, 3, "cpu") != 0)
       continue;
 
+    // Cope with kernel race condition when /proc/stat does not reflect to the

[Pawel Osciak, 2014/08/04 00:55:21]

s/to//

[Justin Chuang, 2014/08/04 08:12:03]

Acknowledged.

+    // current number of online processors in time. Also handle a less-likely

[Pawel Osciak, 2014/08/04 00:55:21]

s/in time//

[Justin Chuang, 2014/08/04 08:12:03]

Acknowledged.

+    // race condition when the number of online processors is changed after it
+    // has been decided in CpuInfoProvider::QueryInfo().
+    if (i == infos->size())

[Pawel Osciak, 2014/08/04 00:55:22]

Please explain how this actually handles the issue? It looks like this just
prevents us from crashing... Or will the caller understand that this CPU went
offline? Removing the DCHECK() below makes me think it doesn't expect it...

[Justin Chuang, 2014/08/04 08:12:04]

Thanks for the comments. It's actually a race condition in Linux kernel. The
caller does know the current online CPU count, it's just that output of
/proc/stat doesn't match it.

A different method to solve this is to add a new API to return
sysconf(_SC_NPROCESSORS_CONF) in addition to _SC_NPROCESSORS_ONLN, then add this
new API all Win32, Mac, Linux, BSD(?) platforms. And we also need change the
original behavior of system_cpu extension API. Probably a overkill or can be
done in later CL. 

For now, I think the current fix is sufficient for the R37/R38 bug. I'll tweak
the comments and logging. 

+      return false;
+
     uint64 user = 0, nice = 0, sys = 0, idle = 0;
     int vals = sscanf(line.c_str(), "%*s %"PRIu64" %"PRIu64" %"PRIu64" %"PRIu64,
            &user, &nice, &sys, &idle);
     DCHECK_EQ(4, vals);
 
-    DCHECK(i < infos->size());
     infos->at(i)->usage.kernel = static_cast<double>(sys);
     infos->at(i)->usage.user = static_cast<double>(user + nice);
     infos->at(i)->usage.idle = static_cast<double>(idle);
     infos->at(i)->usage.total = static_cast<double>(sys + user + nice + idle);
     ++i;
   }
   DCHECK_EQ(infos->size(), i);
   return true;
 }
 
 }  // namespace extensions