VDA unittest should destroy GLRenderingVDAClient in the correct thread |
||||||||
Issue descriptionVersion: elm 8799.0.0 with chrome 17fe3c43c3dcfa4c81486e63548a3f985402f68f and fix of http://crbug.com/654429 . What steps will reproduce the problem? (1) Run VDA unittest in DUT. ./video_decode_accelerator_unittest --test_video_data='test-25fps.vp8:320:240:250:258:35:150:11' --gtest_filter=Thumbnail/VideoDecodeAcceleratorParamTest.TestSimpleDecode/0 What do you see instead? [----------] 1 test from Thumbnail/VideoDecodeAcceleratorParamTest [ RUN ] Thumbnail/VideoDecodeAcceleratorParamTest.TestSimpleDecode/0 [1011/112603:WARNING:screen_manager.cc(101)] Display controller (crtc=30) already present. ../../media/gpu/video_decode_accelerator_unittest.cc:1426: Failure Value of: video_file->num_frames Actual: 250 Expected: client->num_decoded_frames() Which is: 245 [1011/112604:INFO:video_decode_accelerator_unittest.cc(1434)] Decoder 0 fps: 225.795 [1011/112605:ERROR:video_decode_accelerator_unittest.cc(1471)] Unknown thumbnails MD5: a0c8198e7b924fde1780dbb5acb40c1f ../../media/gpu/video_decode_accelerator_unittest.cc:1480: Failure Expected: (match) != (golden_md5s.end()), actual: 4-byte object <48-64 61-F9> vs 4-byte object <48-64 61-F9> [1011/112605:FATAL:weak_ptr.cc(20)] Check failed: sequence_checker_.CalledOnValidSequence() || HasOneRef(). WeakPtrs must be invalidated on the same sequenced thread. #0 0x0000f60ff18e base::debug::StackTrace::StackTrace() #1 0x0000f60338c6 logging::LogMessage::~LogMessage() #2 0x0000f60380e2 base::internal::WeakReference::Flag::Invalidate() #3 0x0000f60383e4 base::internal::WeakReferenceOwner::Invalidate() #4 0x0000f603833c base::internal::WeakReferenceOwner::~WeakReferenceOwner() #5 0x0000f5d2573c base::WeakPtrFactory<>::~WeakPtrFactory() #6 0x0000f5d1d440 media::(anonymous namespace)::GLRenderingVDAClient::~GLRenderingVDAClient() #7 0x0000f5d1d51c media::(anonymous namespace)::GLRenderingVDAClient::~GLRenderingVDAClient() #8 0x0000f5d2990a std::default_delete<>::operator()() #9 0x0000f5d26f84 std::unique_ptr<>::~unique_ptr() #10 0x0000f5d2f6d0 std::_Destroy<>() #11 0x0000f5d2dd48 std::_Destroy_aux<>::__destroy<>() #12 0x0000f5d2b918 std::_Destroy<>() #13 0x0000f5d2947a std::_Destroy<>() #14 0x0000f5d26b12 std::vector<>::~vector() #15 0x0000f5d23634 media::(anonymous namespace)::VideoDecodeAcceleratorParamTest_TestSimpleDecode_Test::TestBody() #16 0x0000f6171738 testing::internal::HandleSehExceptionsInMethodIfSupported<>() #17 0x0000f616f37e testing::internal::HandleExceptionsInMethodIfSupported<>() #18 0x0000f6163c5a testing::Test::Run() #19 0x0000f616426e testing::TestInfo::Run() #20 0x0000f616476a testing::TestCase::Run() #21 0x0000f6169526 testing::internal::UnitTestImpl::RunAllTests() #22 0x0000f6171780 testing::internal::HandleSehExceptionsInMethodIfSupported<>() #23 0x0000f616f3ea testing::internal::HandleExceptionsInMethodIfSupported<>() #24 0x0000f61687a6 testing::UnitTest::Run() #25 0x0000f5d353ea RUN_ALL_TESTS() #26 0x0000f5d252f6 main #27 0x0000f56c6308 __libc_start_main The test body has many ASSERT_xx. When it fails, the test aborts and doesn't delete GLRenderingVDAClient in GetRenderingTaskRunner. The debug version of VDA unittest will crash.
,
Nov 14 2016
Load balancing.
,
Nov 21 2016
Haven't looked into it yet, will find some time to do.
,
Nov 28 2016
It's easy to reproduce on Elm. Just have some thoughts about it and need to try them on.
,
Dec 5 2016
Not much progress here.
,
Dec 23 2016
I uploaded a version of modification for this issue: https://codereview.chromium.org/2596193005/ Here are my thoughts for these changes: 1. For thumbnail test, I think it is not necessary to use ASSERT_xx for missing md5 file, or invalid entry in md5 file (in ReadGoldenThumbnailMD5s()), or md5 isn't matched. So I change them to EXPECT_xx, LOG(ERROR), even LOG(WARNING). 2. There are many ASSERT state checks in decoder running for-loop. I use a boolean |terminate_early| instead to indicate state check error, which will bypass other checks and break for-loop if true, and then delete clients and terminate test gracefully.
,
Jan 10 2017
Please see the latest change in this CL: https://codereview.chromium.org/2596193005/ After we provide the TearDown resolution to prevent Assertion crash from test body. We found that the deletion of GLRenderingVDAClient might hit LOG_ASSERT() with particular situation: https://codereview.chromium.org/2596193005/diff/60001/media/gpu/video_decode_accelerator_unittest.cc#newcode892
,
Jan 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2ea32a808df771b5c5acb53a3819010cf054e021 commit 2ea32a808df771b5c5acb53a3819010cf054e021 Author: johnylin <johnylin@chromium.org> Date: Wed Jan 11 10:06:29 2017 vda unittest: delete GLRenderingVDAClient in correct thread In VideoDecodeAcceleratorParamTest.TestSimpleDecode there are many ASSERT_xx while testing, which aborts test and doesn't delete GLRenderingVDAClient in GetRenderingTaskRunner. 1. For thumbnail test, in fact it doesn't need ASSERT for MD5 missing, wrong format, or comparison failure since it won't break any other things. Change to EXPECT_xx and LOG(ERROR). 2. Delete GLRenderingVDAClient in VideoDecodeAcceleratorParamTest::TearDown(). When ASSERT happens it aborts test immediately and call TearDown(), so we can make sure GLRenderingVDAClient is deleted. BUG= 654677 TEST=test debug version of video_decode_accelerator_unittest on Elm 1. For thumbnail test, try to not push md5 file and the test will fail and end gracefully. Add an invalid entry in md5 and the test will pass with error messages. 2. Try to make assertion in any place inside TEST_P. Test will fail and end gracefully. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2596193005 Cr-Commit-Position: refs/heads/master@{#442835} [modify] https://crrev.com/2ea32a808df771b5c5acb53a3819010cf054e021/media/gpu/video_decode_accelerator_unittest.cc
,
Jan 16 2017
,
Mar 4 2017
,
Apr 17 2017
,
May 30 2017
,
Aug 1 2017
,
Oct 14 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by wuchengli@chromium.org
, Oct 26 2016