Move OomKillsMonitor from a single-threaded SequencedWorkerPool to a non-joinable thread.

components/arc/metrics/oom_kills_monitor.h

Index: components/arc/metrics/oom_kills_monitor.h
diff --git a/components/arc/metrics/oom_kills_monitor.h b/components/arc/metrics/oom_kills_monitor.h
index c6cf7995ae835043f084b1eb1d3aa3e34734be0a..25ad43e774f3a8cc76ab66065608ffcab995cce7 100644
--- a/components/arc/metrics/oom_kills_monitor.h
+++ b/components/arc/metrics/oom_kills_monitor.h
@@ -5,33 +5,54 @@
 #ifndef COMPONENTS_ARC_METRICS_OOM_KILLS_MONITOR_H_
 #define COMPONENTS_ARC_METRICS_OOM_KILLS_MONITOR_H_
 
+#include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "base/threading/sequenced_worker_pool.h"
+#include "base/threading/thread.h"
 
 namespace arc {
 
 // Traces kernel OOM kill events and lowmemorykiller (if enabled) events.
 //
 // OomKillsMonitor listens to kernel messages for both OOM kills and
-// lowmemorykiller kills, then reports to UMA.
+// lowmemorykiller kills, then reports to UMA. It uses a non-joinable thread
+// in order to avoid blocking shutdown.
 //
 // Note: There should be only one OomKillsMonitor instance globally at any given
 // time, otherwise UMA would receive duplicate events.
-class OomKillsMonitor {
+class OomKillsMonitor : public base::Thread {
  public:
-  OomKillsMonitor();
-  ~OomKillsMonitor();
+  // A handle representing the OomKillsMonitor's lifetime (the monitor itself
+  // can't be destroyed per being a non-joinable Thread).
+  class OomKillsMonitorHandle {
+   public:
+    // Constructs a handle that will flag |outer| as shutting down on
+    // destruction.
+    explicit OomKillsMonitorHandle(OomKillsMonitor* outer);
+
+    ~OomKillsMonitorHandle();
+
+   private:
+    OomKillsMonitor* const outer_;

[Luis Héctor Chávez, 2016/08/01 15:48:14]

Now that I think of it, doesn't this also need a friend declaration since
|is_shutting_down_| is private? If the compiler doesn't like this code as-is,
rather than adding a friend declaration, can you just pass in the reference to
|is_shutting_down_|?

[gab, 2016/08/04 22:42:36]

Inner classes have access to their Outer class' private state (as they do in
Java, the main difference with Java is that in C++ you need a pointer to your
Outer instead of being able to use the fields directly -- because in C++ this
wouldn't make sense per lack of automatic refcounting).

+  }
 
-  void Start();
-  void Stop();
+  // The instance has to be leaked on shutdown per being a non-joinable Thread.
+  ~OomKillsMonitor() = delete;
+
+  // Instantiates the OomKillsMonitor instance and starts it. This can only
+  // be invoked once per process.
+  static OomKillsMonitorHandle StartMonitoring();
 
  private:
-  // Keeps a reference to worker_pool_ in case |this| is deleted in
-  // shutdown process while this thread returns from a blocking read.
-  static void Run(scoped_refptr<base::SequencedWorkerPool> worker_pool);
+  friend struct base::internal::LeakyLazyInstanceTraits;
+  OomKillsMonitor();
+
+  void Run() override;
 
-  scoped_refptr<base::SequencedWorkerPool> worker_pool_;
+  // A flag set when OomKillsMonitor is shutdown so that its thread can poll
+  // it and attempt to wind down from that point (to avoid unecessary work, not
+  // because it blocks shutdown).
+  base::AtomicFlag is_shutting_down_;
 
   DISALLOW_COPY_AND_ASSIGN(OomKillsMonitor);
 };