New issue
Advanced search Search tips

Issue 913217 link

Starred by 1 user

Issue metadata

Status: Verified
Owner: ----
Closed: Dec 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

setTargetAtTime followed by cancelAndHoldAtTime causes page to crash

Reported by realhors...@gmail.com, Dec 8

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Safari/537.36

Steps to reproduce the problem:
1. Create an AudioContext and an AudioNode with an output and an AudioParam to schedule a change for.
2. Connect the AudioNode to another and start it's output
3. Use setTargetAtTime with a time constant > 0 and after it cancelAndHoldAtTime with a startTime lower than the one used for setTargetAtTime

What is the expected behavior?
An error or exception

What went wrong?
The page crashes (Aw, Snap!)

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 70.0.3538.110  Channel: stable
OS Version: 10.0
Flash Version:
 
webaudio-crash.html
902 bytes View Download
Labels: Needs-Triage-M70
Cc: swarnasree.mukkala@chromium.org
Labels: Triaged-ET Needs-Feedback
Tested the issue on chrome version #60.0.3112.113, reported chrome version #70.0.3538.110, latest stable #71.0.3578.80 and latest chrome # 73.0.3637.0 using Windows 10. Below are the observations noted while testing the issue.

Observations:
=============
1.In #60.0.3112.113, when opened the "webaudio-crash.html" the audio got played automatically without any error/exception.
2.Able to reproduce the issue on #70.0.3538.110, observed the page crashes.
3.In #71.0.3578.80 and #73.0.3637.0, after opening "webaudio-crash.html" neither the audio plays nor the page crashes.

@reporter: Could you please help us by confirming whether above behaviours can be considered as good/bad behaviour. So that it would be really helpful in triaging the issue further.
Thanks.!
This is my first time filing a bug report, I don't know if you expect test case files to run automatically. I specifically bound my test case to the button click so it doesn't just run on page load.
Which is why your case #1 surprises me, how could it have played automatically? The ConstantSourceNode's start() is bound to the button click.

