[sensors](Linux) Fix tsan data race in sensor reader

device/generic_sensor/platform_sensor_reader_linux.cc

 // 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.
 
 #include "device/generic_sensor/platform_sensor_reader_linux.h"
 
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/timer/timer.h"
 #include "device/generic_sensor/linux/sensor_data_linux.h"
 #include "device/generic_sensor/platform_sensor_linux.h"
 #include "device/generic_sensor/public/cpp/sensor_reading.h"
 
 namespace device {
 
 class PollingSensorReader : public SensorReader {
  public:
-  PollingSensorReader(
-      const SensorInfoLinux* sensor_device,
-      PlatformSensorLinux* sensor,
-      scoped_refptr<base::SingleThreadTaskRunner> polling_task_runner);
+  PollingSensorReader(const SensorInfoLinux* sensor_device,
+                      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~PollingSensorReader() override;
 
   // SensorReader implements:
-  bool StartFetchingData(
+  void StartFetchingData(
       const PlatformSensorConfiguration& configuration) override;
   void StopFetchingData() override;
 
  private:
   // Initializes a read timer.
   void InitializeTimer(const PlatformSensorConfiguration& configuration);
 
   // Polls data and sends it to a |sensor_|.
   void PollForData();
 
@@ skipping 13 matching lines @@
   // destructor is called.
   base::RepeatingTimer* timer_;
 
   base::WeakPtrFactory<PollingSensorReader> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PollingSensorReader);
 };
 
 PollingSensorReader::PollingSensorReader(
     const SensorInfoLinux* sensor_device,
-    PlatformSensorLinux* sensor,
-    scoped_refptr<base::SingleThreadTaskRunner> polling_task_runner)
-    : SensorReader(sensor, polling_task_runner),
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+    : SensorReader(std::move(task_runner)),
       sensor_file_paths_(sensor_device->device_reading_files),
       scaling_value_(sensor_device->device_scaling_value),
       offset_value_(sensor_device->device_offset_value),
       apply_scaling_func_(sensor_device->apply_scaling_func),
       timer_(new base::RepeatingTimer()),
       weak_factory_(this) {}
 
 PollingSensorReader::~PollingSensorReader() {
   polling_task_runner_->DeleteSoon(FROM_HERE, timer_);
 }
 
-bool PollingSensorReader::StartFetchingData(
+void PollingSensorReader::StartFetchingData(
     const PlatformSensorConfiguration& configuration) {
+  DCHECK(polling_task_runner_->BelongsToCurrentThread());
+  DCHECK(sensor_);
   if (is_reading_active_)
     StopFetchingData();
-
-  return polling_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&PollingSensorReader::InitializeTimer,
-                            weak_factory_.GetWeakPtr(), configuration));
+  InitializeTimer(configuration);
 }
 
 void PollingSensorReader::StopFetchingData() {
+  DCHECK(polling_task_runner_->BelongsToCurrentThread());
   is_reading_active_ = false;
   timer_->Stop();
 }
 
 void PollingSensorReader::InitializeTimer(
     const PlatformSensorConfiguration& configuration) {
   DCHECK(polling_task_runner_->BelongsToCurrentThread());
+  DCHECK(!is_reading_active_);
   timer_->Start(FROM_HERE, base::TimeDelta::FromMicroseconds(
                                base::Time::kMicrosecondsPerSecond /
                                configuration.frequency()),
                 this, &PollingSensorReader::PollForData);
   is_reading_active_ = true;
 }
 
 void PollingSensorReader::PollForData() {
   DCHECK(polling_task_runner_->BelongsToCurrentThread());
-  base::ThreadRestrictions::AssertIOAllowed();
 
   SensorReading readings;
   DCHECK_LE(sensor_file_paths_.size(), arraysize(readings.values));
   int i = 0;
   for (const auto& path : sensor_file_paths_) {
     std::string new_read_value;
     if (!base::ReadFileToString(path, &new_read_value)) {
       NotifyReadError();
       StopFetchingData();
       return;
@@ skipping 14 matching lines @@
   if (is_reading_active_) {
     task_runner_->PostTask(
         FROM_HERE, base::Bind(&PlatformSensorLinux::UpdatePlatformSensorReading,
                               base::Unretained(sensor_), readings));
   }
 }
 
 // static
 std::unique_ptr<SensorReader> SensorReader::Create(
     const SensorInfoLinux* sensor_device,
-    PlatformSensorLinux* sensor,
-    scoped_refptr<base::SingleThreadTaskRunner> polling_thread_task_runner) {
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+  base::ThreadRestrictions::AssertIOAllowed();
   // TODO(maksims): implement triggered reading. At the moment,
   // only polling read is supported.
-  return base::MakeUnique<PollingSensorReader>(sensor_device, sensor,
-                                               polling_thread_task_runner);
+  return base::MakeUnique<PollingSensorReader>(sensor_device,
+                                               std::move(task_runner));
 }
 
 SensorReader::SensorReader(
-    PlatformSensorLinux* sensor,
-    scoped_refptr<base::SingleThreadTaskRunner> polling_task_runner)
-    : sensor_(sensor),
-      polling_task_runner_(polling_task_runner),
-      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+    : sensor_(nullptr),

[Reilly Grant (use Gerrit), 2016/12/14 18:56:08]

Why do we no longer pass this through the constructor?

[Mikhail, 2016/12/15 10:57:48]

think, we could pass callback closures instead of a sensor instance

[maksims (do not use this acc), 2016/12/16 08:39:22]

Yep, we'll pass callbacks along with a task runner

+      polling_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      task_runner_(std::move(task_runner)),
       is_reading_active_(false) {}
 
 SensorReader::~SensorReader() = default;
 
 void SensorReader::NotifyReadError() {
   if (is_reading_active_) {
     task_runner_->PostTask(
         FROM_HERE, base::Bind(&PlatformSensorLinux::NotifyPlatformSensorError,
                               base::Unretained(sensor_)));
   }
 }
 
+void SensorReader::SetPlatformSensorLinux(PlatformSensorLinux* sensor) {
+  DCHECK(!sensor_);
+  sensor_ = sensor;
+}
+
 }  // namespace device