New issue
Advanced search Search tips

Issue 884059 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 856641



Sign in to add a comment

Finalizers should not require the graph lock

Project Member Reported by rtoy@chromium.org, Sep 14

Issue description

From the comment https://bugs.chromium.org/p/chromium/issues/detail?id=882314#c12 we should check the code and make sure finalizers don't require the graph lock.
 
Blocking: 856641
This is needed before we can get rid of WTF::RecursiveMutex
Cc: jbroman@chromium.org
Cc: hongchan@chromium.org rtoy@chromium.org
Rough design doc for what I hope will resolve the rest of this:
https://docs.google.com/document/d/1kLLBk_2G25jHPyk0U80f0cl2QDSHijbzVKdUrqmE91w/edit?usp=sharing

It kind of moves complexity around but it's hard to actually remove the complexity that arises from this GC/ref-counted split. If this seems reasonable, I'll send out incremental CLs.
Owner: jbroman@chromium.org
Status: Started (was: Available)
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 5

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

commit 158dabdaff39816c9e1209c4795b9571caaa905e
Author: Jeremy Roman <jbroman@chromium.org>
Date: Wed Dec 05 17:08:15 2018

webaudio: Hold a DeferredTaskHandler reference from the AudioHandler.

It's planned to allow AudioHandler to be destroyed after the pre-finalization
step of BaseAudioContext. Notably if the BAC is no longer needed, it can
be pre-finalized in the same pass as its associated AudioNodes, but
destruction of the handlers will happen later as it involves non-trivial
work which should not be part of a pre-finalizer.

Consequentially, accessing the DeferredTaskHandler through the context will
not be valid at destruction time, and so it is necessary for the handler to
hold a direct reference which will not be invalidated by finalization of
the BaseAudioContext.

This reference cycle will not last, because the DeferredTaskHandler only
briefly holds references to AudioHandler instances.

Bug: 884059
Change-Id: I7cc4c33a89183c6723ac77e800d8af6a01c14dc7
Reviewed-on: https://chromium-review.googlesource.com/c/1354311
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614000}
[modify] https://crrev.com/158dabdaff39816c9e1209c4795b9571caaa905e/third_party/blink/renderer/modules/webaudio/audio_node.cc
[modify] https://crrev.com/158dabdaff39816c9e1209c4795b9571caaa905e/third_party/blink/renderer/modules/webaudio/audio_node.h
[modify] https://crrev.com/158dabdaff39816c9e1209c4795b9571caaa905e/third_party/blink/renderer/modules/webaudio/audio_node_output.h

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 27

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

commit 7ed172bc60e0a65cbd50f2d6096fab55422957b2
Author: Jeremy Roman <jbroman@chromium.org>
Date: Thu Dec 27 16:08:58 2018

webaudio: Always delete handlers in a non-nestable task on the main thread.

Deleting handlers does considerable non-trivial work that requires the
graph lock. Allowing this to happen during finalization introduces
considerable complexity because it can happen during any GC heap allocation,
including ones that occur while the graph lock is already held.

Instead, when the last reference is dropped, the handler is not deleted
immediately, but a non-nestable task to delete it is enqueued. Since the
task is non-nestable, it will not run while the graph lock is held by the
main thread (and if it is held by another thread, it will block on it).

This slightly extends the lifetime of audio handlers, during which time
it is illegal to take a new reference (and RefCounted checks this in
debug builds). So a flag is set in the destructor (though it could be
sensibly set as early as the ref count reaches zero) that prevents the
handler from adding itself to the tail processing queue if it is already
being destroyed.

The cycle collection layout test is updated to handle the fact that the
handler count does not go to zero immediately on GC, but shortly thereafter.
This has been done by merging with another recently added unit test, which
tests something very similar (a cycle of one element).

Bug: 884059
Change-Id: I2109d7a0400d5e2c013aa1b113a80ec16d0118f2
Reviewed-on: https://chromium-review.googlesource.com/c/1261966
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619039}
[modify] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/renderer/modules/webaudio/BUILD.gn
[modify] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/renderer/modules/webaudio/audio_node.cc
[modify] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/renderer/modules/webaudio/audio_node.h
[modify] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/renderer/modules/webaudio/audio_param.h
[modify] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
[add] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.cc
[add] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.h
[delete] https://crrev.com/d378328be2a30761d73cd2d15f4131f7041435ca/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc-expected.txt
[modify] https://crrev.com/7ed172bc60e0a65cbd50f2d6096fab55422957b2/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc.html
[delete] https://crrev.com/d378328be2a30761d73cd2d15f4131f7041435ca/third_party/blink/web_tests/webaudio/internals/self-connection-gc.html

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 27

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

