New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 756844 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

SafeBrowsingBlockingQuietPageTest fails with CrOS Asan

Project Member Reported by kylec...@chromium.org, Aug 18 2017

Issue description

All three SafeBrowsingBlockingQuietPageTests fail under CrOS ASan. Right now unit_tests isn't running on CrOS ASan trybot. I am trying to get unit_tests running on it again and this is blocking that.

GN args:
dcheck_always_on = true
is_asan = true
is_component_build = false
is_debug = false
is_lsan = true
strip_absolute_paths_from_debug_symbols = true
symbol_level = 1
target_os = "chromeos"
use_goma = true

$ export ASAN_OPTIONS="detect_leaks=1 symbolize=1 external_symbolizer_path=./third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer"
$ ninja -C out/asan unit_tests
$ out/asan/unit_tests --gtest_filter=SafeBrowsingBlockingQuietPageTest.*

There ares lots of leaks but I think it all has to do with LocalSafeBrowsingDatabaseManager::database_ not getting deleted? Here is one example.

Indirect leak of 784 byte(s) in 2 object(s) allocated from:
    #0 0xac8572 in operator new(unsigned long) (/usr/local/google/data/chrome/src/out/asan/unit_tests+0xac8572)
    #1 0x16ac9a26 in make_unique<safe_browsing::SafeBrowsingDatabaseNew, const scoped_refptr<base::SequencedTaskRunner> &, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *> buildtools/third_party/libc++/trunk/include/memory:3065:28
    #2 0x16ac9a26 in MakeUnique<safe_browsing::SafeBrowsingDatabaseNew, const scoped_refptr<base::SequencedTaskRunner> &, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *, safe_browsing::SafeBrowsingStoreFile *> base/memory/ptr_util.h:25
    #3 0x16ac9a26 in safe_browsing::SafeBrowsingDatabaseFactoryImpl::CreateSafeBrowsingDatabase(scoped_refptr<base::SequencedTaskRunner> const&, bool, bool, bool, bool, bool, bool, bool) chrome/browser/safe_browsing/safe_browsing_database.cc:292
    #4 0x16aa9ce8 in safe_browsing::SafeBrowsingDatabase::Create(scoped_refptr<base::SequencedTaskRunner> const&, bool, bool, bool, bool, bool, bool, bool) chrome/browser/safe_browsing/safe_browsing_database.cc:330:20
    #5 0x16a854e3 in safe_browsing::LocalSafeBrowsingDatabaseManager::GetDatabase() chrome/browser/safe_browsing/local_database_manager.cc:826:52
    #6 0x1092e3e5 in Run base/callback.h:91:12
    #7 0x1092e3e5 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:59
    #8 0x10aa9cfb in base::internal::TaskTracker::PerformRunTask(std::__1::unique_ptr<base::internal::Task, std::__1::default_delete<base::internal::Task> >, base::internal::Sequence*) base/task_scheduler/task_tracker.cc:335:28
    #9 0x10aab2a9 in base::internal::TaskTrackerPosix::PerformRunTask(std::__1::unique_ptr<base::internal::Task, std::__1::default_delete<base::internal::Task> >, base::internal::Sequence*) base/task_scheduler/task_tracker_posix.cc:22:16
    #10 0xe3535ca in base::test::ScopedTaskEnvironment::TestTaskTracker::PerformRunTask(std::__1::unique_ptr<base::internal::Task, std::__1::default_delete<base::internal::Task> >, base::internal::Sequence*) base/test/scoped_task_environment.cc:237:49
    #11 0x10aa83ab in base::internal::TaskTracker::RunNextTask(base::internal::Sequence*) base/task_scheduler/task_tracker.cc:251:5
    #12 0x10bc50ac in base::internal::SchedulerWorker::Thread::ThreadMain() base/task_scheduler/scheduler_worker.cc:73:34
    #13 0x10ac20e2 in base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:75:13
    #14 0x7f5426b4d183 in start_thread /build/eglibc-SvCtMH/eglibc-2.19/nptl/pthread_create.c:312

+edwardjung can you help triage? It looks like |database_| is supposed to be destroyed on another thread but I'm not sure how that interacts with the tests.
 
Blocking: 742623
Cc: nparker@chromium.org ntfschr@chromium.org jialiul@chromium.org
Sorry for the delay, I'm just catching up from OOO. 

I'm not sure either how LocalSafeBrowsingDatabaseManager:database_ is used in the tests. Adding some folks who might have knowledge about this.


