New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 828826 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug-Regression

Blocked on:
issue 829767



Sign in to add a comment

DCHECK failure in DeferredTaskHandler::RemoveMarkedSummingJunction

Reported by andrew.macpherson@soundtrap.com, Apr 4 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36

Steps to reproduce the problem:
1. Go to https://jsfiddle.net/ha46jr3j/36/ in a ToT debug build.
2. Press start and wait up to a minute.
3. See the attached DCHECK failure.

What is the expected behavior?

What went wrong?
In Canary and in a ToT build from Monday (67.0.3387.0) we get no sound from our custom synths in Soundtrap. I bisected the problem down to this commit:

https://chromium.googlesource.com/chromium/src/+/45a0c949c2a456761ffa3011f7058c3d8b7a2d96

In trying to come up with a minimal repro case for the issue I hit this DCHECK failure, which also doesn't appear to occur prior to that commit. I'm not sure if this is related but am filing this in case it is.

If this issue isn't related to the problem with no sound then I will look further into another repro case for that.

[0404/141039.663850:FATAL:DeferredTaskHandler.cpp(101)] Check failed: IsMainThread().
0   libbase.dylib                       0x000000011cfb934e base::debug::StackTrace::StackTrace(unsigned long) + 174
1   libbase.dylib                       0x000000011cfb940d base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x000000011cfb78bc base::debug::StackTrace::StackTrace() + 28
3   libbase.dylib                       0x000000011d0538cc logging::LogMessage::~LogMessage() + 460
4   libbase.dylib                       0x000000011d051635 logging::LogMessage::~LogMessage() + 21
5   libblink_modules.dylib              0x000000014cdd888f blink::DeferredTaskHandler::RemoveMarkedSummingJunction(blink::AudioSummingJunction*) + 191
6   libblink_modules.dylib              0x000000014cd86fa3 blink::AudioSummingJunction::~AudioSummingJunction() + 51
7   libblink_modules.dylib              0x000000014cd5e2ef blink::AudioParamHandler::~AudioParamHandler() + 95
8   libblink_modules.dylib              0x000000014cd5aab5 blink::AudioParamHandler::~AudioParamHandler() + 21
9   libblink_modules.dylib              0x000000014cd5aad9 blink::AudioParamHandler::~AudioParamHandler() + 25
10  libblink_modules.dylib              0x000000014cd25e5b void WTF::ThreadSafeRefCounted<blink::AudioParamHandler, WTF::DefaultThreadSafeRefCountedTraits<blink::AudioParamHandler> >::DeleteInternal<blink::AudioParamHandler>(blink::AudioParamHandler const*) + 43
11  libblink_modules.dylib              0x000000014cd25e25 WTF::DefaultThreadSafeRefCountedTraits<blink::AudioParamHandler>::Destruct(blink::AudioParamHandler const*) + 21
12  libblink_modules.dylib              0x000000014cd25dfc base::RefCountedThreadSafe<blink::AudioParamHandler, WTF::DefaultThreadSafeRefCountedTraits<blink::AudioParamHandler> >::Release() const + 60
13  libblink_modules.dylib              0x000000014cd25db9 scoped_refptr<blink::AudioParamHandler>::Release(blink::AudioParamHandler*) + 25
14  libblink_modules.dylib              0x000000014cd25d9a scoped_refptr<blink::AudioParamHandler>::~scoped_refptr() + 42
15  libblink_modules.dylib              0x000000014cd20e65 scoped_refptr<blink::AudioParamHandler>::~scoped_refptr() + 21
16  libblink_modules.dylib              0x000000014cde9c72 blink::DynamicsCompressorHandler::~DynamicsCompressorHandler() + 66
17  libblink_modules.dylib              0x000000014cde9da5 blink::DynamicsCompressorHandler::~DynamicsCompressorHandler() + 21
18  libblink_modules.dylib              0x000000014cde9dc9 blink::DynamicsCompressorHandler::~DynamicsCompressorHandler() + 25
19  libblink_modules.dylib              0x000000014cd12ecb void WTF::ThreadSafeRefCounted<blink::AudioHandler, WTF::DefaultThreadSafeRefCountedTraits<blink::AudioHandler> >::DeleteInternal<blink::AudioHandler>(blink::AudioHandler const*) + 43
20  libblink_modules.dylib              0x000000014cd12e95 WTF::DefaultThreadSafeRefCountedTraits<blink::AudioHandler>::Destruct(blink::AudioHandler const*) + 21
21  libblink_modules.dylib              0x000000014cd12e6c base::RefCountedThreadSafe<blink::AudioHandler, WTF::DefaultThreadSafeRefCountedTraits<blink::AudioHandler> >::Release() const + 60
22  libblink_modules.dylib              0x000000014cd12e29 scoped_refptr<blink::AudioHandler>::Release(blink::AudioHandler*) + 25
23  libblink_modules.dylib              0x000000014cd12e0a scoped_refptr<blink::AudioHandler>::~scoped_refptr() + 42
24  libblink_modules.dylib              0x000000014cd11a85 scoped_refptr<blink::AudioHandler>::~scoped_refptr() + 21
25  libblink_modules.dylib              0x000000014cdda256 blink::DeferredTaskHandler::UpdateTailProcessingHandlers() + 374
26  libblink_modules.dylib              0x000000014cddb5d6 blink::DeferredTaskHandler::HandleDeferredTasks() + 70
27  libblink_modules.dylib              0x000000014cdc21ff blink::BaseAudioContext::HandlePreRenderTasks(blink::AudioIOPosition const&) + 239
28  libblink_modules.dylib              0x000000014cd29db9 blink::AudioDestinationHandler::Render(blink::AudioBus*, blink::AudioBus*, unsigned long, blink::AudioIOPosition const&) + 665
29  libblink_platform.dylib             0x0000000147b66a71 blink::AudioDestination::RequestRender(unsigned long, unsigned long, double, double, unsigned long) + 1185
30  libblink_platform.dylib             0x0000000147b65daa blink::AudioDestination::Render(blink::WebVector<float*> const&, unsigned long, double, double, unsigned long) + 2218
31  libcontent.dylib                    0x00000001264f8bb4 content::RendererWebAudioDeviceImpl::Render(base::TimeDelta, base::TimeTicks, int, media::AudioBus*) + 692
32  libmedia.dylib                      0x000000013209fdbe media::SilentSinkSuspender::Render(base::TimeDelta, base::TimeTicks, int, media::AudioBus*) + 1598
33  libmedia.dylib                      0x0000000131f102dc media::AudioOutputDevice::AudioThreadCallback::Process(unsigned int) + 1244
34  libmedia.dylib                      0x0000000131eb7726 media::AudioDeviceThread::ThreadMain() + 326
35  libbase.dylib                       0x000000011d2a3bea base::(anonymous namespace)::ThreadFunc(void*) + 682
36  libsystem_pthread.dylib             0x00007fff5bc37661 _pthread_body + 340
37  libsystem_pthread.dylib             0x00007fff5bc3750d _pthread_body + 0
38  libsystem_pthread.dylib             0x00007fff5bc36bf9 thread_start + 13

