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

Issue 755118 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocked on:
issue 755670



Sign in to add a comment

SafeBrowsingUIManagerTest.Whitelist* flakily leaks

Project Member Reported by treib@chromium.org, Aug 14 2017

Issue description

Various tests in SafeBrowsingUIManagerTest sometimes leak memory, causing failures on "Linux ASan LSan Tests (1)" and in the CQ.

I've seen the following tests fail:
.Whitelist
.WhitelistIgnoresPath
.WhitelistIgnoresSitesNotAdded
.WhitelistRemembersThreatType

First failure, from August 11:
https://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%281%29/builds/37927

List of builds from the ASan LSan bot:
https://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%281%29?numbuilds=200

CCing OWNERS, since I couldn't find a recent change that looks related.
 

Comment 1 by treib@chromium.org, Aug 14 2017

Labels: Sheriff-Chromium

Comment 2 by treib@chromium.org, Aug 14 2017

I'm disabling the leaky tests on LSan: https://chromium-review.googlesource.com/c/613165
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 14 2017

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

commit 3d26e9293b5bc4503b8af4af335b5dce8ae70127
Author: Marc Treib <treib@chromium.org>
Date: Mon Aug 14 16:58:26 2017

Disable leaky SafeBrowsingUIManagerTest.Whitelist* on LSan

TBR=jialiul@chromium.org

Bug: 755118
Change-Id: Ia919fe8edd8effb3fff6d92905e326321af941ce
Reviewed-on: https://chromium-review.googlesource.com/613165
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494075}
[modify] https://crrev.com/3d26e9293b5bc4503b8af4af335b5dce8ae70127/chrome/browser/safe_browsing/ui_manager_unittest.cc

Comment 4 by treib@chromium.org, Aug 14 2017

Components: Tests>Disabled
Labels: -Sheriff-Chromium
Taking out of the sheriff queue as the offending tests have been disabled.
Labels: -Pri-1 Pri-2
Owner: vakh@chromium.org
Status: Assigned (was: Untriaged)
(To be clear these were disabled for leak detection, but they're still run for correctness.)

The leaks all look related to SafeBrowsingDatabaseFactoryImpl::CreateSafeBrowsingDatabase()

This is unlikely to cause progressive leaks within production code, but we should fix it and renable the test.

Comment 6 by vakh@chromium.org, Aug 15 2017

Blockedon: 755670
Labels: SafeBrowsing-Triaged
This only leaks in the that test, but the "leaking" code is run in other tests so it's probably an issue with this test fixture.
Project Member

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

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

commit 199c013b4cae899bcd7dd22f563fb8d399e8bdda
Author: Evgenii Stepanov <eugenis@google.com>
Date: Fri Aug 18 21:19:41 2017

Disable the rest of SafeBrowsingUIManagerTest under LSan.

Flaky leak reports on this bot:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinuxASan%20tester
BUG=755118
TBR=jialiul@chromium.org, treib@chromium.org

Change-Id: I200d3e709a2a6ad601dd1b037cd24fb36919b8f4
Reviewed-on: https://chromium-review.googlesource.com/619855
Reviewed-by: Evgeniy Stepanov <eugenis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495688}
[modify] https://crrev.com/199c013b4cae899bcd7dd22f563fb8d399e8bdda/chrome/browser/safe_browsing/ui_manager_unittest.cc

Comment 9 by est...@chromium.org, Nov 10 2017

Labels: Hotlist-EnamelAndFriendsFixIt
Components: Tests>Disabled
Labels: Test-Disabled
Labels: -Hotlist-EnamelAndFriendsFixIt
Cc: linds...@chromium.org
Labels: Hotlist-DisableReview
These tests have been disabled since aug, should they just get deleted if there isn't a fix?
Theses tests are still run, for correctness.  

Sign in to add a comment