Cc: vakh@chromium.org
Components: Mobile>WebView Services>Safebrowsing
Status: Assigned (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1474c3ee4a892d535837c52d29dddfc75fe79f85

commit 1474c3ee4a892d535837c52d29dddfc75fe79f85
Author: kylechar <kylechar@chromium.org>
Date: Mon Aug 28 19:54:12 2017

Enable tests on CrOS ASan trybot.

Enable browser_tests and unit_tests on CrOS ASan trybot. Both tests have
failing test cases, so add a filter file for both.

Bug:  742623 ,  756844 ,  759291 
Change-Id: Iffe10dba5d17f8539ebf57c18c0ef40e4e6221da
Reviewed-on: https://chromium-review.googlesource.com/636397
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497851}
[modify] https://crrev.com/1474c3ee4a892d535837c52d29dddfc75fe79f85/testing/buildbot/chromium.memory.json
[add] https://crrev.com/1474c3ee4a892d535837c52d29dddfc75fe79f85/testing/buildbot/filters/browser_tests_cros_asan.filter
[add] https://crrev.com/1474c3ee4a892d535837c52d29dddfc75fe79f85/testing/buildbot/filters/unit_tests_cros_asan.filter

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/354d866de4a66eb05a285828d4931a83f34de7e2

commit 354d866de4a66eb05a285828d4931a83f34de7e2
Author: Henrik Andreasson <henrika@chromium.org>
Date: Tue Aug 29 09:17:55 2017

Revert "Enable tests on CrOS ASan trybot."

This reverts commit 1474c3ee4a892d535837c52d29dddfc75fe79f85.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=759944

Original change's description:
> Enable tests on CrOS ASan trybot.
> 
> Enable browser_tests and unit_tests on CrOS ASan trybot. Both tests have
> failing test cases, so add a filter file for both.
> 
> Bug:  742623 ,  756844 ,  759291 
> Change-Id: Iffe10dba5d17f8539ebf57c18c0ef40e4e6221da
> Reviewed-on: https://chromium-review.googlesource.com/636397
> Commit-Queue: kylechar <kylechar@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#497851}

TBR=sky@chromium.org,kylechar@chromium.org

Change-Id: I63f9a5a2baddf69b0d74d85edb51fb11cff35139
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  742623 ,  756844 ,  759291 
Reviewed-on: https://chromium-review.googlesource.com/640710
Reviewed-by: Henrik Andreasson <henrika@chromium.org>
Commit-Queue: Henrik Andreasson <henrika@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498060}
[modify] https://crrev.com/354d866de4a66eb05a285828d4931a83f34de7e2/testing/buildbot/chromium.memory.json
[delete] https://crrev.com/d4287fc3f51190e6e8c698e36903b47c464596b1/testing/buildbot/filters/browser_tests_cros_asan.filter
[delete] https://crrev.com/d4287fc3f51190e6e8c698e36903b47c464596b1/testing/buildbot/filters/unit_tests_cros_asan.filter

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4933d77d77e88629508587e5c36420d2e102c358

commit 4933d77d77e88629508587e5c36420d2e102c358
Author: kylechar <kylechar@chromium.org>
Date: Tue Aug 29 17:32:36 2017

Enable tests on CrOS ASan trybot.

Enable browser_tests and unit_tests on CrOS ASan trybot. Both tests have
failing test cases, so add a filter file for both. Also add the filter
files to data_deps in the appropriate test. The missing data_deps caused
this CL to be reverted originally.

Bug:  742623 ,  756844 ,  759291 
Change-Id: If6f45f418d4afb7e17fd6c439aa90a9e4062cf08
Reviewed-on: https://chromium-review.googlesource.com/641210
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498153}
[modify] https://crrev.com/4933d77d77e88629508587e5c36420d2e102c358/chrome/test/BUILD.gn
[modify] https://crrev.com/4933d77d77e88629508587e5c36420d2e102c358/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/4933d77d77e88629508587e5c36420d2e102c358/testing/buildbot/filters/BUILD.gn
[add] https://crrev.com/4933d77d77e88629508587e5c36420d2e102c358/testing/buildbot/filters/browser_tests_cros_asan.filter
[add] https://crrev.com/4933d77d77e88629508587e5c36420d2e102c358/testing/buildbot/filters/unit_tests_cros_asan.filter

Comment 9 by vakh@chromium.org, Sep 1 2017

Cc: edwardjung@chromium.org
Labels: SafeBrowsing-Triaged
Owner: vakh@chromium.org
This seems to be related to PVer3 deprecation so assigning to myself.
Labels: WebView-SafeBrowsing
Labels: Hotlist-EnamelAndFriendsFixIt
Labels: -OS-iOS
Labels: -Hotlist-EnamelAndFriendsFixIt
Blocking: -742623
I'm removing the filter for these tests as they don't exist anymore. Not sure if this bug is still relevant at all anymore?
Wait scratch that. The filter had the wrong test name, SafeBrowsingBlockingQuietPageTests instead of SafeBrowsingBlockingQuietPageTest. The tests are still there and the filter wasn't doing anything.
Status: WontFix (was: Assigned)
Sounds like the test isn't all that flaky then :-)

Sign in to add a comment