Did this work before? Yes Works in stable 65.0.3325.181

Does this work in other browsers? Yes

Chrome version: 67.0.3387.0  Channel: canary
OS Version: OS X 10.13.4
Flash Version:
 
Labels: Needs-Bisect Needs-Triage-M67

Comment 2 by rtoy@chromium.org, Apr 4 2018

Thanks for the repro case; I can confirm the issue.

The test case seems to be quite CPU-intensive; I get audio glitches on my Z840.

The backtrace seems to indicate that when updating the tail processing nodes because it's done processing, the update should happen on the main thread, not the audio thread.
Cc: sindhu.chelamcherla@chromium.org
Labels: -Needs-Bisect Triaged-ET M-67 Target-67 FoundIn-67
Owner: rtoy@chromium.org
Status: Assigned (was: Unconfirmed)
Checked the issue on reported version 67.0.3387.0, on latest canary 67.0.3388.0 and able to hear audio when clicked on audio button.

We don't have set up with debug build to bisect this .Hence removing the needs bisect label and assigning to rtoy@ as per the bisect mentioned in  C#0 for more inputs.

Based on comment#0 adding Milestone, TargetIn and FoundIn as 67. Please remove if not the case.

Thanks!

Comment 4 by rtoy@chromium.org, Apr 5 2018

Status: Started (was: Assigned)

Comment 5 by rtoy@chromium.org, Apr 10 2018