commit f9a49552d9d65a729fe6f5c0ce1885b4f918a43f
Author: Jeremy Roman <jbroman@chromium.org>
Date: Thu Dec 27 18:48:17 2018

Revert "webaudio: Always delete handlers in a non-nestable task on the main thread."

This reverts commit 7ed172bc60e0a65cbd50f2d6096fab55422957b2.

Reason for revert: Causing reported leaks:
https://test-results.appspot.com/data/layout_results/WebKit_Linux_Trusty_Leak/27998/webkit_layout_tests/layout-test-results/results.html

Need to dig further to determine whether this is a case of the leak detector needing to be tweaked to account for the slightly deferred destruction, or whether this CL introduced genuine leaks.

Original change's description:
> webaudio: Always delete handlers in a non-nestable task on the main thread.
> 
> Deleting handlers does considerable non-trivial work that requires the
> graph lock. Allowing this to happen during finalization introduces
> considerable complexity because it can happen during any GC heap allocation,
> including ones that occur while the graph lock is already held.
> 
> Instead, when the last reference is dropped, the handler is not deleted
> immediately, but a non-nestable task to delete it is enqueued. Since the
> task is non-nestable, it will not run while the graph lock is held by the
> main thread (and if it is held by another thread, it will block on it).
> 
> This slightly extends the lifetime of audio handlers, during which time
> it is illegal to take a new reference (and RefCounted checks this in
> debug builds). So a flag is set in the destructor (though it could be
> sensibly set as early as the ref count reaches zero) that prevents the
> handler from adding itself to the tail processing queue if it is already
> being destroyed.
> 
> The cycle collection layout test is updated to handle the fact that the
> handler count does not go to zero immediately on GC, but shortly thereafter.
> This has been done by merging with another recently added unit test, which
> tests something very similar (a cycle of one element).
> 
> Bug: 884059
> Change-Id: I2109d7a0400d5e2c013aa1b113a80ec16d0118f2
> Reviewed-on: https://chromium-review.googlesource.com/c/1261966
> Reviewed-by: Hongchan Choi <hongchan@chromium.org>
> Commit-Queue: Jeremy Roman <jbroman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619039}

TBR=jbroman@chromium.org,hongchan@chromium.org

Change-Id: I28812f754a3ae142ecad48318b17212624ee7702
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 884059
Reviewed-on: https://chromium-review.googlesource.com/c/1391772
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619053}
[modify] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/renderer/modules/webaudio/BUILD.gn
[modify] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/renderer/modules/webaudio/audio_node.cc
[modify] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/renderer/modules/webaudio/audio_node.h
[modify] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/renderer/modules/webaudio/audio_param.h
[modify] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
[delete] https://crrev.com/ffe65028319b24fe413809cdf6048d7936e252c3/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.cc
[delete] https://crrev.com/ffe65028319b24fe413809cdf6048d7936e252c3/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.h
[add] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc-expected.txt
[modify] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc.html
[add] https://crrev.com/f9a49552d9d65a729fe6f5c0ce1885b4f918a43f/third_party/blink/web_tests/webaudio/internals/self-connection-gc.html

