New issue
Advanced search Search tips

Issue 856317 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

FATAL:base_audio_context.cc(108)] Check failed: !active_source_nodes_.size().

Project Member Reported by dalecur...@chromium.org, Jun 25 2018

Issue description

STDERR: [1:1:0625/130031.437458:FATAL:base_audio_context.cc(108)] Check failed: !active_source_nodes_.size(). 
STDERR: #0 0x7f50a31acfec base::debug::StackTrace::StackTrace()
STDERR: #1 0x7f50a30f61cb logging::LogMessage::~LogMessage()
STDERR: #2 0x7f509ac42420 blink::BaseAudioContext::~BaseAudioContext()
STDERR: #3 0x7f509b48840c blink::NormalPage::Sweep()
STDERR: #4 0x7f509b486e1a blink::NormalPageArena::LazySweepPages()
STDERR: #5 0x7f509b4832a6 blink::BaseArena::LazySweep()
STDERR: #6 0x7f509b487331 blink::NormalPageArena::OutOfLineAllocate()
STDERR: #7 0x7f509a64a312 blink::NormalPageArena::AllocateObject()
STDERR: #8 0x7f509a64a010 blink::ThreadHeap::AllocateOnArenaIndex()
STDERR: #9 0x7f509ac5daa6 blink::OfflineAudioCompletionEvent::Create()
STDERR: #10 0x7f509ac60b23 blink::OfflineAudioContext::FireCompletionEvent()
STDERR: #11 0x7f509ac662f6 blink::OfflineAudioDestinationHandler::NotifyComplete()
STDERR: #12 0x7f509ac2f94b _ZN4base8internal7InvokerINS0_9BindStateIMN5blink27AudioScheduledSourceHandlerEFvvEJ13scoped_refptrIS4_EEEEFvvEE3RunEPNS0_13BindStateBaseE
STDERR: #13 0x7f509b45614c blink::(anonymous namespace)::RunCrossThreadClosure()
STDERR: #14 0x7f509b456bf3 _ZN4base8internal7InvokerINS0_9BindStateIPFvN3WTF19CrossThreadFunctionIFvvEEEEJS6_EEES5_E7RunOnceEPNS0_13BindStateBaseE
STDERR: #15 0x7f50a30d6dfd base::debug::TaskAnnotator::RunTask()
STDERR: #16 0x7f509b51b6ad base::sequence_manager::internal::ThreadControllerImpl::DoWork()
STDERR: #17 0x7f509b51d668 _ZN4base8internal7InvokerINS0_9BindStateIMNS_16sequence_manager8internal20ThreadControllerImplEFvNS5_8WorkTypeEEJNS_7WeakPtrIS5_EES6_EEEFvvEE3RunEPNS0_13BindStateBaseE
STDERR: #18 0x7f50a30d6dfd base::debug::TaskAnnotator::RunTask()
STDERR: #19 0x7f50a3102776 base::internal::IncomingTaskQueue::RunTask()
STDERR: #20 0x7f50a3106747 base::MessageLoop::RunTask()
STDERR: #21 0x7f50a3106b5a base::MessageLoop::DeferOrRunPendingTask()
STDERR: #22 0x7f50a3106dee base::MessageLoop::DoWork()
STDERR: #23 0x7f50a3109146 base::MessagePumpDefault::Run()
STDERR: #24 0x7f50a3106071 base::MessageLoop::Run()
STDERR: #25 0x7f50a313a346 base::RunLoop::Run()
STDERR: #26 0x7f50a27dbe65 content::RendererMain()
STDERR: #27 0x7f50a28b7f7e content::RunZygote()
STDERR: #28 0x7f50a28b9430 content::ContentMainRunnerImpl::Run()
STDERR: #29 0x7f509ee6080f service_manager::Main()
STDERR: #30 0x7f50a28b7484 content::ContentMain()
STDERR: #31 0x0000002f314b main
STDERR: #32 0x7f5098a792b1 __libc_start_main
STDERR: #33 0x0000002f302a _start
STDERR: 

Hit this while running the webaudio/Analyser/realtimeanalyser-float-data.html layout test in release mode with dcheck's enabled.
 

Comment 1 by rtoy@chromium.org, Jun 25 2018

