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

Issue 906769 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: GCMConnectionHandlerImplTest.InitIncompleteTimeout



Sign in to add a comment

Many tests in gcm_unit_tests are flakily leaking

Project Member Reported by Findit, Nov 19

Issue description

Components: Services>CloudMessaging
Summary: Many tests in gcm_unit_tests are flakily leaking (was: GCMConnectionHandlerImplTest.InitIncompleteTimeout is flaky)
There are many tests in gcm_unit_tests failing in the same manner. There's no
obvious culprit in the recent CLs.

I'm going to dedupe similar bugs into this.
 Issue 906770  has been merged into this issue.
 Issue 906771  has been merged into this issue.
 Issue 906772  has been merged into this issue.
 Issue 906773  has been merged into this issue.
 Issue 906774  has been merged into this issue.
 Issue 906775  has been merged into this issue.
 Issue 906776  has been merged into this issue.
 Issue 906777  has been merged into this issue.
 Issue 906778  has been merged into this issue.
 Issue 906819  has been merged into this issue.
 Issue 906820  has been merged into this issue.
 Issue 906821  has been merged into this issue.
 Issue 906822  has been merged into this issue.
 Issue 906823  has been merged into this issue.
 Issue 906824  has been merged into this issue.
 Issue 906825  has been merged into this issue.
 Issue 906826  has been merged into this issue.
 Issue 906827  has been merged into this issue.
 Issue 906828  has been merged into this issue.
Really many objects are leaked, but the ultimate source of the leak seems like:

Indirect leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x55a24ab11c12 in operator new(unsigned long) /b/swarming/w/ir/kitchen-workdir/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:106:3
    #1 0x55a24e29bd2b in base::MessageLoopImpl::MessageLoopImpl(base::MessageLoopBase::Type) base/message_loop/message_loop_impl.cc:309:11
    #2 0x55a24e2955e5 in make_unique<base::MessageLoopImpl, base::MessageLoopBase::Type &> buildtools/third_party/libc++/trunk/include/memory:3118:32
    #3 0x55a24e2955e5 in CreateMessageLoopImpl base/message_loop/message_loop.cc:210
    #4 0x55a24e2955e5 in base::MessageLoop::MessageLoop(base::MessageLoopBase::Type, base::OnceCallback<std::__1::unique_ptr<base::MessagePump, std::__1::default_delete<base::MessagePump> > ()>, base::MessageLoop::BackendType) base/message_loop/message_loop.cc:196
    #5 0x55a24e295354 in MessageLoop base/message_loop/message_loop.cc:187:7
    #6 0x55a24e295354 in base::MessageLoop::CreateUnbound(base::MessageLoopBase::Type, base::OnceCallback<std::__1::unique_ptr<base::MessagePump, std::__1::default_delete<base::MessagePump> > ()>) base/message_loop/message_loop.cc:183
    #7 0x55a24e41387f in base::Thread::StartWithOptions(base::Thread::Options const&) base/threading/thread.cc:101:7
    #8 0x55a24e611cdd in net::NetworkChangeNotifierLinux::NetworkChangeNotifierLinux(std::__1::unordered_set<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) net/base/network_change_notifier_linux.cc:107:21
    #9 0x55a24e5f568e in net::NetworkChangeNotifier::Create() net/base/network_change_notifier.cc:219:14
    #10 0x55a24f14d226 in CreateNetworkChangeNotifierIfNeeded services/network/network_service.cc:100:29
    #11 0x55a24f14d226 in network::NetworkService::NetworkService(std::__1::unique_ptr<service_manager::BinderRegistryWithArgs<>, std::__1::default_delete<service_manager::BinderRegistryWithArgs<> > >, mojo::InterfaceRequest<network::mojom::NetworkService>, net::NetLog*) services/network/network_service.cc:182
    #12 0x55a24f14f973 in network::NetworkService::CreateForTesting() services/network/network_service.cc:261:11
    #13 0x55a24ab6c77d in gcm::ConnectionFactoryImplTest::ConnectionFactoryImplTest() google_apis/gcm/engine/connection_factory_impl_unittest.cc:323:24
    #14 0x55a24ab8a60e in ConnectionFactoryImplTest_ConnectSuccess_Test google_apis/gcm/engine/connection_factory_impl_unittest.cc:368:1
    #15 0x55a24ab8a60e in testing::internal::TestFactoryImpl<gcm::ConnectionFactoryImplTest_ConnectSuccess_Test>::CreateTest() third_party/googletest/src/googletest/include/gtest/internal/gtest-internal.h:472
    #16 0x55a24b57ab1a in HandleExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test *> third_party/googletest/src/googletest/src/gtest.cc
    #17 0x55a24b57ab1a in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2693
    #18 0x55a24b57c2a6 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2825:28
    #19 0x55a24b5a4f66 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:5227:43
    #20 0x55a24b5a42c2 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc
    #21 0x55a24b5a42c2 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4835
    #22 0x55a24e4e59a7 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2369:46
    #23 0x55a24e4e59a7 in base::TestSuite::Run() base/test/test_suite.cc:294
    #24 0x55a24e4ef864 in Run base/callback.h:99:12
    #25 0x55a24e4ef864 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) base/test/launcher/unit_test_launcher.cc:225
    #26 0x55a24e4ef30d in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) base/test/launcher/unit_test_launcher.cc:575:10
    #27 0x55a24acabc22 in main google_apis/run_all_unittests.cc:13:10
    #28 0x7f86adb02f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
