New issue
Advanced search Search tips

Issue 759108 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 3
Type: Bug

Blocking:
issue 882906
issue 754861



Sign in to add a comment

TaskScheduler cleanup failure in VideoCaptureImplTest

Project Member Reported by scottmg@chromium.org, Aug 25 2017

Issue description

If, on Fuchsia, I run these two tests:

out/fuch/bin/run_content_unittests --gtest_filter=VideoCaptureImplTest.EndedBeforeStop:PresentationDispatcherTest.TestStartPresentation

PresentationDispatcherTest CHECKs because there's already a TaskScheduler instance set.

VideoCaptureImplTest causes it to be set by below, but doesn't cause it to be cleared on test shutdown.

[00001.618] 03414.03441> [3:1937305221:0825/174943.400442:1617998:ERROR:task_scheduler.cc(75)] TaskScheduler::SetInstance, 0x794f7fd50700
#00: base::debug::StackTrace::StackTrace(unsigned long) at base/debug/stack_trace_fuchsia.cc:173
#01: base::debug::StackTrace::StackTrace() at base/debug/stack_trace.cc:199
#02: base::TaskScheduler::SetInstance(std::__1::unique_ptr<base::TaskScheduler, std::__1::default_delete<base::TaskScheduler> >) at base/task_scheduler/task_scheduler.cc:76
#03: base::TaskScheduler::Create(base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >) at base/task_scheduler/task_scheduler.cc:70
#04: base::TaskScheduler::CreateAndStartWithDefaultParams(base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >) at base/task_scheduler/task_scheduler.cc:60
#05: content::ChildProcess::ChildProcess(base::ThreadPriority, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::unique_ptr<base::TaskScheduler::InitParams, std::__1::default_delete<base::TaskScheduler::InitParams> >) at content/child/child_process.cc:53
#06: content::VideoCaptureImplTest::VideoCaptureImplTest() at content/renderer/media/video_capture_impl_unittest.cc:98
#07: content::VideoCaptureImplTest_EndedBeforeStop_Test::VideoCaptureImplTest_EndedBeforeStop_Test() at content/renderer/media/video_capture_impl_unittest.cc:344
#08: testing::internal::TestFactoryImpl<content::VideoCaptureImplTest_EndedBeforeStop_Test>::CreateTest() at third_party/googletest/src/googletest/include/gtest/internal/gtest-internal.h:484
#09: testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2399
#10: testing::Test* testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2452
#11: testing::TestInfo::Run() at third_party/googletest/src/googletest/src/gtest.cc:2644
#12: testing::TestCase::Run() at third_party/googletest/src/googletest/src/gtest.cc:2770
#13: testing::internal::UnitTestImpl::RunAllTests() at third_party/googletest/src/googletest/src/gtest.cc:4647
#14: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2399
#15: bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) at third_party/googletest/src/googletest/src/gtest.cc:2452
#16: testing::UnitTest::Run() at third_party/googletest/src/googletest/src/gtest.cc:4256
#17: RUN_ALL_TESTS() at third_party/googletest/src/googletest/include/gtest/gtest.h:2237
#18: base::TestSuite::Run() at base/test/test_suite.cc:270
#19: content::UnitTestTestSuite::Run() at content/public/test/unittest_test_suite.cc:45
#20: int base::internal::FunctorTraits<int (content::UnitTestTestSuite::*)(), void>::Invoke<content::UnitTestTestSuite*>(int (content::UnitTestTestSuite::*)(), content::UnitTestTestSuite*&&) at base/bind_internal.h:194
#21: int base::internal::InvokeHelper<false, int>::MakeItSo<int (content::UnitTestTestSuite::* const&)(), content::UnitTestTestSuite*>(int (content::UnitTestTestSuite::* const&)(), content::UnitTestTestSuite*&&) at base/bind_internal.h:263
#22: int base::internal::Invoker<base::internal::BindState<int (content::UnitTestTestSuite::*)(), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int ()>::RunImpl<int (content::UnitTestTestSuite::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > const&, 0ul>(int (content::UnitTestTestSuite::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > const&, std::__1::integer_sequence<unsigned long, 0ul>) at base/bind_internal.h:335
#23: base::internal::Invoker<base::internal::BindState<int (content::UnitTestTestSuite::*)(), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int ()>::Run(base::internal::BindStateBase*) at base/bind_internal.h:317
#24: base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>::Run() const & at base/callback.h:80
#25: base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, unsigned long, int, bool, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) at base/test/launcher/unit_test_launcher.cc:216
#26: base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) at base/test/launcher/unit_test_launcher.cc:475
#27: main at content/test/run_all_unittests.cc:20
#28: pc 0x66fa2961a04e (libc.so,0x1c04e)

 
It seems a bit weird that VideoCaptureImplTest embeds a content::ChildProcess directly.

