Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2569763004:
[sensors](Linux) Fix tsan data race in sensor reader (Closed)
|
Created:
4 years ago by maksims (do not use this acc) Modified:
4 years ago CC:
chromium-reviews, wanming.lin, shalamov, Mikhail Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[sensors](Linux) Fix tsan data race in sensor reader
This CL fixes a tsan data race, which is caused by calling StopFetchingData
from different threads. It must not be allowed. Use the same thread by using
a PostTask.
BUG=673760
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng
Committed: https://crrev.com/d52b2f15ba4712b9dc4a2d08a45aa3210999dc8f
Cr-Commit-Position: refs/heads/master@{#439798}
Patch Set 1 #
Total comments: 3
Patch Set 2 : change threading model. SensorReader now belongs to a polling thread #
Total comments: 1
Patch Set 3 : modify code comments #
Total comments: 11
Patch Set 4 : use callbacks with binded messageloop and fix comments #Patch Set 5 : don't use pointer for timer #
Total comments: 2
Patch Set 6 : don't use BindToCurrentLoop, but rather pass a task runner and callbacks #
Total comments: 5
Patch Set 7 : pass weak_ptr and task_runner #Patch Set 8 : rebased #
Messages
Total messages: 70 (46 generated)
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [sensors](Linux) Fix tsan data race This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 ========== to ========== [sensors](Linux) Fix tsan data race in provider and sensor tests This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 ==========
maksim.sisov@intel.com changed reviewers: + mikhail.pozdnyakov@intel.com, reillyg@chromium.org
please review
Description was changed from ========== [sensors](Linux) Fix tsan data race in provider and sensor tests This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 ========== to ========== [sensors](Linux) Fix tsan data race in sensor reader This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 ==========
gab@chromium.org changed reviewers: + gab@chromium.org
https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... File device/generic_sensor/platform_sensor_reader_linux.cc (right): https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... device/generic_sensor/platform_sensor_reader_linux.cc:125: weak_factory_.GetWeakPtr())); 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).
https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... File device/generic_sensor/platform_sensor_reader_linux.cc (right): https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... device/generic_sensor/platform_sensor_reader_linux.cc:125: weak_factory_.GetWeakPtr())); On 2016/12/13 18:12:45, gab wrote: > 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). Do you mean I need 2 to create two different weak factories for different threads?
https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... File device/generic_sensor/platform_sensor_reader_linux.cc (right): https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... device/generic_sensor/platform_sensor_reader_linux.cc:125: weak_factory_.GetWeakPtr())); On 2016/12/13 18:18:19, maksims wrote: > On 2016/12/13 18:12:45, gab wrote: > > 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). > > Do you mean I need 2 to create two different weak factories for different > threads? 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..?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/12/13 18:23:02, gab wrote: > https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... > File device/generic_sensor/platform_sensor_reader_linux.cc (right): > > https://codereview.chromium.org/2569763004/diff/1/device/generic_sensor/platf... > device/generic_sensor/platform_sensor_reader_linux.cc:125: > weak_factory_.GetWeakPtr())); > On 2016/12/13 18:18:19, maksims wrote: > > On 2016/12/13 18:12:45, gab wrote: > > > 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). > > > > Do you mean I need 2 to create two different weak factories for different > > threads? > > 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..? +1 for separate objects for separate sequences.
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Please review
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Top-level review. Is any of this thread-affine? If not, prefer SequencedTaskRunner which still provides thread-safety and is sufficient for the vast majority of use cases. And I didn't look very closely but FYI, using SequencedTaskRunner::RunsOnCurrentThread() is fine but if all you need is checking a class runs on the same sequence you can use base::SequenceChecker. https://codereview.chromium.org/2569763004/diff/40001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_reader_linux.h (right): https://codereview.chromium.org/2569763004/diff/40001/device/generic_sensor/p... device/generic_sensor/platform_sensor_reader_linux.h:44: // Thread safe. From the implementation this isn't "thread-safe", it's only fine as-is because it's called before the SensorReader is active. i.e. you want something like: // // Sets sensor that this reader reports to. Must be called after the SensorReader is constructed, before tasks are posted to it from another sequence.
On 2016/12/14 14:52:21, gab wrote: > Top-level review. > > Is any of this thread-affine? If not, prefer SequencedTaskRunner which still > provides thread-safety and is sufficient for the vast majority of use cases. > Basically, Destruction is. Timer must be destroyed on the same thread it was started. Other methods like PollForData must be called on a thread that allows IO and do not block browser threads, because the operation it does is reading from sysfs files. > And I didn't look very closely but FYI, using > SequencedTaskRunner::RunsOnCurrentThread() is fine but if all you need is > checking a class runs on the same sequence you can use base::SequenceChecker. > What about base::ThreadChecker? > https://codereview.chromium.org/2569763004/diff/40001/device/generic_sensor/p... > File device/generic_sensor/platform_sensor_reader_linux.h (right): > > https://codereview.chromium.org/2569763004/diff/40001/device/generic_sensor/p... > device/generic_sensor/platform_sensor_reader_linux.h:44: // Thread safe. > From the implementation this isn't "thread-safe", it's only fine as-is because > it's called before the SensorReader is active. > > i.e. you want something like: > > // // Sets sensor that this reader reports to. Must be called after the > SensorReader is constructed, before tasks are posted to it from another > sequence. Yeah...right
On 2016/12/14 17:35:43, maksims wrote: > On 2016/12/14 14:52:21, gab wrote: > > Top-level review. > > > > Is any of this thread-affine? If not, prefer SequencedTaskRunner which still > > provides thread-safety and is sufficient for the vast majority of use cases. > > > Basically, Destruction is. Timer must be destroyed on the same thread it was > started. > Other methods like PollForData must be called on a thread that allows IO and do > not block > browser threads, because the operation it does is reading from sysfs files. Ah that's unfortunate, we're working on making many base/ APIs also respect this rule (and Timer is a tricky one per an existing race in the current impl: https://codereview.chromium.org/2491613004/). Don't expect you to block on that so SingleThreadTaskRunner it is for you and in a brave eventual world it will be possible to switch this to sequenced :). > > > And I didn't look very closely but FYI, using > > SequencedTaskRunner::RunsOnCurrentThread() is fine but if all you need is > > checking a class runs on the same sequence you can use base::SequenceChecker. > > > What about base::ThreadChecker? Yep that works too (you might need to DetachFromThread() in constructor if constructing on different thread than rest of usage -- see other users in codebase). > > > https://codereview.chromium.org/2569763004/diff/40001/device/generic_sensor/p... > > File device/generic_sensor/platform_sensor_reader_linux.h (right): > > > > > https://codereview.chromium.org/2569763004/diff/40001/device/generic_sensor/p... > > device/generic_sensor/platform_sensor_reader_linux.h:44: // Thread safe. > > From the implementation this isn't "thread-safe", it's only fine as-is because > > it's called before the SensorReader is active. > > > > i.e. you want something like: > > > > // // Sets sensor that this reader reports to. Must be called after the > > SensorReader is constructed, before tasks are posted to it from another > > sequence. > Yeah...right
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Reilly, can you comment please?
https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_provider_linux.cc (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.cc:171: base::Unretained(this), type)); It doesn't seem necessary to pass this as a callback. SensorReaderCreated can just call NotifySensorCreated directly. https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_provider_linux.h (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.h:58: // thread. *created on |polling_thread_|. https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.h:63: SensorInfoLinux* const sensor_device, You mean const SensorInfoLinux*. https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_reader_linux.cc (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_reader_linux.cc:144: : sensor_(nullptr), Why do we no longer pass this through the constructor?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_provider_linux.cc (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.cc:60: base::Bind(SensorReader::Create, sensor_device, Let's delegate SensorReader creation to the sensor itself. https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_reader_linux.cc (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_reader_linux.cc:144: : sensor_(nullptr), think, we could pass callback closures instead of a sensor instance
Description was changed from ========== [sensors](Linux) Fix tsan data race in sensor reader This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 ========== to ========== [sensors](Linux) Fix tsan data race in sensor reader This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng ==========
FYI, suppressed this TSAN failure for now in https://codereview.chromium.org/2579003002/ given it was the last blocker to a green TSAN bot, please remove the suppression in this CL. Also, I added CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng to this CL to force the TSAN bot in your CQ. Thanks, Gab
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/12/15 12:02:49, gab wrote: > FYI, suppressed this TSAN failure for now in > https://codereview.chromium.org/2579003002/ given it was the last blocker to a > green TSAN bot, please remove the suppression in this CL. > > Also, I added > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng > to this CL to force the TSAN bot in your CQ. > > Thanks, > Gab Thanks, Gab. I will do that once we solve this problem in the most accurate and clear way.
https://codereview.chromium.org/2569763004/diff/100001/device/generic_sensor/... File device/generic_sensor/platform_sensor_linux.cc (right): https://codereview.chromium.org/2569763004/diff/100001/device/generic_sensor/... device/generic_sensor/platform_sensor_linux.cc:45: sensor_device, task_runner_, update_reading_cb, notify_read_error_cb); Since these callbacks are bound to the current message loop the task_runner_ passed to SensorReader::Create is unused. Like my earlier comments, since SensorReader will only ever call these two functions on PlatformSensorLinux I would rather we didn't use callbacks here and just passed task_runner_ and weak_factory_.GetWeakPtr() to SensorReader::Create but either way works fine.
On 2016/12/15 18:37:31, maksims wrote: > On 2016/12/15 12:02:49, gab wrote: > > FYI, suppressed this TSAN failure for now in > > https://codereview.chromium.org/2579003002/ given it was the last blocker to a > > green TSAN bot, please remove the suppression in this CL. > > > > Also, I added > > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng > > to this CL to force the TSAN bot in your CQ. > > > > Thanks, > > Gab > > Thanks, Gab. I will do that once we solve this problem in the most accurate and > clear way. To be clear, you should remove the suppression in this CL so the CQ gets a shot at your fix.
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Patchset #6 (id:120001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_provider_linux.cc (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.cc:60: base::Bind(SensorReader::Create, sensor_device, On 2016/12/15 10:57:48, Mikhail wrote: > Let's delegate SensorReader creation to the sensor itself. Done. https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.cc:171: base::Unretained(this), type)); On 2016/12/14 18:56:08, Reilly Grant wrote: > It doesn't seem necessary to pass this as a callback. SensorReaderCreated can > just call NotifySensorCreated directly. Done. https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_provider_linux.h (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.h:58: // thread. On 2016/12/14 18:56:08, Reilly Grant wrote: > *created on |polling_thread_|. comment removed https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_provider_linux.h:63: SensorInfoLinux* const sensor_device, On 2016/12/14 18:56:08, Reilly Grant wrote: > You mean const SensorInfoLinux*. Done. https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... File device/generic_sensor/platform_sensor_reader_linux.cc (right): https://codereview.chromium.org/2569763004/diff/60001/device/generic_sensor/p... device/generic_sensor/platform_sensor_reader_linux.cc:144: : sensor_(nullptr), Yep, we'll pass callbacks along with a task runner https://codereview.chromium.org/2569763004/diff/100001/device/generic_sensor/... File device/generic_sensor/platform_sensor_linux.cc (right): https://codereview.chromium.org/2569763004/diff/100001/device/generic_sensor/... device/generic_sensor/platform_sensor_linux.cc:45: sensor_device, task_runner_, update_reading_cb, notify_read_error_cb); On 2016/12/15 18:51:31, Reilly Grant wrote: > Since these callbacks are bound to the current message loop the task_runner_ > passed to SensorReader::Create is unused. > > Like my earlier comments, since SensorReader will only ever call these two > functions on PlatformSensorLinux I would rather we didn't use callbacks here and > just passed task_runner_ and weak_factory_.GetWeakPtr() to SensorReader::Create > but either way works fine. I think passing a callback with binded weakPtr to a sensor and a task runner is our solution.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... File device/generic_sensor/platform_sensor_linux.cc (right): https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... device/generic_sensor/platform_sensor_linux.cc:42: sensor_device, task_runner_, update_reading_cb, notify_read_error_cb); Now you've negated the benefit of passing callbacks because they are no longer bound to the thread. Please either: a) Pass a TaskRunner and WeakPtr<PlatformSensorLinux>. b) Pass 2 callbacks created with base::BindToCurrentLoop (which apparently doesn't exist anymore?) https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... File device/generic_sensor/platform_sensor_reader_linux.h (right): https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... device/generic_sensor/platform_sensor_reader_linux.h:63: // to the same MessageLoop where they are created. This comment is out of date because they are no longer created with base::BindToCurrentLoop. https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... device/generic_sensor/platform_sensor_reader_linux.h:68: scoped_refptr<base::SingleThreadTaskRunner> polling_task_runner_; This field is unused.
Patchset #7 (id:160001) has been deleted
Patchset #7 (id:180001) has been deleted
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Please review https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... File device/generic_sensor/platform_sensor_linux.cc (right): https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... device/generic_sensor/platform_sensor_linux.cc:42: sensor_device, task_runner_, update_reading_cb, notify_read_error_cb); On 2016/12/16 21:34:42, Reilly Grant wrote: > Now you've negated the benefit of passing callbacks because they are no longer > bound to the thread. Please either: > > a) Pass a TaskRunner and WeakPtr<PlatformSensorLinux>. > b) Pass 2 callbacks created with base::BindToCurrentLoop (which apparently > doesn't exist anymore?) Done. https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... File device/generic_sensor/platform_sensor_reader_linux.h (right): https://codereview.chromium.org/2569763004/diff/140001/device/generic_sensor/... device/generic_sensor/platform_sensor_reader_linux.h:68: scoped_refptr<base::SingleThreadTaskRunner> polling_task_runner_; On 2016/12/16 21:34:42, Reilly Grant wrote: > This field is unused. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
On 2016/12/19 20:42:04, Reilly Grant wrote: > lgtm lgtm for build/sanitizers/tsan_suppressions.cc and r-s lgtm for the rest based on Reilly's review (assuming my initial comments about the design have since been addressed).
The CQ bit was checked by maksim.sisov@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org, reillyg@chromium.org Link to the patchset: https://codereview.chromium.org/2569763004/#ps220001 (title: "rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 220001, "attempt_start_ts": 1482236959358250,
"parent_rev": "ff550cd0eae2487918e37f139dea8d6b8c99169e", "commit_rev":
"ee2392000a31e2180dda008968474a0b41ee8250"}
Message was sent while issue was closed.
Description was changed from ========== [sensors](Linux) Fix tsan data race in sensor reader This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng ========== to ========== [sensors](Linux) Fix tsan data race in sensor reader This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng Review-Url: https://codereview.chromium.org/2569763004 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:220001)
Message was sent while issue was closed.
Description was changed from ========== [sensors](Linux) Fix tsan data race in sensor reader This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng Review-Url: https://codereview.chromium.org/2569763004 ========== to ========== [sensors](Linux) Fix tsan data race in sensor reader This CL fixes a tsan data race, which is caused by calling StopFetchingData from different threads. It must not be allowed. Use the same thread by using a PostTask. BUG=673760 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng Committed: https://crrev.com/d52b2f15ba4712b9dc4a2d08a45aa3210999dc8f Cr-Commit-Position: refs/heads/master@{#439798} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/d52b2f15ba4712b9dc4a2d08a45aa3210999dc8f Cr-Commit-Position: refs/heads/master@{#439798} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
