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

device/generic_sensor/platform_sensor_reader_linux.cc

Index: device/generic_sensor/platform_sensor_reader_linux.cc
diff --git a/device/generic_sensor/platform_sensor_reader_linux.cc b/device/generic_sensor/platform_sensor_reader_linux.cc
index 5c014a4b200b43b9f8697bcfa8c5ee1cfc457fb9..e0d3cc8e40e2e4e327882338057ea2c38baaadef 100644
--- a/device/generic_sensor/platform_sensor_reader_linux.cc
+++ b/device/generic_sensor/platform_sensor_reader_linux.cc
@@ -17,14 +17,12 @@ 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;
 
@@ -58,9 +56,8 @@ class PollingSensorReader : public SensorReader {
 
 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),
@@ -72,17 +69,17 @@ 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();
 }
@@ -90,6 +87,7 @@ void PollingSensorReader::StopFetchingData() {
 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()),
@@ -99,7 +97,6 @@ void PollingSensorReader::InitializeTimer(
 
 void PollingSensorReader::PollForData() {
   DCHECK(polling_task_runner_->BelongsToCurrentThread());
-  base::ThreadRestrictions::AssertIOAllowed();
 
   SensorReading readings;
   DCHECK_LE(sensor_file_paths_.size(), arraysize(readings.values));
@@ -134,20 +131,19 @@ void PollingSensorReader::PollForData() {
 // 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;
@@ -160,4 +156,9 @@ void SensorReader::NotifyReadError() {
   }
 }
 
+void SensorReader::SetPlatformSensorLinux(PlatformSensorLinux* sensor) {
+  DCHECK(!sensor_);
+  sensor_ = sensor;
+}
+
 }  // namespace device