Blockedon: 829767
Need  issue 829767  to be fixed before fixing this.  They touch the same bit of code.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 12 2018

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

commit 5748a5de89a1a5f5072b2e76e33229b1f970dbc7
Author: Raymond Toy <rtoy@chromium.org>
Date: Thu Apr 12 07:30:56 2018

Handle finishing of tail processing nodes on the main thread

When a tail processing node is finished processing (because it now
propagates silence), copy the node to another vector.  On the main
thread, process this vector by disabling any outputs.

If this were done on the audio thread, disabling outputs can cause
some handlers to be deleted which also can remove summing junctions.
But summing junctions needed to be deleted in the main thread.

Bug:  828826 
Test: Run repro case and observe no crashes after many minutes.
Change-Id: I14acf7f54d621ee83e6ed910a0784ec6a0a34b9d
Reviewed-on: https://chromium-review.googlesource.com/996387
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550075}
[modify] https://crrev.com/5748a5de89a1a5f5072b2e76e33229b1f970dbc7/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
[modify] https://crrev.com/5748a5de89a1a5f5072b2e76e33229b1f970dbc7/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 13 2018

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

commit cecfc61c401ecdee47132344a6b5347dfb1c85f0
Author: Raymond Toy <rtoy@chromium.org>
Date: Fri Apr 13 01:50:49 2018

Fix leaks in WebAudio layout tests

When the context is closing, the nodes that were processing their
tails need to be stopped.  This disables the output of the node, which
might cause other nodes to disable their outputs, and these nodes are
placed on |finished_tail_processing_handlers_|.  These also need to be
handled when finishing tail processing.  So loop around processing
|finished_tail_processing_handlers_| and |tail_processing_handlers_|
until disabling of outputs no longer adds anything to either of these.


Bug:  832043 ,  828826 
Test: run-webkit-tests --enable-leak-detection no longer detects leaks
Change-Id: I6ca195352b240d343095c208c9b70b0c5553c925
Reviewed-on: https://chromium-review.googlesource.com/1010833
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550468}
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h

Comment 8 by rtoy@chromium.org, Apr 13 2018

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 16 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b70b324da48af23dfd7a05c8ef4a96eab57ed65

commit 7b70b324da48af23dfd7a05c8ef4a96eab57ed65
Author: Raymond Toy <rtoy@chromium.org>
Date: Mon Apr 16 14:31:40 2018

Fix leaks in WebAudio layout tests

When the context is closing, the nodes that were processing their
tails need to be stopped.  This disables the output of the node, which
might cause other nodes to disable their outputs, and these nodes are
placed on |finished_tail_processing_handlers_|.  These also need to be
handled when finishing tail processing.  So loop around processing
|finished_tail_processing_handlers_| and |tail_processing_handlers_|
until disabling of outputs no longer adds anything to either of these.


Bug:  832043 ,  828826 
Test: run-webkit-tests --enable-leak-detection no longer detects leaks
Change-Id: I6ca195352b240d343095c208c9b70b0c5553c925
Reviewed-on: https://chromium-review.googlesource.com/1010833
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550468}(cherry picked from commit cecfc61c401ecdee47132344a6b5347dfb1c85f0)
Reviewed-on: https://chromium-review.googlesource.com/1014164
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#14}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/7b70b324da48af23dfd7a05c8ef4a96eab57ed65/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
[modify] https://crrev.com/7b70b324da48af23dfd7a05c8ef4a96eab57ed65/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 17 2018

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

commit cecfc61c401ecdee47132344a6b5347dfb1c85f0
Author: Raymond Toy <rtoy@chromium.org>
Date: Fri Apr 13 01:50:49 2018

Fix leaks in WebAudio layout tests

When the context is closing, the nodes that were processing their
tails need to be stopped.  This disables the output of the node, which
might cause other nodes to disable their outputs, and these nodes are
placed on |finished_tail_processing_handlers_|.  These also need to be
handled when finishing tail processing.  So loop around processing
|finished_tail_processing_handlers_| and |tail_processing_handlers_|
until disabling of outputs no longer adds anything to either of these.


Bug:  832043 ,  828826 
Test: run-webkit-tests --enable-leak-detection no longer detects leaks
Change-Id: I6ca195352b240d343095c208c9b70b0c5553c925
Reviewed-on: https://chromium-review.googlesource.com/1010833
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550468}
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h

Sign in to add a comment