New issue
Advanced search Search tips

Issue 880608 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification



Sign in to add a comment

NetworkServiceTestWithService.RawRequestHeadersAbsent is flaky

Project Member Reported by mmenke@chromium.org, Sep 4

Issue description

If you look at a run on the cast bots on the tree, you'll often see this test fail with:

[ RUN      ] NetworkServiceTestWithService.RawRequestHeadersAbsent
Received signal 11 SEGV_MAPERR 000000000018
#0 0x00000250195c base::debug::StackTrace::StackTrace()
#1 0x0000025014d1 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7f6dfdbee330 <unknown>
#3 0x7f6dfdbe8404 __GI___pthread_mutex_lock
#4 0x000002509f68 base::internal::LockImpl::Lock()
#5 0x000002aa36e0 net::NetLog::RemoveObserver()
#6 0x000002db1a2a net::FileNetLogObserver::StopObserving()
#7 0x000002dae8f2 network::MojoNetLog::ShutDown()
#8 0x000002dc604d network::NetworkService::~NetworkService()
#9 0x000002dc639e network::NetworkService::~NetworkService()
#10 0x00000251d7fd service_manager::ServiceContext::~ServiceContext()
#11 0x00000251d82e service_manager::ServiceContext::~ServiceContext()
#12 0x000001088e46 network::(anonymous namespace)::ServiceTestClient::~ServiceTestClient()
#13 0x000001080a7e network::(anonymous namespace)::ServiceTestClient::~ServiceTestClient()
#14 0x00000251d7fd service_manager::ServiceContext::~ServiceContext()
#15 0x00000251d82e service_manager::ServiceContext::~ServiceContext()

It's unclear what's going on here - the MojoNetLog is a singleton, and it's destroying its FileNetLogObserver, which calls back into itself...and grabs the lock the MojoNetLog owns, so everything in the stack should be valid.  I suppose a double Shutdown call could possibly cause an issue.
 
Also, the NetLog is now a global, so it should definitely be non-null.
Can you link to one of the bots?
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8936278947589171664/+/steps/services_unittests/0/stdout / https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Cast%20Linux/58618.

Can find more as needed by poking at the linux -> cast -> vid bots on the main builder (Unclear of failure rate, but currently I believe there are two failures on the most recent 5 runs).  It passes enough not to turn the bots red.
I presume this is a regression from https://chromium-review.googlesource.com/c/chromium/src/+/1195549, and that we are calling MojoNetLog::ShutDown() on the singleton twice.

The second call to StopObserving() would try to remove an observer that no longer exists.
Owner: eroman@chromium.org
Status: Assigned (was: Untriaged)
I think we can just null the that out on shutdown, looking.
Yea, that would be my guess as well, but we only call Shutdown() in one place, and the test only creates one NetworkService...I guess it could be that multiple tests are running without restarting the binary?  If that's the case, just deleting the observer should be enough, I assume - haven't really dug into it.
FWIW I confirmed that ShutDown() gets called twice.

When running all the tests, NetworkServiceTestWithService.StartsNetLog calls StopObserving the first time, and then RawRequestHeadersAbsent calls it the second time.
Minimal repro is:

services_unittests --gtest_filter="NetworkServiceTestWithService.StartsNetLog:NetworkServiceTestWithService.RawRequestHeadersAbsent
Status: Fixed (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 5

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

commit 3e9bb8a4b5d3a863aafd847a51394884a950d518
Author: Eric Roman <eroman@chromium.org>
Date: Wed Sep 05 15:58:37 2018

Don't crash if MojoNetLog::ShutDown() is called twice.

Bug:  880608 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I065803782935f7e69310a791ecb3053d573fcc61
Reviewed-on: https://chromium-review.googlesource.com/1205534
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588881}
[modify] https://crrev.com/3e9bb8a4b5d3a863aafd847a51394884a950d518/services/network/mojo_net_log.cc
[modify] https://crrev.com/3e9bb8a4b5d3a863aafd847a51394884a950d518/services/network/mojo_net_log.h

Sign in to add a comment