Skip to content

Commit 896ad97

Browse files
cdumezcdumez
committed
Crash under JSC::Heap::acquireAccessSlow() / JSC::Heap::releaseAccess() when starting an AudioWorklet
https://bugs.webkit.org/show_bug.cgi?id=219183 <rdar://problem/71188544> Reviewed by Mark Lam. Source/WebCore: When audio rendering has already started when the AudioWorklet is constructed, it is possible for AudioWorkletGlobalScope::handlePreRenderTasks() to get called initially on the initial audio rendering thread instead of the audio worklet thread. Once the AudioWorklet is ready, the next rendering quantums will actually get processed on the audio worklet thread. However, there is a race when audio rendering has already started when the AudioWorklet gets created. This is not normally an issue. However, AudioWorkletGlobalScope::handlePreRenderTasks() grabs a JavaScript Lock and it is only safe to do so on the thread where we constructed the VM (i.e. the Audio Worklet thread). To address the issue, we now only grab the lock if we are on the audio worklet thread. Note that this lock is only used to delay the draining of the microtask queue until the end of the rendering quantum. Test: webaudio/worklet-crash.html * Modules/webaudio/AudioWorkletGlobalScope.cpp: (WebCore::AudioWorkletGlobalScope::handlePreRenderTasks): LayoutTests: Add layout test coverage. * webaudio/worklet-crash-expected.txt: Added. * webaudio/worklet-crash.html: Added. Canonical link: https://commits.webkit.org/231795@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@270056 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 8a664b1 commit 896ad97

5 files changed

Lines changed: 61 additions & 1 deletion

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2020-11-19 Chris Dumez <cdumez@apple.com>
2+
3+
Crash under JSC::Heap::acquireAccessSlow() / JSC::Heap::releaseAccess() when starting an AudioWorklet
4+
https://bugs.webkit.org/show_bug.cgi?id=219183
5+
<rdar://problem/71188544>
6+
7+
Reviewed by Mark Lam.
8+
9+
Add layout test coverage.
10+
11+
* webaudio/worklet-crash-expected.txt: Added.
12+
* webaudio/worklet-crash.html: Added.
13+
114
2020-11-19 Chris Dumez <cdumez@apple.com>
215

316
Unreviewed, skip some of the tests imported in r270037 on iOS because they are timing out.

‎LayoutTests/webaudio/worklet-crash-expected.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
This test passes if it does not crash.

‎LayoutTests/webaudio/worklet-crash.html‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!DOCTYPE html><!-- webkit-test-runner [ jscOptions=--collectContinuously=true ] -->
2+
<html>
3+
<body>
4+
This test passes if it does not crash.
5+
<script>
6+
if (window.testRunner) {
7+
testRunner.dumpAsText();
8+
testRunner.waitUntilDone();
9+
}
10+
11+
new AudioContext().createBiquadFilter().context.audioWorklet.addModule('').then(() => {
12+
testRunner.notifyDone();
13+
}, (e) => {
14+
console.log(e);
15+
testRunner.notifyDone();
16+
});
17+
</script>
18+
</body>
19+
</html>

‎Source/WebCore/ChangeLog‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,27 @@
1+
2020-11-19 Chris Dumez <cdumez@apple.com>
2+
3+
Crash under JSC::Heap::acquireAccessSlow() / JSC::Heap::releaseAccess() when starting an AudioWorklet
4+
https://bugs.webkit.org/show_bug.cgi?id=219183
5+
<rdar://problem/71188544>
6+
7+
Reviewed by Mark Lam.
8+
9+
When audio rendering has already started when the AudioWorklet is constructed, it is possible for
10+
AudioWorkletGlobalScope::handlePreRenderTasks() to get called initially on the initial audio
11+
rendering thread instead of the audio worklet thread. Once the AudioWorklet is ready, the next
12+
rendering quantums will actually get processed on the audio worklet thread. However, there is a
13+
race when audio rendering has already started when the AudioWorklet gets created. This is not
14+
normally an issue. However, AudioWorkletGlobalScope::handlePreRenderTasks() grabs a JavaScript
15+
Lock and it is only safe to do so on the thread where we constructed the VM (i.e. the Audio
16+
Worklet thread). To address the issue, we now only grab the lock if we are on the audio worklet
17+
thread. Note that this lock is only used to delay the draining of the microtask queue until the
18+
end of the rendering quantum.
19+
20+
Test: webaudio/worklet-crash.html
21+
22+
* Modules/webaudio/AudioWorkletGlobalScope.cpp:
23+
(WebCore::AudioWorkletGlobalScope::handlePreRenderTasks):
24+
125
2020-11-19 Wenson Hsieh <wenson_hsieh@apple.com>
226

327
ASSERT NOT REACHED in WebCore::DisplayList::DrawImageBuffer::apply seen with TestWebKitAPI.DisplayListTests.ReplayWithMissingResource

‎Source/WebCore/Modules/webaudio/AudioWorkletGlobalScope.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,10 @@ void AudioWorkletGlobalScope::handlePreRenderTasks()
164164
{
165165
// We grab the JS API lock at the beginning of rendering and release it at the end of rendering.
166166
// This makes sure that we only drain the MicroTask queue after each render quantum.
167-
m_lockDuringRendering.emplace(script()->vm());
167+
// It is only safe to grab the lock if we are on the context thread. We might get called on
168+
// another thread if audio rendering started before the audio worklet got started.
169+
if (isContextThread())
170+
m_lockDuringRendering.emplace(script()->vm());
168171
}
169172

170173
void AudioWorkletGlobalScope::handlePostRenderTasks(size_t currentFrame)

0 commit comments

Comments
 (0)