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

components/arc/metrics/oom_kills_monitor.h

 // Copyright 2016 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.
 
 #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 null handle.
+    OomKillsMonitorHandle() = default;

[Luis Héctor Chávez, 2016/07/29 21:48:45]

Can this be removed?

[gab, 2016/08/01 15:15:02]

Now that it's created in initializer list, yes :-)

 
-  void Start();
-  void Stop();
+    // Constructs a handle that will flag |outer| as shutting down on
+    // destruction.
+    OomKillsMonitorHandle(OomKillsMonitor* outer);

[Luis Héctor Chávez, 2016/07/29 21:48:45]

explicit

[gab, 2016/08/01 15:15:02]

Done.

+
+    ~OomKillsMonitorHandle();
+
+   private:
+    OomKillsMonitor* outer_ = nullptr;

[Luis Héctor Chávez, 2016/07/29 21:48:45]

Can this be OomKilssMonitor* const outer_;?

[gab, 2016/08/01 15:15:02]

Done.

+  }

[Luis Héctor Chávez, 2016/07/29 21:48:45]

Can this be DISALLOW_COPY_AND_ASSIGN?

[gab, 2016/08/01 15:15:02]

Now that it's in initializer list, yes :-) -- and it makes sense because we
wouldn't want destructor invoked on copy!

+
+  // 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 void OomKillsMonitorHandle StartMonitoring();

[Luis Héctor Chávez, 2016/07/29 21:48:45]

static OomKillsMonitorHandle StartMonitoring(); ?

[gab, 2016/08/01 15:15:02]

Oops yes, not sure how that compiled..?!

 
  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();
 
-  scoped_refptr<base::SequencedWorkerPool> worker_pool_;
+  void Run() override;
+
+  // 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);
 };
 
 }  // namespace arc
 
 #endif  // COMPONENTS_ARC_METRICS_OOM_KILLS_MONITOR_H_