Status: Available (was: Untriaged)
Yeah, I've seen this.  It's flaky so hard to figure out why things are getting removed.
It's not that flaky. With --repeat-each=50 it's crashing 1 out of every 5 runs so shouldn't be too hard to debug.
I have a repeatable test case in https://chromium-review.googlesource.com/c/chromium/src/+/1212270

Do a release build with dcheck_always_on = true, run chrome, and open dev console.  Then open sub-sample-scheduling.html.  Wait for 10-15 sec.  And maybe scroll the window a bit and move the mouse around.  It crashes for me very reliably.

This is a layout test that only uses an offline context.  I wonder if, when we deliver the completion event for this, we should also call PerformCleanupOnMainThread to clean things up on the offline context.  If we don't I think things are left dangling.

hongchan@ WDYT?
Oh, and we I change the completion event to call PerformCleanupOnMainThread, I can't get the assert any more. (Not conclusive, but certainly better than without.)
Cc: haraken@chromium.org
Components: Blink>MemoryAllocator>GarbageCollection
I already can see the problem; the crash is coming from blink::BaseAudioContext::~BaseAudioContext().

|active_source_nodes_| is on Oilpan heap and should not be touched within the destructor. I think simply removing the check will fix the crash. It's not supposed to be there in the first place.

With that said, I agree with #3 on taking care of the dangling objects. Perhaps BaseAudioContext needs to have a pre-finalizer so we can do the clean up before the destructor?

Because this is related to Oilpan and GC, I am asking opinions from haraken@.

Yes, the dcheck is invalid.  If you add a printf to print out the size, you can't even compile anymore.  So that needs to be removed, but I think the actual issue is that we don't do the cleanup when offline context is finished so things are left kind of unfinished.
If you really need to touch other on-heap objects, you can add EARGERLY_FINALIZED() to BaseAudioContext.

Stepping back... I was assuming that Uninitialize() is called before BaseAudioContext gets destructed. Then we can clean up things in Uninitialize()...?

Owner: hongchan@chromium.org
Status: Started (was: Available)
Yes, that was my guess. Simply moving DCHECKs to the end of Uninitialize() would fix this issue.

The missing clean-up in OAC should be addressed in a separate CL, though. Right?
My interpretation is the missing call to PerformCleanup is the cause of this issue.  Moving the DCHECKs is a different issue. But I can go with either.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 25

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

commit e516f97c983f81b79e3baaf09b00db13c66c6452
Author: Hongchan Choi <hongchan@chromium.org>
Date: Tue Sep 25 19:18:20 2018

Do PerformCleanupOnMainThread() when OfflineAudioContext rendering ends

Currently OfflineAudioContext does not do the clean up when rendering
is finished. This CL adds the clean up process when `oncomplete` event
is fired.

with --repeat-each=50 option.

Bug:  856317 
Test: webaudio/Analyser/realtimeanalyser-float-data.html passes
Change-Id: Ia0556f666bc49ddb92666beab4171cfcf5c385ba
Reviewed-on: https://chromium-review.googlesource.com/1237384
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594049}
[modify] https://crrev.com/e516f97c983f81b79e3baaf09b00db13c66c6452/third_party/blink/renderer/modules/webaudio/base_audio_context.h
[modify] https://crrev.com/e516f97c983f81b79e3baaf09b00db13c66c6452/third_party/blink/renderer/modules/webaudio/offline_audio_context.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 25

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

commit e5087536adb709a5bbb63b1f619a2dc77cf95f30
Author: Hongchan Choi <hongchan@chromium.org>
Date: Tue Sep 25 23:28:48 2018

Move DCHECKs from BaseAudioContext destructor to Uinitialize()

Currently the destructor of BaseAudioContext has 3 DCHECKs touching
GC-managed objects. This is a violation of GC system. Move them from
the destructor to Uinitialize() method, which gets called before the
destruction.

with --repeat-each=50 option.

Bug:  856317 
Test: webaudio/Analyser/realtimeanalyser-float-data.html passes
Change-Id: Ifa898d95feda3fcc1ae82e3e99a27e45edca38ab
Reviewed-on: https://chromium-review.googlesource.com/1237247
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594143}
[modify] https://crrev.com/e5087536adb709a5bbb63b1f619a2dc77cf95f30/third_party/blink/renderer/modules/webaudio/base_audio_context.cc

Status: Fixed (was: Started)

Sign in to add a comment