But I guess ChildProcess ought to clear out the TaskScheduler in the case where it created it.
Cc: robliao@chromium.org gab@chromium.org fdoray@chromium.org
Actually, I don't much like ChildProcess creating a TaskScheduler when it's not in the browser or maybe tests or maybe some other times like Thursdays and the probably cleaning it up (https://cs.chromium.org/chromium/src/content/child/child_process.cc?rcl=5c3b54706eb37989819326c10285040626e22d53&l=43 :/

I can fix VideoCaptureImplTest by putting a scoped environment in its fixture, but is there something we should do to make ChildProcess a bit less prone to error, like forcing it to know if it's being created in browser/child/test?

Comment 3 by w...@chromium.org, Aug 25 2017

Can we have ChildProcess simply require that the caller has set up the
TaskEnvironment? Lots of tests will suddenly fail, but we can fix those,
and then we don't have production code sometimes creating things because it
might be in a test, which would be nice.
Ha! When I fix VideoCaptureImplTest to use a ScopedTaskEnvironment it gets screwed by something before it, and hits the same CHECK that it was previously causing PresentationDispatcherTest to hit. It's flake all the way down.

"Payback, eh VideoCaptureImplTest? I'm going to check you in like that so you learn your lesson."
Ah, it seems it was your partner in crime, one VideoCaptureImplManagerTest.

Messrs. VideoCaptureImplTest & Sons shall be vanquished yet.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 25 2017

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

commit 9e41c6f2269f715b5d2dccb79f91f284df3650ac
Author: Scott Graham <scottmg@chromium.org>
Date: Fri Aug 25 19:05:32 2017

Make VideoCaptureImpl tests create their own TaskScheduler instance

This showed up as flake on Fuchsia when the test after the
VideoCaptureImplTest fixture tried to set a TaskScheduler instance, but
could occur on all platforms.

(Previously they were relying on ChildProcess to do this. We probably
shouldn't do that at all, but that will be addressed in subsequent
changes as that will likely touch a lot more tests.)

Bug:  759108 
Change-Id: Icae3ee04735a6ee04bcd32db190b18edeab1857c
Reviewed-on: https://chromium-review.googlesource.com/636192
Commit-Queue: Scott Graham <scottmg@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497481}
[modify] https://crrev.com/9e41c6f2269f715b5d2dccb79f91f284df3650ac/content/renderer/media/video_capture_impl_manager_unittest.cc
[modify] https://crrev.com/9e41c6f2269f715b5d2dccb79f91f284df3650ac/content/renderer/media/video_capture_impl_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 25 2017

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

commit f7e1576b2b1869dc5fa552dbabb2f93c87d09e37
Author: Scott Graham <scottmg@chromium.org>
Date: Fri Aug 25 21:00:29 2017

fuchsia: Update content_unittests filter for recent changes

- PresentationConnectionProxyTest are fixed by https://chromium-review.googlesource.com/c/chromium/src/+/636192
- AsyncResourceHandlerTest were fixed by the handle limit fix
- disable the AppCacheUpdateJobTest and LegacyInputRouterImplTest as they've been flaking on the bot
- move QuotaPolicyCookieStoreTest down with the other sqlite ones

Bug:  759108 ,  754861 
TBR: kmarshall@chromium.org
Change-Id: I726fe721db0426518bb84a730defa9289222c92e
Reviewed-on: https://chromium-review.googlesource.com/636195
Reviewed-by: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497530}
[modify] https://crrev.com/f7e1576b2b1869dc5fa552dbabb2f93c87d09e37/testing/buildbot/filters/fuchsia.content_unittests.filter

Status: Fixed (was: Started)
Blocking: 882906

Sign in to add a comment