Use-after-free for AddToHomescreenDataFetcherTestCommon.ManifestShort* tests |
|
Issue descriptionPerhaps related to the flakiness that originally reverted your CL? https://uberchromegw.corp.google.com/i/internal.client.clank/builders/asan-clang-phone/builds/1541/steps/unit_tests/logs/stdio
,
Jan 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a286ed6b9406edfb1031ae1d48766f79652a6ca commit 4a286ed6b9406edfb1031ae1d48766f79652a6ca Author: pkotwicz <pkotwicz@chromium.org> Date: Tue Jan 17 20:45:26 2017 Fix use-after-free in base::Timer::StopAndAbandon() A timer may be owned by a ref counted class. If |Timer::user_task_| is a method in the ref counted class, it is possible for |Timer::user_task_| to hold the only reference to the ref counted class. This CL changes Timer::StopAndAbandon() so that Timer::Stop() is called after Timer::AbandonScheduledTask(). If |Timer::user_task_| holds the only reference to ref counted class, Timer::Stop() destroys the timer object. Timer::StopAndAbandon() can be called while |Timer::user_task_| holds the only reference to the ref counted class if the message loop is shut down. BUG= 678592 TEST=MessageLoopShutdownSelfOwningTimer Review-Url: https://codereview.chromium.org/2624133004 Cr-Commit-Position: refs/heads/master@{#444132} [modify] https://crrev.com/4a286ed6b9406edfb1031ae1d48766f79652a6ca/base/timer/timer.h [modify] https://crrev.com/4a286ed6b9406edfb1031ae1d48766f79652a6ca/base/timer/timer_unittest.cc
,
Jun 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a commit 4e844bb5f686dbd3a062c6aaf83c8a1df171a08a Author: gab <gab@chromium.org> Date: Thu Jun 01 16:23:36 2017 Make base::Timer sequence-friendly. This CL was also going to fix a race condition in the existing implementation when SetTaskRunner() was used but the test-only failures caused by tests having long-lived with the status quo are proving hard than expected to fix. The race is no worse than before by making this code sequence-friendly (and generally healthier). Will fix race in a follow-up CL. Of note in this CL: - Patch set 1 is https://codereview.chromium.org/1433373003 so looking at diff from 1 might be less work (especially for tests). - (not true anymore, postponed to race fix CL:) The Timer's delayed task now always lives on the sequence it was started from (and even SetTaskRunner() was used, a task is posted to it when the delay expires instead of having the Timer's delayed task live on it -- this solves the aforementioned race condition). - This required adapting tests for MediaCodecLoop and UploadProgressTracker. BUG= 587199 , 552633 , 678592 , 684640, 675631 Review-Url: https://codereview.chromium.org/2491613004 Cr-Commit-Position: refs/heads/master@{#476317} [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/base/timer/timer.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/base/timer/timer.h [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/base/timer/timer_unittest.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/build/sanitizers/tsan_suppressions.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/content/browser/loader/upload_progress_tracker.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/content/browser/loader/upload_progress_tracker.h [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/content/browser/loader/upload_progress_tracker_unittest.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/content/browser/service_worker/service_worker_browsertest.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/content/browser/service_worker/service_worker_job_unittest.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/media/base/android/media_codec_loop.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/media/base/android/media_codec_loop.h [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/media/base/android/media_codec_loop_unittest.cc [modify] https://crrev.com/4e844bb5f686dbd3a062c6aaf83c8a1df171a08a/media/filters/android/media_codec_audio_decoder.cc |
|
►
Sign in to add a comment |
|
Comment 1 by pkotw...@chromium.org
, Jan 12 2017Status: Duplicate (was: Untriaged)