Just tried it in 71.0.3578.80 and it still crashes (which it didn't for you, did you click the button?)
Also there should not be audio playing, at least not audibly, nothing should reach the output. It's just a ConstantSourceNode connected to a AnalyzerNode which is NOT connected to the AudioContext.destination. But that step of connecting the ConstantSourceNode to another Node IS still necessary in order to cause the crash.
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 12

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: OS-Mac
Status: Available (was: Unconfirmed)
Confirmed it crashes 73.0.3638.0.
Here's the stack trace:

---
* thread #28, name = 'AudioOutputDevice', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001917d3d6c libblink_modules.dylib`blink::AudioParamTimeline::ParamEvent::GetType(this=0x0000000000000000) const at audio_param_timeline.h:164
    frame #1: 0x00000001917dae62 libblink_modules.dylib`blink::AudioParamTimeline::HandleCancelValues(this=0x0000001c51354df8, current_event=0x00007ffed1b77a60, next_event=0x00007ffed0c31c30, value2=0, time2=1.9999899999999999) at audio_param_timeline.cc:1336
    frame #2: 0x00000001917d97b0 libblink_modules.dylib`blink::AudioParamTimeline::ValuesForFrameRangeImpl(this=0x0000001c51354df8, start_frame=0, end_frame=128, default_value=1, values=0x0000001c512edfa0, number_of_values=128, sample_rate=48000, control_rate=48000) at audio_param_timeline.cc:951
    frame #3: 0x00000001917d9044 libblink_modules.dylib`blink::AudioParamTimeline::ValuesForFrameRange(this=0x0000001c51354df8, start_frame=0, end_frame=128, default_value=1, values=0x0000001c512edfa0, number_of_values=128, sample_rate=48000, control_rate=48000, min_value=-3.40282347E+38, max_value=3.40282347E+38) at audio_param_timeline.cc:853
    frame #4: 0x00000001917c5fab libblink_modules.dylib`blink::AudioParamHandler::CalculateTimelineValues(this=0x0000001c51354d90, values=0x0000001c512edfa0, number_of_values=128) at audio_param.cc:296
    frame #5: 0x00000001917c5972 libblink_modules.dylib`blink::AudioParamHandler::CalculateFinalValues(this=0x0000001c51354d90, values=0x0000001c512edfa0, number_of_values=128, sample_accurate=true) at audio_param.cc:246
    frame #6: 0x00000001917c5e66 libblink_modules.dylib`blink::AudioParamHandler::CalculateSampleAccurateValues(this=0x0000001c51354d90, values=0x0000001c512edfa0, number_of_values=128) at audio_param.cc:229
    frame #7: 0x00000001918318ab libblink_modules.dylib`blink::ConstantSourceHandler::Process(this=0x0000001c512cc430, frames_to_process=128) at constant_source_node.cc:77
    frame #8: 0x000000019179e78d libblink_modules.dylib`blink::AudioHandler::ProcessIfNecessary(this=0x0000001c512cc430, frames_to_process=128) at audio_node.cc:349
    frame #9: 0x00000001917b7210 libblink_modules.dylib`blink::AudioNodeOutput::Pull(this=0x0000001c51234640, in_place_bus=0x0000001c51248910, frames_to_process=128) at audio_node_output.cc:143
    frame #10: 0x00000001917b50e9 libblink_modules.dylib`blink::AudioNodeInput::Pull(this=0x0000001c512346d0, in_place_bus=0x0000001c51248910, frames_to_process=128) at audio_node_input.cc:150
    frame #11: 0x000000019177f16f libblink_modules.dylib`blink::AudioBasicInspectorHandler::PullInputs(this=0x0000001c513ac8d0, frames_to_process=128) at audio_basic_inspector_node.cc:50
    frame #12: 0x000000019179e70c libblink_modules.dylib`blink::AudioHandler::ProcessIfNecessary(this=0x0000001c513ac8d0, frames_to_process=128) at audio_node.cc:334
    frame #13: 0x00000001918389e7 libblink_modules.dylib`blink::DeferredTaskHandler::ProcessAutomaticPullNodes(this=0x0000001c513cc910, frames_to_process=128) at deferred_task_handler.cc:165
    frame #14: 0x0000000191836398 libblink_modules.dylib`blink::DefaultAudioDestinationHandler::Render(this=0x0000001c51344880, destination_bus=0x0000001c51248568, number_of_frames=128, output_position=0x000070000c0e9e88) at default_audio_destination_node.cc:194
---

It seems like ParamEvent is a null pointer. Somehow. rtoy@ would know better why.

Also I did some more experiments and this crash always happens with a "setTarget" followed by "cancel".
A couple of problems in the AudioParamTimeline::HandleCancleValues()

1. |next_event| argument can be a null-pointer. Although it bypasses the code when it is null, in general not a good practice.
2. |next_event->SaveEvent()| can be also null-pointer and the code is not prepared to handle such case. (hence crash here)

The suggested fix is to add a check on next_event->SavedEvent() before processing an event. However, the implication from this change is unsure. I will make the fix and test it against the test suite.
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 12

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

commit 46f2e65bb93dea9d7eeda2603c8cd65b1af5051f
Author: Hongchan Choi <hongchan@chromium.org>
Date: Wed Dec 12 23:40:33 2018

Check if |SavedEvent| exists handling cancel values

In AudioParamTimeline::HandleCancelValues() function, the code expects
the |saved_event| pointer from the next event to be non-null. There are
corner cases where this is not true.

Adding one more check on the saved event pointer fixes the crash.
Locally confirmed the attached web test crashes without this fix.

Bug:  913217 
Test: webaudio/AudioParam/cancel-values-crash-913217.html
Change-Id: I4760b939534ef92a2475087540e17c8abc784b3b
Reviewed-on: https://chromium-review.googlesource.com/c/1374492
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616107}
[modify] https://crrev.com/46f2e65bb93dea9d7eeda2603c8cd65b1af5051f/third_party/blink/renderer/modules/webaudio/audio_param_timeline.cc
[modify] https://crrev.com/46f2e65bb93dea9d7eeda2603c8cd65b1af5051f/third_party/blink/renderer/modules/webaudio/audio_param_timeline.h
[add] https://crrev.com/46f2e65bb93dea9d7eeda2603c8cd65b1af5051f/third_party/blink/web_tests/webaudio/AudioParam/cancel-values-crash-913217.html

Status: Verified (was: Available)

Sign in to add a comment