"ContentHashFetcherTest.MissingVerifiedContents" is flaky |
||||||||||||
Issue description"ContentHashFetcherTest.MissingVerifiedContents" is flaky. This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label. We have detected 5 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
,
Mar 20 2017
Detected 3 new flakes for test/step "ContentHashFetcherTest.MissingVerifiedContents". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
,
Mar 22 2017
Detected 5 new flakes for test/step "ContentHashFetcherTest.MissingVerifiedContents". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. This message was posted automatically by the chromium-try-flakes app.
,
Mar 24 2017
Detected 3 new flakes for test/step "ContentHashFetcherTest.MissingVerifiedContents". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. This message was posted automatically by the chromium-try-flakes app.
,
Mar 25 2017
Detected 4 new flakes for test/step "ContentHashFetcherTest.MissingVerifiedContents". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. This message was posted automatically by the chromium-try-flakes app.
,
Mar 27 2017
,
Mar 27 2017
,
Mar 28 2017
Detected 5 new flakes for test/step "ContentHashFetcherTest.MissingVerifiedContents". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
,
Mar 29 2017
This is still popping up in the Sheriff queue. Any progress that we can make here?
,
Mar 29 2017
I know how to fix this, but don't have enough time to implement the fix right now. I created a revert of https://codereview.chromium.org/2672623006 in the meantime (https://codereview.chromium.org/2787453003/).
,
Mar 30 2017
Detected 3 new flakes for test/step "ContentHashFetcherTest.MissingVerifiedContents". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
,
Mar 31 2017
,
Apr 5 2017
Detected 3 new flakes for test/step "ContentHashFetcherTest.MissingVerifiedContents". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOQsSBUZsYWtlIi5Db250ZW50SGFzaEZldGNoZXJUZXN0Lk1pc3NpbmdWZXJpZmllZENvbnRlbnRzDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
,
Apr 5 2017
Reverting my CL didn't fix the problem. The issue is that the net::URLFetcher owned by ContentHashFetcherJob is not deleted on the sequence from which it is created. IIUC, the net::URLFetcher is created on a browser thread in ContentHashFetcherJob::DoneCheckingForVerifiedContents(). It is then destroyed when the last reference to the ContentHashFetcherJob is dropped. Unfortunately, multiple sequences hold a reference to ContentHashFetcherJob... E.g. Reference owned by the reply running on a browser thread: https://cs.chromium.org/chromium/src/extensions/browser/content_hash_fetcher.cc?l=206 Reference owned by a task running in the blocking pool: https://cs.chromium.org/chromium/src/extensions/browser/content_hash_fetcher.cc?l=344 Assigning to extensions/ owner.
,
Apr 5 2017
,
Apr 5 2017
ContentHashFetcherJob ref count thing is a bit wonky, I guess it's because we expect to call Cancel() on it from any thread?
,
Apr 6 2017
Removing Sheriff-Chromium as it is assigned.
,
Apr 7 2017
This popped up again on sheriff-o-matic today - I'll disable ContentHashFetcherTest.MissingVerifiedContents on Linux.
,
Apr 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d821806d4441025512588c514e66756323eb1408 commit d821806d4441025512588c514e66756323eb1408 Author: lukasza <lukasza@chromium.org> Date: Fri Apr 07 18:35:32 2017 Disable flaky ContentHashFetcherTest.MissingVerifiedContents on Linux. BUG= 702300 TBR=lazyboy@chromium.org Review-Url: https://codereview.chromium.org/2805443005 Cr-Commit-Position: refs/heads/master@{#462936} [modify] https://crrev.com/d821806d4441025512588c514e66756323eb1408/extensions/browser/content_hash_fetcher_unittest.cc
,
Apr 12 2017
I've identified and finally managed to repro the failure. The last ref to ContentHashFetcherJob (CHFJ) is released from ContentHashFetcherWaiter::WaitForCallback(), however if the destruction of CHFJ comes after the test class's TestBrowserThreadBundle destruction, there won't be any thread and the test will fail.
,
Apr 17 2017
Narrowed down the root cause: PostTask T [1] from blocking pool [2] sometimes causes T to run synchronously. I have a isolated repro here: https://codereview.chromium.org/2824033002/ To repro you'd run the test repeatedly [3] while your machine is under stress [4]. fdoray@, do you have any insight to this? [1] https://cs.chromium.org/chromium/src/extensions/browser/content_hash_fetcher.cc?rcl=fb11d048bac3ddc72bb78399e5b7e8057fd050c7&l=342 [2] https://cs.chromium.org/chromium/src/extensions/browser/content_hash_fetcher.cc?rcl=fb11d048bac3ddc72bb78399e5b7e8057fd050c7&l=362 [3] $ ./out/Release/extensions_unittests --enable-pixel-output-in-tests --gtest_filter=ContentHashFetcherTest.TestCase --gtest_repeat=200 [4] For me I'd either run another instance of chrome build w/o goma: $ ninja -C out/Debug chrome -j 1024 or using stress tool (random numbers below) $ stress --cpu 2048 -i 512 --timeout 60
,
Apr 18 2017
What is the expected failure when trying the repro https://codereview.chromium.org/2824033002/?
,
Apr 18 2017
@22, if you run the tests repeatedly, the particular test that hits the case will fail. In particular: ASSERT_TRUE(job->post_task_finished()); will fail because the callback was called during BrowserThread::PostTask. A successful test log will look like: Task1OnBlockingPool BEG Task1OnBlockingPool END Task2OnCreationThread JobWaiter::Callback A failed test log looks like: Task1OnBlockingPool BEG Task2OnCreationThread <- called during PostTask JobWaiter::Callback Task1OnBlockingPool END
,
Apr 18 2017
The blocking pool has its own threads which can run tasks in parallel with the main thread. As soon as Task1OnBlockingPool() posts Task2OnCreationThread() to |creation_thread_|, Task2OnCreationThread() can run. The API doesn't say that Task1OnBlockingPool() has to finish running before Task2OnCreationThread() runs.
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/825099a68908767ecd35d79b0888887c4ffccd0b commit 825099a68908767ecd35d79b0888887c4ffccd0b Author: lazyboy <lazyboy@chromium.org> Date: Wed Apr 19 02:12:04 2017 Fix two ContentHashFetcherTest.* tests' flakiness. ContentHashFetcherJob::url_fetcher_ is always created on ContentHashFetcherJob::creation_thread_ but it was possible for it to be destroyed on blocking pool with the destruction of ContentHashFetcherJob. This causes test flakiness, and this CL makes sure we always destroy url_fetcher_ on correct thread. As an optimization for the most common case, also destroy ContentHashFetcherJob::url_fetcher_ once we're done with it (earlier than above). However, note that because jobs can be cancelled, we still need to make sure to destroy url_fetcher_ on correct thread from ContentHashFetcherJob destructor. BUG= 702300 Test=No visible change. Fixing test flakiness and potential ref issue in content verification code. To repro the flake, I have put my machine on heavy load (compiling chromium without goma and -j 1024), then ran these tests repeatedly with --gtest_repeat flag. Tests will be flaky without this change, but they always pass with this CL. Review-Url: https://codereview.chromium.org/2815243002 Cr-Commit-Position: refs/heads/master@{#465468} [modify] https://crrev.com/825099a68908767ecd35d79b0888887c4ffccd0b/extensions/browser/content_hash_fetcher.cc [modify] https://crrev.com/825099a68908767ecd35d79b0888887c4ffccd0b/extensions/browser/content_hash_fetcher_unittest.cc
,
Apr 19 2017
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by vasi...@chromium.org
, Mar 17 2017Owner: fdoray@chromium.org