For posterity, the leaks reported were in:

 editing/deleting/delete_image.html
 editing/selection/modify_move/move_left_word_09_rtl_multi_line.html
 external/wpt/mediacapture-record/MediaRecorder-error.html
 external/wpt/mst-content-hint/MediaStreamTrack-contentHint.html
 external/wpt/mst-content-hint/idlharness.window.html
 external/wpt/webaudio/idlharness.https.window.html
 external/wpt/webaudio/the-audio-api/the-analysernode-interface/realtimeanalyser-fft-scaling.html
 external/wpt/webaudio/the-audio-api/the-analysernode-interface/test-analyser-gain.html
 external/wpt/webaudio/the-audio-api/the-analysernode-interface/test-analyser-scale.html
 external/wpt/webaudio/the-audio-api/the-analysernode-interface/test-analysernode.html
 external/wpt/webaudio/the-audio-api/the-audiobuffer-interface/audiobuffer-reuse.html
 external/wpt/webaudio/the-audio-api/the-audiobuffer-interface/ctor-audiobuffer.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-basic.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-channels.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-ended.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-grain.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-multi-channels.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-one-sample-loop.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-playbackrate-zero.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-start.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiosource-onended.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiosource-time-limits.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/buffer-resampling.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/ctor-audiobuffersource.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/note-grain-on-play.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/note-grain-on-timing.html
 external/wpt/webaudio/the-audio-api/the-audiobuffersourcenode-interface/sample-accurate-scheduling.html
 external/wpt/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-suspend-resume.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/audionode-channel-rules.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/audionode-connect-method-chaining.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/audionode-connect-order.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/audionode-connect-return-value.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/audionode-disconnect-audioparam.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/audionode-disconnect.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/audionode.html
 external/wpt/webaudio/the-audio-api/the-audionode-interface/channel-mode-interp-basic.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-connect-audioratesignal.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-exceptional-values.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-exponentialRampToValueAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-large-endtime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-linearRampToValueAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-method-chaining.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-setTargetAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-setValueAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-setValueCurve-exceptions.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-setValueCurveAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/audioparam-summingjunction.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/automation-rate.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/event-insertion.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-audioworklet.https.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-biquad.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-constant-source.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-delay.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-gain.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-oscillator.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-panner.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/k-rate-stereo-panner.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/retrospective-exponentialRampToValueAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/retrospective-linearRampToValueAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/retrospective-setTargetAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/retrospective-setValueAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/retrospective-setValueCurveAtTime.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/setTargetAtTime-after-event-within-block.html
 external/wpt/webaudio/the-audio-api/the-audioparam-interface/setValueAtTime-within-block.html
 external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-audioparam-size.https.html
 external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworklet-audioparam.https.html
 external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworkletglobalscope-sample-rate.https.html
 external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworkletnode-automatic-pull.https.html
 external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworkletnode-channel-count.https.html
 external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworkletnode-disconnected-input.https.html
 external/wpt/webaudio/the-audio-api/the-audioworklet-interface/audioworkletnode-output-channel-count.https.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-allpass.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-automation.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-bandpass.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-basic.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-getFrequencyResponse.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-highpass.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-highshelf.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-lowpass.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-lowshelf.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-notch.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-peaking.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquad-tail.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/biquadfilternode-basic.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/ctor-biquadfilter.html
 external/wpt/webaudio/the-audio-api/the-biquadfilternode-interface/no-dezippering.html
 external/wpt/webaudio/the-audio-api/the-channelmergernode-interface/audiochannelmerger-disconnect.html
 external/wpt/webaudio/the-audio-api/the-channelmergernode-interface/audiochannelmerger-input-non-default.html
 external/wpt/webaudio/the-audio-api/the-channelmergernode-interface/audiochannelmerger-input.html
 external/wpt/webaudio/the-audio-api/the-channelsplitternode-interface/audiochannelsplitter.html
 external/wpt/webaudio/the-audio-api/the-constantsourcenode-interface/constant-source-basic.html
 external/wpt/webaudio/the-audio-api/the-constantsourcenode-interface/constant-source-onended.html
 external/wpt/webaudio/the-audio-api/the-constantsourcenode-interface/constant-source-output.html
 external/wpt/webaudio/the-audio-api/the-constantsourcenode-interface/ctor-constantsource.html
 external/wpt/webaudio/the-audio-api/the-constantsourcenode-interface/test-constantsourcenode.html
 external/wpt/webaudio/the-audio-api/the-convolvernode-interface/convolution-mono-mono.html
 external/wpt/webaudio/the-audio-api/the-convolvernode-interface/convolver-cascade.html
 external/wpt/webaudio/the-audio-api/the-convolvernode-interface/convolver-response-1-chan.html
 external/wpt/webaudio/the-audio-api/the-convolvernode-interface/convolver-response-2-chan.html
 external/wpt/webaudio/the-audio-api/the-convolvernode-interface/convolver-response-4-chan.html
 external/wpt/webaudio/the-audio-api/the-convolvernode-interface/convolver-upmixing-1-channel-response.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/ctor-delay.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/delaynode-channel-count-1.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/delaynode-max-default-delay.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/delaynode-max-nondefault-delay.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/delaynode-maxdelay.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/delaynode-maxdelaylimit.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/delaynode-scheduling.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/delaynode.html
 external/wpt/webaudio/the-audio-api/the-delaynode-interface/no-dezippering.html
 external/wpt/webaudio/the-audio-api/the-dynamicscompressornode-interface/ctor-dynamicscompressor.html
 external/wpt/webaudio/the-audio-api/the-dynamicscompressornode-interface/dynamicscompressor-basic.html
 external/wpt/webaudio/the-audio-api/the-gainnode-interface/ctor-gain.html
 external/wpt/webaudio/the-audio-api/the-gainnode-interface/gain-basic.html
 external/wpt/webaudio/the-audio-api/the-gainnode-interface/gain.html
 external/wpt/webaudio/the-audio-api/the-gainnode-interface/no-dezippering.html
 external/wpt/webaudio/the-audio-api/the-iirfilternode-interface/ctor-iirfilter.html
 external/wpt/webaudio/the-audio-api/the-iirfilternode-interface/iirfilter-getFrequencyResponse.html
 external/wpt/webaudio/the-audio-api/the-iirfilternode-interface/iirfilter.html
 external/wpt/webaudio/the-audio-api/the-oscillatornode-interface/ctor-oscillator.html
 external/wpt/webaudio/the-audio-api/the-oscillatornode-interface/detune-limiting.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/ctor-panner.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/distance-exponential.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/distance-inverse.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/distance-linear.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/panner-automation-basic.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/panner-automation-equalpower-stereo.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/panner-automation-position.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/panner-distance-clamping.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/panner-equalpower-stereo.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/panner-equalpower.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/panner-rolloff-clamping.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/pannernode-basic.html
 external/wpt/webaudio/the-audio-api/the-pannernode-interface/test-pannernode-automation.html
 external/wpt/webaudio/the-audio-api/the-stereopanner-interface/ctor-stereopanner.html
 external/wpt/webaudio/the-audio-api/the-stereopanner-interface/no-dezippering.html
 external/wpt/webaudio/the-audio-api/the-stereopanner-interface/stereopannernode-basic.html
 external/wpt/webaudio/the-audio-api/the-stereopanner-interface/stereopannernode-panning.html
 external/wpt/webaudio/the-audio-api/the-waveshapernode-interface/curve-tests.html
 external/wpt/webaudio/the-audio-api/the-waveshapernode-interface/silent-inputs.html
 external/wpt/webaudio/the-audio-api/the-waveshapernode-interface/waveshaper-copy-curve.html
 external/wpt/webaudio/the-audio-api/the-waveshapernode-interface/waveshaper-limits.html
 external/wpt/webaudio/the-audio-api/the-waveshapernode-interface/waveshaper.html
 external/wpt/webrtc/RTCDTMFSender-insertDTMF.https.html
 external/wpt/webrtc/RTCDTMFSender-ontonechange-long.https.html
 external/wpt/webrtc/RTCDTMFSender-ontonechange.https.html
 external/wpt/webrtc/RTCPeerConnection-addTrack.https.html
 external/wpt/webrtc/RTCPeerConnection-getStats.https.html
 external/wpt/webrtc/RTCPeerConnection-ontrack.https.html
 external/wpt/webrtc/RTCPeerConnection-removeTrack.https.html
 external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-replaceTrack.https.html
 external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html
 external/wpt/webrtc/RTCPeerConnection-track-stats.https.html
 external/wpt/webrtc/RTCRtpReceiver-getContributingSources.https.html
 external/wpt/webrtc/RTCRtpReceiver-getStats.https.html
 external/wpt/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html
 external/wpt/webrtc/RTCRtpSender-getStats.https.html
 external/wpt/webrtc/idlharness.https.window.html
 external/wpt/webrtc/simplecall-no-ssrcs.https.html
 external/wpt/webrtc/simplecall.https.html
 external/wpt/webrtc/legacy/RTCPeerConnection-addStream.https.html
 external/wpt/webrtc/legacy/RTCPeerConnection-createOffer-offerToReceive.html
 external/wpt/webrtc/protocol/missing-fields.html
 fast/mediastream/MediaStreamTrack-clone.html
 media/autoplay/webaudio-audio-context-resume.html
 media/autoplay/webaudio-node-start.html
 virtual/disabled-service-worker-servicification/external/wpt/service-workers/service-worker/skip-waiting-without-using-registration.https.html
 virtual/not-site-per-process/http/tests/webaudio/autoplay-crossorigin.html
 virtual/video-surface-layer/media/autoplay/webaudio-audio-context-resume.html
 virtual/video-surface-layer/media/autoplay/webaudio-node-start.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCDTMFSender-insertDTMF.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCDTMFSender-ontonechange-long.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCDTMFSender-ontonechange.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-addTrack.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-getStats.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-ontrack.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-removeTrack.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-replaceTrack.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-track-stats.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCRtpReceiver-getContributingSources.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCRtpReceiver-getStats.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCRtpSender-getStats.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/idlharness.https.window.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/simplecall-no-ssrcs.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/simplecall.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/legacy/RTCPeerConnection-addStream.https.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/legacy/RTCPeerConnection-createOffer-offerToReceive.html
 virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/protocol/missing-fields.html
 webaudio/dom-exceptions.html
 webaudio/mixing.html
 webaudio/Analyser/automatic-pull-node.html
 webaudio/Analyser/handle-silent-inputs.html
 webaudio/Analyser/realtimeanalyser-byte-data.html
 webaudio/Analyser/realtimeanalyser-downmix.html
 webaudio/Analyser/realtimeanalyser-fftsize-reset.html
 webaudio/Analyser/realtimeanalyser-float-data.html
 webaudio/Analyser/realtimeanalyser-freq-data-smoothing.html
 webaudio/Analyser/realtimeanalyser-freq-data.html
 webaudio/Analyser/realtimeanalyser-multiple-calls.html
 webaudio/Analyser/realtimeanalyser-zero.html
 webaudio/AudioBuffer/audiobuffer-resample.html
 webaudio/AudioBufferSource/audiobuffersource-detune-modulated-impulse.html
 webaudio/AudioBufferSource/audiobuffersource-detune-modulation.html
 webaudio/AudioBufferSource/audiobuffersource-late-start.html
 webaudio/AudioBufferSource/audiobuffersource-loop-comprehensive.html
 webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration.html
 webaudio/AudioBufferSource/audiobuffersource-loop-points.html
 webaudio/AudioBufferSource/audiobuffersource-playbackrate-modulated-impulse.html
 webaudio/AudioBufferSource/audiobuffersource-playbackrate-modulation.html
 webaudio/AudioBufferSource/audiobuffersource-playbackrate.html
 webaudio/AudioBufferSource/audiobuffersource-premature-loop-stop.html
 webaudio/AudioContext/autoplay-state-change.html
 webaudio/AudioListener/audiolistener-automation-position.html
 webaudio/AudioListener/audiolistener-set-position.html
 webaudio/AudioNode/tail-connections.html
 webaudio/AudioNode/tail-processing.html
 webaudio/AudioParam/audioparam-automation-clamping.html
 webaudio/AudioParam/audioparam-cancel-and-hold.html
 webaudio/AudioParam/audioparam-clamp-time-to-current-time.html
 webaudio/AudioParam/audioparam-initial-event.html
 webaudio/AudioParam/audioparam-k-rate.html
 webaudio/AudioParam/audioparam-linearRamp-value-attribute.html
 webaudio/AudioParam/audioparam-negative-exponentialRamp.html
 webaudio/AudioParam/audioparam-nominal-range.html
 webaudio/AudioParam/audioparam-processing.html
 webaudio/AudioParam/audioparam-sampling.html
 webaudio/AudioParam/audioparam-setTarget-timeConstant-0.html
 webaudio/AudioParam/audioparam-setTargetAtTime-continuous.html
 webaudio/AudioParam/audioparam-setTargetAtTime-limit.html
 webaudio/AudioParam/audioparam-setTargetAtTime-sampling.html
 webaudio/AudioParam/audioparam-setValueCurve-copy.html
 webaudio/AudioParam/audioparam-setValueCurve-duration.html
 webaudio/AudioParam/audioparam-setValueCurve-end.html
 webaudio/AudioParam/audioparam-setValueCurveAtTime-interpolation.html
 webaudio/AudioParam/audioparam-update-value-attribute.html
 webaudio/AudioParam/audioparam-value-setter-error.html
 webaudio/AudioParam/cancel-values-crash-913217.html
 webaudio/AudioParam/value-setter-warnings.html
 webaudio/AudioParam/worklet-warnings.html
 webaudio/BiquadFilter/biquad-829349.html
 webaudio/BiquadFilter/tail-time-allpass.html
 webaudio/BiquadFilter/tail-time-bandpass.html
 webaudio/BiquadFilter/tail-time-highpass.html
 webaudio/BiquadFilter/tail-time-highshelf.html
 webaudio/BiquadFilter/tail-time-lowpass.html
 webaudio/BiquadFilter/tail-time-lowshelf.html
 webaudio/BiquadFilter/tail-time-notch.html
 webaudio/BiquadFilter/tail-time-peaking.html
 webaudio/ChannelMerger/audiochannelmerger-cycle.html
 webaudio/DynamicsCompressor/dynamicscompressor-clear-internal-state.html
 webaudio/DynamicsCompressor/dynamicscompressor-simple.html
 webaudio/IIRFilter/iir-tail-time.html
 webaudio/OfflineAudioContext/offlineaudiocontext-event-listener-gc.html
 webaudio/OfflineAudioContext/offlineaudiocontext-promise-basic.html
 webaudio/OfflineAudioContext/offlineaudiocontext-promise.html
 webaudio/OfflineAudioContext/offlineaudiocontext-suspend-resume-basic.html
 webaudio/OfflineAudioContext/offlineaudiocontext-suspend-resume-graph-manipulation.html
 webaudio/OfflineAudioContext/onstatechange.html
 webaudio/Oscillator/no-dezippering.html
 webaudio/Oscillator/osc-440hz.html
 webaudio/Oscillator/osc-low-freq.html
 webaudio/Oscillator/osc-negative-freq.html
 webaudio/Oscillator/osc-sweep-snr-custom.html
 webaudio/Oscillator/osc-sweep-snr-sawtooth.html
 webaudio/Oscillator/osc-sweep-snr-sine.html
 webaudio/Oscillator/osc-sweep-snr-square.html
 webaudio/Oscillator/osc-sweep-snr-triangle.html
 webaudio/Oscillator/oscillator-basic.html
 webaudio/Oscillator/oscillator-ended.html
 webaudio/Oscillator/oscillator-late-start.html
 webaudio/Oscillator/start-sampling.html
 webaudio/Panner/panner-loop.html
 webaudio/Panner/panner-set-position.html
 webaudio/PeriodicWave/ctor-periodicwave.html
 webaudio/PeriodicWave/periodicwave-contexts.html
 webaudio/PeriodicWave/periodicwave-lengths.html
 webaudio/PeriodicWave/periodicwave-normalization.html
 webaudio/ScriptProcessor/scriptprocessor-offlineaudiocontext.html
 webaudio/ScriptProcessor/scriptprocessornode-downmix8-2channel-input.html
 webaudio/ScriptProcessor/scriptprocessornode-upmix2-8channel-input.html
 webaudio/ScriptProcessor/scriptprocessornode-zero-input-channels.html
 webaudio/ScriptProcessor/scriptprocessornode.html
 webaudio/WaveShaper/waveshaper-oversample-2x.html
 webaudio/WaveShaper/waveshaper-oversample-4x.html
 webaudio/internals/audiobuffersource.html
 webaudio/internals/audiocontext-close.html
 webaudio/internals/audiosource-premature-gc.html
 webaudio/internals/offlineaudiocontext-detached-no-crash.html
 webaudio/internals/scriptprocessornode-premature-death.html
 webaudio/internals/scriptprocessornode-rewrap.html
 webaudio/unit-tests/audit.html
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 11

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

