Disable mojo power notifications on shutdown

PowerMonitorBroadcastSource receives mojo power notifications on IO
thread, and can use PowerMonitor after it's destroyed on main thread.
Disabling power notifications on the IO thread prevents this data race.

Bug: 834312
Change-Id: I1677b6098d158ecc18dfce60eaeed9590110a962
Reviewed-on: https://chromium-review.googlesource.com/1041215
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556603}

services/device/public/cpp/power_monitor/power_monitor_broadcast_source.cc

@@ -25,6 +25,12 @@ PowerMonitorBroadcastSource::PowerMonitorBroadcastSource(
     : client_(std::move(client)), task_runner_(task_runner) {}
 
 PowerMonitorBroadcastSource::~PowerMonitorBroadcastSource() {
+  // When power monitor and source are destroyed, the IO thread could still be
+  // receiving mojo messages that access the monitor and source in
+  // |ProcessPowerEvent|, thus causing a data race. Calling Shutdown() on the
+  // client tells it to ignore future mojo messages, and thus prevents the data
+  // race.
+  client_->Shutdown();
   task_runner_->DeleteSoon(FROM_HERE, client_.release());
 }
 
@@ -47,6 +53,9 @@ PowerMonitorBroadcastSource::Client::~Client() {}
 
 void PowerMonitorBroadcastSource::Client::Init(
     std::unique_ptr<service_manager::Connector> connector) {
+  base::AutoLock auto_lock(is_shutdown_lock_);
+  if (is_shutdown_)
+    return;
   connector_ = std::move(connector);
   device::mojom::PowerMonitorPtr power_monitor;
   connector_->BindInterface(device::mojom::kServiceName,
@@ -56,17 +65,32 @@ void PowerMonitorBroadcastSource::Client::Init(
   power_monitor->AddClient(std::move(client));
 }
 
+void PowerMonitorBroadcastSource::Client::Shutdown() {
+  base::AutoLock auto_lock(is_shutdown_lock_);
+  DCHECK(!is_shutdown_);
+  is_shutdown_ = true;
+}
+
 void PowerMonitorBroadcastSource::Client::PowerStateChange(
     bool on_battery_power) {
+  base::AutoLock auto_lock(is_shutdown_lock_);
+  if (is_shutdown_)
+    return;
   last_reported_on_battery_power_state_ = on_battery_power;
   ProcessPowerEvent(PowerMonitorSource::POWER_STATE_EVENT);
 }
 
 void PowerMonitorBroadcastSource::Client::Suspend() {
+  base::AutoLock auto_lock(is_shutdown_lock_);
+  if (is_shutdown_)
+    return;
   ProcessPowerEvent(PowerMonitorSource::SUSPEND_EVENT);
 }
 
 void PowerMonitorBroadcastSource::Client::Resume() {
+  base::AutoLock auto_lock(is_shutdown_lock_);
+  if (is_shutdown_)
+    return;
   ProcessPowerEvent(PowerMonitorSource::RESUME_EVENT);
 }
 

services/device/public/cpp/power_monitor/power_monitor_broadcast_source.h

@@ -45,11 +45,18 @@ class PowerMonitorBroadcastSource : public base::PowerMonitorSource {
   FRIEND_TEST_ALL_PREFIXES(PowerMonitorMessageBroadcasterTest,
                            PowerMessageBroadcast);
 
+  // Client holds the mojo connection. It is created on the main thread, and
+  // destroyed on task runner's thread. Unless otherwise noted, all its methods
+  // all called on the task runner's thread.
   class Client : public device::mojom::PowerMonitorClient {
    public:
     Client();
     ~Client() override;
 
+    // Called on main thread when the source is destroyed. Prevents data race
+    // on the power monitor and source due to use on task runner thread.
+    void Shutdown();
+
     void Init(std::unique_ptr<service_manager::Connector> connector);
 
     bool last_reported_on_battery_power_state() const {
@@ -65,6 +72,9 @@ class PowerMonitorBroadcastSource : public base::PowerMonitorSource {
     std::unique_ptr<service_manager::Connector> connector_;
     mojo::Binding<device::mojom::PowerMonitorClient> binding_;
 
+    base::Lock is_shutdown_lock_;
+    bool is_shutdown_ = false;
+
     bool last_reported_on_battery_power_state_ = false;
 
     DISALLOW_COPY_AND_ASSIGN(Client);