New issue
Advanced search Search tips

Issue 789160 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 717265



Sign in to add a comment

VP9Picture is created on a background thread but destroyed on another

Project Member Reported by mcasas@chromium.org, Nov 28 2017

Issue description

VP9Picture is a RefCounted non thread safe class, created on the
decoder background thread [1] but at least one of the releases
comes from the GPU main thread -- this causes my GPU process to
crash when closing a playback on a Release build with 
  dcheck_always_on=true:

[30592:30592:1128/103522.095870:FATAL:ref_counted.h(83)] Check failed: CalledOnValidSequence(). 
#0 0x5bf18e7e334c base::debug::StackTrace::StackTrace()
#1 0x5bf18e802043 logging::LogMessage::~LogMessage()
#2 0x5bf18bdd77fc base::subtle::RefCountedBase::Release()
#3 0x5bf192a9875d media::VP9Decoder::~VP9Decoder()
#4 0x5bf192a987be media::VP9Decoder::~VP9Decoder()
#5 0x5bf192a7f410 media::VaapiVideoDecodeAccelerator::~VaapiVideoDecodeAccelerator()
#6 0x5bf192a7f6ee media::VaapiVideoDecodeAccelerator::~VaapiVideoDecodeAccelerator()
#7 0x5bf192a85a7c media::VaapiVideoDecodeAccelerator::Destroy()
#8 0x5bf18ff5b5a0 media::GpuVideoDecodeAccelerator::OnWillDestroyStub()
#9 0x5bf18ff5a7e1 media::GpuVideoDecodeAccelerator::OnDestroy()
#10 0x5bf18ff5a666 _ZN3IPC8MessageTI39AcceleratedVideoDecoderMsg_Destroy_MetaNSt3__15tupleIJEEEvE8DispatchIN5media25GpuVideoDecodeAcceleratorES8_vMS8_FvvEEEbPKNS_7MessageEPT_PT0_PT1_T2_
#11 0x5bf18ff58c45 media::GpuVideoDecodeAccelerator::OnMessageReceived()
#12 0x5bf1928274ef IPC::MessageRouter::RouteMessage()
#13 0x5bf192a169ae gpu::GpuChannel::HandleMessageHelper()
#14 0x5bf192a144f7 gpu::GpuChannel::HandleMessage()
#15 0x5bf18be149ed _ZN4base8internal7InvokerINS0_9BindStateIMN11google_apis13RequestSenderEFvRKNS_7WeakPtrINS3_29AuthenticatedRequestInterfaceEEEEJNS5_IS4_EES7_EEEFvvEE3RunEPNS0_13BindStateBaseE
#16 0x5bf192916031 gpu::Scheduler::RunNextTask()
#17 0x5bf18be0c204 _ZN4base8internal7InvokerINS0_9BindStateIMN11google_apis19UrlFetchRequestBaseEFvvEJNS_7WeakPtrINS3_5drive30SingleBatchableDelegateRequestEEEEEEFvvEE3RunEPNS0_13BindStateBaseE
#18 0x5bf18e7e3ba8 base::debug::TaskAnnotator::RunTask()
#19 0x5bf18e8aece6 base::internal::IncomingTaskQueue::RunTask()
#20 0x5bf18e80ad5d base::MessageLoop::RunTask()
#21 0x5bf18e80b16b base::MessageLoop::DeferOrRunPendingTask()
#22 0x5bf18e80b420 base::MessageLoop::DoWork()
#23 0x5bf18e80c7d0 base::MessagePumpDefault::Run()
#24 0x5bf18e80a5ac base::MessageLoop::Run()
#25 0x5bf18e838c85 base::RunLoop::Run()
#26 0x5bf191fbb46a content::GpuMain()
#27 0x5bf18e3df034 content::RunNamedProcessTypeMain()
#28 0x5bf18e3dfb47 content::ContentMainRunnerImpl::Run()
#29 0x5bf18e3ec149 service_manager::Main()
#30 0x5bf18e3de3c1 content::ContentMain()
#31 0x5bf18bd4e4e8 ChromeMain
#32 0x7aa875fcd736 __libc_start_main
#33 0x5bf18bd4e2c9 _start

Landed in [2].

[1] https://cs.chromium.org/chromium/src/media/gpu/vaapi_video_decode_accelerator.cc?type=cs&q=CreateVP9Picture&sq=package:chromium&l=1702
[2] https://chromium.googlesource.com/chromium/src/+/d94b2b080b381dfe4fbb1f8d53faded18d59f400

 

Comment 1 by mcasas@chromium.org, Nov 28 2017

Cc: mcasas@chromium.org

Comment 2 by mcasas@chromium.org, Nov 28 2017

Owner: mcasas@chromium.org
Status: Started (was: Available)

Comment 3 by mcasas@chromium.org, Nov 28 2017