commit 02090026451f32dfc651f6dd2e384bf2c4f45b69
Author: Jeremy Roman <jbroman@chromium.org>
Date: Fri Jan 11 01:06:47 2019

[Reland] webaudio: Always delete handlers in a non-nestable task on the main thread.

Deleting handlers does considerable non-trivial work that requires the
graph lock. Allowing this to happen during finalization introduces
considerable complexity because it can happen during any GC heap allocation,
including ones that occur while the graph lock is already held.

Instead, when the last reference is dropped, the handler is not deleted
immediately, but a non-nestable task to delete it is enqueued. Since the
task is non-nestable, it will not run while the graph lock is held by the
main thread (and if it is held by another thread, it will block on it).

This slightly extends the lifetime of audio handlers, during which time
it is illegal to take a new reference (and RefCounted checks this in
debug builds). So a flag is set in the destructor (though it could be
sensibly set as early as the ref count reaches zero) that prevents the
handler from adding itself to the tail processing queue if it is already
being destroyed.

The cycle collection layout test is updated to handle the fact that the
handler count does not go to zero immediately on GC, but shortly thereafter.
This has been done by merging with another recently added unit test, which
tests something very similar (a cycle of one element).

The leak detector has been augmented to wait for audio handlers that are
awaiting deletion, if there are any.

