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

Issue 869281 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Aug 21
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

google_apis_unittests re-initializes mojo::core which causes leaks

Project Member Reported by mpawlow...@opera.com, Jul 31

Issue description

Chrome Version: (copy from chrome://version)
OS: all

repro:
Build google_apis_unittests with LeakSanitizer and run:
ASAN_OPTIONS="detect_leaks=1" ./google_apis_unittests

The cause is that some test fixtures call mojo::core::Init() in addition to the test runner. This function should be called once per process, not once per test.

This doesn't trigger on bots because they run the binary with --test-launcher-batch-limit=1, which makes each test run in a separate process.
 
Cc: -rockot@google.com roc...@chromium.org
(See also "[chromium-dev] Lots of memory leaks in tests from mojo::core::Initializecore()": https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/5PfKLXKt9Cs)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 20

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

commit a53663e9e5fa4e8e012a89050d818a29589e9119
Author: Maciej Pawlowski <mpawlowski@opera.com>
Date: Mon Aug 20 18:13:53 2018

Don't re-initialize mojo::core in gaia unittests

mojo::core::Init() should be called once per process, not once per test.
run_all_unittests.cc happens to already define a test launcher that
calls mojo::core::Init(), so if we use that instead of the base
launcher, we can get rid of re-initialization in test fixtures.

Bug:  869281 
Change-Id: Ieb96cbe580f5f1750d7f61693c288bb578cb63b0
Reviewed-on: https://chromium-review.googlesource.com/1155595
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584501}
[modify] https://crrev.com/a53663e9e5fa4e8e012a89050d818a29589e9119/google_apis/BUILD.gn
[modify] https://crrev.com/a53663e9e5fa4e8e012a89050d818a29589e9119/google_apis/DEPS
[modify] https://crrev.com/a53663e9e5fa4e8e012a89050d818a29589e9119/google_apis/gaia/DEPS
[modify] https://crrev.com/a53663e9e5fa4e8e012a89050d818a29589e9119/google_apis/gcm/BUILD.gn
[delete] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/google_apis/gcm/test/DEPS
[rename] https://crrev.com/a53663e9e5fa4e8e012a89050d818a29589e9119/google_apis/run_all_unittests.cc
[delete] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/google_apis/test/DEPS
[delete] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/google_apis/test/run_all_unittests.cc

Status: Fixed (was: Untriaged)

Sign in to add a comment