Cc: rmcelrath@chromium.org
Status: Available (was: Untriaged)
Given the above stack, I suspect rmcelrath@'s NetworkChangeNotifier change
r609498 could be the cause, but the appearances of flaky failures do not match
the following timeline of the said patch, so I may be wrong.

Reland r609498: Mon Nov 19 23:21:59 2018 +0000
Revert r608491: Thu Nov 15 20:23:54 2018 +0000
Reland r608265: Thu Nov 15 05:10:03 2018 +0000
Revert r607519: Tue Nov 13 08:28:14 2018 +0000

Cc: peter@chromium.org zea@chromium.org dim...@chromium.org
+Cloud Messaging owners
I don't have time to verify this right now (I will tomorrow if it's not fixed by then), but I'm guessing this was caused by another one of my CLs, crrev.com/c/1336066. I think adding a scoped_task_environment_.RunUntilIdle() to the test's destructor would fix it, or we could do what I did on crrev.com/c/1340913. I think the latter is cleaner than adding a seemingly random RunUntilIdle.
I think ideally we'd have network::NetworkService::CreateForTesting() create a mock NetworkChangeNotifier for us, but that could potentially break a lot of existing tests, so we'd probably want to do that after fixing this flake.
Owner: yutak@chromium.org
Status: Started (was: Available)
OK, let me try to do what's suggested in comment 25.
FTR, the test fixtures that are failing:

GCMConnectionHandlerImplTest
ConnectionFactoryImplTest
GCMSocketStreamTest
CheckinRequestTest
AccountMappingTest

The last two seem innocent; they are not included in the leak reports but
the test harness somehow thinks they have failed. The tests are run in a batch,
so the innocent ones might have been included.
Speculative fix in CQ (going through the try runs because this is a real change):
https://chromium-review.googlesource.com/c/chromium/src/+/1343592
Project Member

Comment 31 by bugdroid1@chromium.org, Nov 20

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

commit b6f5756e40f985d549bd147ad9c202fb2c539ad5
Author: Yuta Kitamura <yutak@chromium.org>
Date: Tue Nov 20 08:49:05 2018

Speculative fix for flaky leaks in gcm_unit_tests.

This CL tries to fix the test flakiness in gcm_unit_tests by using
the mock NetworkChangeNotifier in tests, which avoids receiving real
network status changes.

Bug:  906769 
Change-Id: I08227a6f303328fe2f44cecc84c0ea64bd145415
Tbr: rmcelrath@chromium.org
Tbr: dimich@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1343592
Reviewed-by: Yuta Kitamura <yutak@chromium.org>
Commit-Queue: Yuta Kitamura <yutak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609632}
[modify] https://crrev.com/b6f5756e40f985d549bd147ad9c202fb2c539ad5/google_apis/gcm/base/socket_stream_unittest.cc
[modify] https://crrev.com/b6f5756e40f985d549bd147ad9c202fb2c539ad5/google_apis/gcm/engine/connection_factory_impl_unittest.cc
[modify] https://crrev.com/b6f5756e40f985d549bd147ad9c202fb2c539ad5/google_apis/gcm/engine/connection_handler_impl_unittest.cc

Labels: -Sheriff-Chromium
It looks an owner is assigned. Let me remove Sheriff-Chromium label.
Status: Fixed (was: Started)
The last occurrence of this was at 2018-11-20 07:07:22 UTC, which is about 46
hours ago. Seems like the fix is working! Thanks rmcelrath for your suggestion.

Sign in to add a comment