Bug: 884059
Change-Id: I818bcdbf87dc3e83359ad7a62aa56547dea2d7a5
Reviewed-on: https://chromium-review.googlesource.com/c/1392109
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621840}
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/DEPS
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/controller/blink_leak_detector.cc
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/controller/blink_leak_detector.h
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/modules/webaudio/BUILD.gn
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/modules/webaudio/audio_node.cc
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/modules/webaudio/audio_node.h
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/modules/webaudio/audio_param.h
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
[add] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.cc
[add] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.h
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
[delete] https://crrev.com/3ae9799de9c6e85984b1b1c4618101bf2702d99e/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc-expected.txt
[modify] https://crrev.com/02090026451f32dfc651f6dd2e384bf2c4f45b69/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc.html
[delete] https://crrev.com/3ae9799de9c6e85984b1b1c4618101bf2702d99e/third_party/blink/web_tests/webaudio/internals/self-connection-gc.html

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 11

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

commit 214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f
Author: Jeremy Roman <jbroman@chromium.org>
Date: Fri Jan 11 19:55:06 2019

Revert "[Reland] webaudio: Always delete handlers in a non-nestable task on the main thread."

This reverts commit 02090026451f32dfc651f6dd2e384bf2c4f45b69.

Reason for revert: Causing crashes in canary.

