New issue
Advanced search Search tips

Issue 832882 link

Starred by 1 user
Status: Fixed
Owner:
gab@chromium.org
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Real/RunLoopTest.DisallowRunningForTesting/0 and Mock/RunLoopTest.DisallowRunningForTesting/0 tests can spin-hang

Project Member Reported by w...@chromium.org, Apr 13 2018 
Running base_unittests under Linux/x64 in a Release/Official/DCHECK_always_on build, the test run hung for several minutes, due to two batch sub-process being stuck spinning.

Attaching GDB to one of the processes showed it at the Yield in TestOverridingDelegate::Run():
https://cs.chromium.org/chromium/src/base/run_loop_unittest.cc?l=254

#0  0x00007ff12733ec37 in sched_yield () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x0000000000911634 in base::(anonymous namespace)::TestOverridingDelegate::Run (
    this=0x18adbfe0600, application_tasks_allowed=true) at ../../base/run_loop_unittest.cc:254
#2  0x000000000104767e in base::RunLoop::Run (this=0x18adbfa1ec8) at ../../base/run_loop.cc:130
#3  0x000000000090eed8 in base::RunLoopTest_DisallowRunningForTesting_Test::TestBody (
    this=<optimized out>) at ../../base/run_loop_unittest.cc:641
#4  0x0000000000fd1513 in Run (this=<optimized out>)
    at ../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:920
#5  Run (this=<optimized out>) at ../../third_party/googletest/src/googletest/src/gtest.cc:2667
#6  Run (this=<optimized out>) at ../../third_party/googletest/src/googletest/src/gtest.cc:2785
#7  RunAllTests (this=<optimized out>) at ../../third_party/googletest/src/googletest/src/gtest.cc:5047
#8  0x0000000000fd1513 in testing::UnitTest::Run (this=0x18adbfce780)
#9  0x00000000010d6506 in RUN_ALL_TESTS ()
    at ../../third_party/googletest/src/googletest/include/gtest/gtest.h:2327
#10 base::TestSuite::Run (this=<optimized out>) at ../../base/test/test_suite.cc:275
#11 0x00000000010e154b in Run (this=0x0) at ../../base/callback.h:95
#12 LaunchUnitTestsInternal (run_test_suite=..., parallel_jobs=<optimized out>, 
    default_batch_limit=10, use_job_objects=255, gtest_init=...)
    at ../../base/test/launcher/unit_test_launcher.cc:225
#13 base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) (argc=4, argv=0x7ffe63cd8108, 
    run_test_suite=...) at ../../base/test/launcher/unit_test_launcher.cc:576
#14 0x00000000010cc7de in main (argc=4, argv=0x7ffe63cd8108)
    at ../../base/test/run_all_base_unittests.cc:12

suggesting that it was stuck spinning there, waiting for |should_quit_| to be set.

Comment 1 by gab@chromium.org, May 18 2018

Cc: gab@chromium.org
Components: Test
Owner: w...@chromium.org
Status: Untriaged (was: Assigned)
Hmmm, that's very weird.... Indeed this test is using Run() instead of RunUntilIdle() but it's a death test and it should die on DCHECK(delegate_->allow_running_for_testing_) in RunLoop::BeforeRun() anyways. So somehow the death wasn't caught and the code kept running..?

I can make this use RunUntilIdle() to sooth this specific case but IMO there's a broader problem if DCHECK-death-tests can sometimes flaky run by their DCHECKs... Back to you as the DCHECK expert.

Comment 2 by gab@chromium.org, May 18 2018

Summary: Death tests can flakily keep running after the expected death (causing RunLoopTest.DisallowRunningForTesting to hang) (was: Real/RunLoopTest.DisallowRunningForTesting/0 and Mock/RunLoopTest.DisallowRunningForTesting/0 tests can spin-hang)

Comment 3 by w...@chromium.org, May 19 2018

Status: Assigned (was: Untriaged)
Summary: Real/RunLoopTest.DisallowRunningForTesting/0 and Mock/RunLoopTest.DisallowRunningForTesting/0 tests can spin-hang (was: Death tests can flakily keep running after the expected death (causing RunLoopTest.DisallowRunningForTesting to hang))
Reverting the bug title, since we have only ever observed this issue with these specific tests, suggesting that it is not a generic flakiness in death-tests.

Re Run() vs RunUntilIdle(): Using RunUntilIdle() is preferable, if it will exercise the code-under-test as intended, since that will result in a timely EXPECT_DEATH() failure when the child process exits without crashing, rather than hanging.

It's curious that this occurs in Release+Official+dcheck_always_on, which is an unusual configuration in which CHECK() is an immediate crash and DCHECK() is LogMessage(FATAL).
Project Member

Comment 4 by bugdroid1@chromium.org, May 22 2018 

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

commit f3fd8f08252427e9d4f48a4316792bdfa0eb98de
Author: Gabriel Charette <gab@chromium.org>
Date: Tue May 22 15:31:48 2018

[Flakiness] Fix RunLoopTest death-tests to fail without hanging.

This test is exercising a DCHECK in RunLoop::BeforeRun() which is
exercised the same way by RunUntilIdle(). RunUntileIdle() has the
benefit of not hanging the test on failure.

R=danakj@chromium.org, wez@chromium.org

Bug:  832882 
Change-Id: I905e7f3940130b5648dcb16eab4b55a17d811169
Reviewed-on: https://chromium-review.googlesource.com/1066401
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560602}
[modify] https://crrev.com/f3fd8f08252427e9d4f48a4316792bdfa0eb98de/base/run_loop_unittest.cc

Comment 5 by gab@chromium.org, May 22 2018

Cc: -gab@chromium.org
Owner: gab@chromium.org
Status: Fixed (was: Assigned)

Comment 6 by w...@chromium.org, May 22 2018 

*facepalm*

OK, the failure was actually specific to |dcheck_is_configurable=true|, where LOG_DCHECK defaults to LOG_INFO but can be switched at run-time to LOG_FATAL - there is a special hack to force it to LOG_FATAL in death-test sub-processes, but that hack only applies on platforms where the sub-process is launched with a special command-line, not when it is fork()ed.

Comment 7 by gab@chromium.org, May 23 2018 

Ah Ha ;)!
►

Sign in to add a comment