[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"
@@ skipping 56 matching lines @@
       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(
     const PlatformSensorConfiguration& configuration) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
   if (is_reading_active_)
     StopFetchingData();
 
   return polling_task_runner_->PostTask(
       FROM_HERE, base::Bind(&PollingSensorReader::InitializeTimer,
                             weak_factory_.GetWeakPtr(), configuration));
 }
 
 void PollingSensorReader::StopFetchingData() {
+  DCHECK(task_runner_->BelongsToCurrentThread());
   is_reading_active_ = false;
   timer_->Stop();
 }
 
 void PollingSensorReader::InitializeTimer(
     const PlatformSensorConfiguration& configuration) {
   DCHECK(polling_task_runner_->BelongsToCurrentThread());
   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();
+      task_runner_->PostTask(FROM_HERE,
+                             base::Bind(&PollingSensorReader::StopFetchingData,
+                                        weak_factory_.GetWeakPtr()));
       return;
     }
 
     double new_value = 0;
     base::TrimWhitespaceASCII(new_read_value, base::TRIM_ALL, &new_read_value);
     if (!base::StringToDouble(new_read_value, &new_value)) {
       NotifyReadError();
-      StopFetchingData();
+      task_runner_->PostTask(FROM_HERE,
+                             base::Bind(&PollingSensorReader::StopFetchingData,
+                                        weak_factory_.GetWeakPtr()));

[gab, 2016/12/13 18:12:45]

WeakPtr is also not thread-safe (can't obtain it on one thread and check its
validity on another).

In general only a class's owning sequence should operate on its members (which
weak_factory_ is one of).

[maksims (do not use this acc), 2016/12/13 18:18:19]

Do you mean I need 2 to create two different weak factories for different
threads?

[gab, 2016/12/13 18:23:02]

classes in Chromium are typically bound to a single sequence
(SequencedTaskRunner -- or thread) in this case seems like some methods are
meant to run on one sequence and other methods on another sequence.

Typically this would be split in two classes, one for each sequence.

How do you ensure proper shutdown order today? i.e. there can be pending tasks
on two sequence to one class which can only be deleted on one sequence..?

       return;
     }
     readings.values[i++] = new_value;
   }
   if (!apply_scaling_func_.is_null())
     apply_scaling_func_.Run(scaling_value_, offset_value_, readings);
 
   if (is_reading_active_) {
     task_runner_->PostTask(
         FROM_HERE, base::Bind(&PlatformSensorLinux::UpdatePlatformSensorReading,
@@ skipping 24 matching lines @@
 
 void SensorReader::NotifyReadError() {
   if (is_reading_active_) {
     task_runner_->PostTask(
         FROM_HERE, base::Bind(&PlatformSensorLinux::NotifyPlatformSensorError,
                               base::Unretained(sensor_)));
   }
 }
 
 }  // namespace device