Fix race condition in BrowsingDataCounter |
||
Issue descriptionThis cl showed that there is a race condition when a BrowsingDataCounter is restarted while it is counting: https://codereview.chromium.org/2869683002/diff/1/chrome/browser/browsing_data/cache_counter.cc To fix this there should be a virtual StopCounting() method that is implemented by all asynchronous counters in order to invalidate their weak_ptr_factory or stop cancellable tasks. This way they won't report a result when counting restarted. Log output: 05-08 10:19:14.193 31093-31093/org.chromium.chrome E/chromium: [ERROR:cache_counter.cc(39)] START COUNTING !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 05-08 10:19:14.278 31093-31093/org.chromium.chrome E/chromium: [ERROR:cache_counter.cc(63)] START COUNTING OFFLINE !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 05-08 10:19:14.345 31093-31093/org.chromium.chrome E/chromium: [ERROR:cache_counter.cc(39)] START COUNTING !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 05-08 10:19:14.345 31093-31093/org.chromium.chrome E/chromium: [ERROR:cache_counter.cc(81)] REPORT RESULT OFFLINE!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 05-08 10:19:14.358 31093-31093/org.chromium.chrome E/chromium: [ERROR:cache_counter.cc(63)] START COUNTING OFFLINE !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 05-08 10:19:14.377 31093-31093/org.chromium.chrome E/chromium: [ERROR:cache_counter.cc(81)] REPORT RESULT OFFLINE!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! 05-08 10:19:14.420 31093-31093/org.chromium.chrome A/chromium: [FATAL:browsing_data_counter.cc(86)] Check failed: false.
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a34462a6f8d57df524dd8cb17e61b094dedee879 commit a34462a6f8d57df524dd8cb17e61b094dedee879 Author: dullweber <dullweber@chromium.org> Date: Mon May 08 10:49:38 2017 Fix BrowsingDataCounter race condition There is a chance that a BrowsingDataCounter is still counting when it is restarted. To avoid calling ReportResult twice and hitting a NOTREACHED() all asynchronous tasks have to be cancelled when Count() is called. BUG= 719376 Review-Url: https://codereview.chromium.org/2867033002 Cr-Commit-Position: refs/heads/master@{#469956} [modify] https://crrev.com/a34462a6f8d57df524dd8cb17e61b094dedee879/chrome/browser/browsing_data/cache_counter.cc [modify] https://crrev.com/a34462a6f8d57df524dd8cb17e61b094dedee879/chrome/browser/browsing_data/media_licenses_counter.cc [modify] https://crrev.com/a34462a6f8d57df524dd8cb17e61b094dedee879/chrome/browser/browsing_data/site_data_counter.cc [modify] https://crrev.com/a34462a6f8d57df524dd8cb17e61b094dedee879/components/browsing_data/core/counters/browsing_data_counter.h [modify] https://crrev.com/a34462a6f8d57df524dd8cb17e61b094dedee879/components/browsing_data/core/counters/history_counter.cc
,
May 8 2017
Fixed by cancelling previous tasks in Count() instead of adding another virtual method. |
||
►
Sign in to add a comment |
||
Comment 1 by dullweber@chromium.org
, May 8 2017