Blocking: 778093

Comment 4 by mcasas@chromium.org, Nov 28 2017

Blocking: 717265

Comment 5 by mcasas@chromium.org, Dec 11 2017

Summary: VP9Picture is created on a background thread but destroyed on another (was: VP9Picture is created on a background thread but destroyed in other)
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 12 2017

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

commit 70340ce072cee8a0bdcddb5f312d32567b2269f6
Author: Miguel Casas <mcasas@chromium.org>
Date: Tue Dec 12 08:33:43 2017

vaapi vda: Delete owned objects on worker thread in Cleanup()

This CL adds a SEQUENCE_CHECKER to Vaapi*Accelerator classes, and
posts the destruction of those objects to the appropriate thread on
Cleanup().

Also makes {H264,VP8,VP9}Picture RefCountedThreadSafe, see miu@
comment in
https://chromium-review.googlesource.com/c/chromium/src/+/794091#message-a64bed985cfaf8a19499a517bb110a7ce581dc0f

TEST=play back VP9/VP8/H264 w/ simplechrome on soraka, Release build
unstripped, let video play for a few seconds then navigate back; no
crashes. Unittests as before:
video_decode_accelerator_unittest --test_video_data=test-25fps.vp9:320:240:250:250:35:150:12
video_decode_accelerator_unittest --test_video_data=test-25fps.vp8:320:240:250:250:35:150:11
video_decode_accelerator_unittest --test_video_data=test-25fps.h264:320:240:250:258:35:150:1

Bug: 789160
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: I7d96aaf89c92bf46f00c8b8b36798e057a842ed2
Reviewed-on: https://chromium-review.googlesource.com/794091
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523372}
[modify] https://crrev.com/70340ce072cee8a0bdcddb5f312d32567b2269f6/media/gpu/h264_dpb.h
[modify] https://crrev.com/70340ce072cee8a0bdcddb5f312d32567b2269f6/media/gpu/vaapi/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/70340ce072cee8a0bdcddb5f312d32567b2269f6/media/gpu/vp8_picture.h
[modify] https://crrev.com/70340ce072cee8a0bdcddb5f312d32567b2269f6/media/gpu/vp9_picture.h

Comment 7 by mcasas@chromium.org, Dec 12 2017

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 27 2018

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

commit a237ddcdcb199842f85764970522c69cf526a0fe
Author: Miguel Casas <mcasas@chromium.org>
Date: Sat Jan 27 02:59:16 2018

Vaapi: remove DeleteSoon()s from VaapiVDA

This CL removes the DeleteSoon()s for  |decoder_| and |*_accelerator|s
as a speculative fix for the second bug below crbug.com/799204 that
tracks a crash on the field.

It leaves most of the SEQUENCE_CHECKER usage, since this is useful
as documentation-in-code, and leaves a few TODO()s to figure out,
if DeleteSoon() proves the culprit, why doing the right thing leads
to crashes.

from crosvideo.appspot.com, compiled with dcheck_always_on=true

Bug: 789160,799204
TEST: Tested via simplechrome on Soraka playing back VP9 videos
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: I6dace88a448a08bdf441725701347968326442ac
Reviewed-on: https://chromium-review.googlesource.com/889143
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532141}
[modify] https://crrev.com/a237ddcdcb199842f85764970522c69cf526a0fe/media/gpu/vaapi/vaapi_video_decode_accelerator.cc

Comment 9 by mcasas@chromium.org, Jan 29 2018

Status: Assigned (was: Fixed)
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 9 2018

Labels: merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d7546e130617afc377814b45e58178c5d171e72

commit 5d7546e130617afc377814b45e58178c5d171e72
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Feb 09 22:12:46 2018

Vaapi: remove DeleteSoon()s from VaapiVDA

This CL removes the DeleteSoon()s for  |decoder_| and |*_accelerator|s
as a speculative fix for the second bug below crbug.com/799204 that
tracks a crash on the field.

It leaves most of the SEQUENCE_CHECKER usage, since this is useful
as documentation-in-code, and leaves a few TODO()s to figure out,
if DeleteSoon() proves the culprit, why doing the right thing leads
to crashes.

from crosvideo.appspot.com, compiled with dcheck_always_on=true

Bug: 789160,799204
TEST: Tested via simplechrome on Soraka playing back VP9 videos
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: I6dace88a448a08bdf441725701347968326442ac
Reviewed-on: https://chromium-review.googlesource.com/889143
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#532141}(cherry picked from commit a237ddcdcb199842f85764970522c69cf526a0fe)
Reviewed-on: https://chromium-review.googlesource.com/912111
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#416}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/5d7546e130617afc377814b45e58178c5d171e72/media/gpu/vaapi/vaapi_video_decode_accelerator.cc

Blocking: -778093

Sign in to add a comment