Original change's description:
> [Reland] webaudio: Always delete handlers in a non-nestable task on the main thread.
>
> Deleting handlers does considerable non-trivial work that requires the
> graph lock. Allowing this to happen during finalization introduces
> considerable complexity because it can happen during any GC heap allocation,
> including ones that occur while the graph lock is already held.
>
> Instead, when the last reference is dropped, the handler is not deleted
> immediately, but a non-nestable task to delete it is enqueued. Since the
> task is non-nestable, it will not run while the graph lock is held by the
> main thread (and if it is held by another thread, it will block on it).
>
> This slightly extends the lifetime of audio handlers, during which time
> it is illegal to take a new reference (and RefCounted checks this in
> debug builds). So a flag is set in the destructor (though it could be
> sensibly set as early as the ref count reaches zero) that prevents the
> handler from adding itself to the tail processing queue if it is already
> being destroyed.
>
> The cycle collection layout test is updated to handle the fact that the
> handler count does not go to zero immediately on GC, but shortly thereafter.
> This has been done by merging with another recently added unit test, which
> tests something very similar (a cycle of one element).
>
> The leak detector has been augmented to wait for audio handlers that are
> awaiting deletion, if there are any.
>
> Bug: 884059
> Change-Id: I818bcdbf87dc3e83359ad7a62aa56547dea2d7a5
> Reviewed-on: https://chromium-review.googlesource.com/c/1392109
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Hongchan Choi <hongchan@chromium.org>
> Commit-Queue: Jeremy Roman <jbroman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#621840}

TBR=jbroman@chromium.org,haraken@chromium.org,hongchan@chromium.org

Change-Id: Ib13b613f6fa696935f64362f65eb94cba50acdfd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 884059,921018
Reviewed-on: https://chromium-review.googlesource.com/c/1407501
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622109}
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/DEPS
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/controller/blink_leak_detector.cc
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/controller/blink_leak_detector.h
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/modules/webaudio/BUILD.gn
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/modules/webaudio/audio_node.cc
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/modules/webaudio/audio_node.h
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/modules/webaudio/audio_param.h
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/renderer/modules/webaudio/base_audio_context.cc
[delete] https://crrev.com/dcc0fd9fc4cdcb05eb07a2efd2f083c88f1e31c6/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.cc
[delete] https://crrev.com/dcc0fd9fc4cdcb05eb07a2efd2f083c88f1e31c6/third_party/blink/renderer/modules/webaudio/delete_soon_with_graph_lock.h
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
[add] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc-expected.txt
[modify] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/web_tests/webaudio/internals/cycle-connection-gc.html
[add] https://crrev.com/214678c7af4b7b3de4ca2cc95c1cc37ff7bceb2f/third_party/blink/web_tests/webaudio/internals/self-connection-gc.html

Sign in to add a comment