- Revision
- 283402
- Author
- cdu...@apple.com
- Date
- 2021-10-01 14:24:36 -0700 (Fri, 01 Oct 2021)
Log Message
GC may not know about memory used by AudioBufferSourceNode's buffer
https://bugs.webkit.org/show_bug.cgi?id=230966
Reviewed by Darin Adler.
Source/WebCore:
AudioBufferSourceNode has a 'buffer' attribute settable by _javascript_ of type AudioBuffer.
An AudioBuffer may use a significant amount of memory and AudioBuffer.idl contains [ReportExtraMemoryCost]
extended attribute to report this memory to the GC. However, there is an issue if JS constructs a large
AudioBuffer, then sets it on an AudioBufferSourceNode and then the JSAudioBuffer wrapper gets garbage
collected. At this point, GC thinks it recovered the extra memory reported by the JSAudioBuffer but this
is not true since that AudioBuffer (and its internal memory) are still kept alive by AudioBufferSourceNode /
JSAudioBufferSourceNode. To address this issue, the proposal in this patch is to have JSAudioBufferSourceNode
mark its buffer when visiting childen during GC. As a result, the JSAudioBuffer wrapper will stay alive (and
report extra memory use) as long as its associated JSAudioBufferSourceNode wrapper is still alive.
Test: webaudio/AudioBufferSource/audiobuffersource-buffer-gc.html
* Modules/webaudio/AudioBuffer.idl:
* Modules/webaudio/AudioBufferSourceNode.h:
* Modules/webaudio/AudioBufferSourceNode.idl:
* Sources.txt:
* bindings/js/JSAudioBufferSourceNodeCustom.cpp: Added.
(WebCore::JSAudioBufferSourceNode::visitAdditionalChildren):
LayoutTests:
Add layout test coverage.
* webaudio/AudioBufferSource/audiobuffersource-buffer-gc-expected.txt: Added.
* webaudio/AudioBufferSource/audiobuffersource-buffer-gc.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (283401 => 283402)
--- trunk/LayoutTests/ChangeLog 2021-10-01 21:14:56 UTC (rev 283401)
+++ trunk/LayoutTests/ChangeLog 2021-10-01 21:24:36 UTC (rev 283402)
@@ -1,3 +1,15 @@
+2021-10-01 Chris Dumez <cdu...@apple.com>
+
+ GC may not know about memory used by AudioBufferSourceNode's buffer
+ https://bugs.webkit.org/show_bug.cgi?id=230966
+
+ Reviewed by Darin Adler.
+
+ Add layout test coverage.
+
+ * webaudio/AudioBufferSource/audiobuffersource-buffer-gc-expected.txt: Added.
+ * webaudio/AudioBufferSource/audiobuffersource-buffer-gc.html: Added.
+
2021-10-01 Ayumi Kojima <ayumi_koj...@apple.com>
[ BigSur wk1 ] printing/css2.1/page-break-after-000.html is a flaky failure.
Added: trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-buffer-gc-expected.txt (0 => 283402)
--- trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-buffer-gc-expected.txt (rev 0)
+++ trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-buffer-gc-expected.txt 2021-10-01 21:24:36 UTC (rev 283402)
@@ -0,0 +1,11 @@
+Make sure that AudioBufferSourceNode.buffer keeps returning the same object
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS audioBufferSource.buffer.foo is 1
+PASS audioBufferSource.buffer.foo is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-buffer-gc.html (0 => 283402)
--- trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-buffer-gc.html (rev 0)
+++ trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-buffer-gc.html 2021-10-01 21:24:36 UTC (rev 283402)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Make sure that AudioBufferSourceNode.buffer keeps returning the same object");
+jsTestIsAsync = true;
+
+let context = new AudioContext();
+let audioBufferSource = context.createBufferSource();
+audioBufferSource.buffer = context.createBuffer(1, 1024, context.sampleRate);
+audioBufferSource.buffer.foo = 1;
+gc();
+setTimeout(() => {
+ gc();
+ shouldBe("audioBufferSource.buffer.foo", "1");
+ setTimeout(() => {
+ gc();
+ shouldBe("audioBufferSource.buffer.foo", "1");
+ finishJSTest();
+ }, 0);
+}, 0);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (283401 => 283402)
--- trunk/Source/WebCore/ChangeLog 2021-10-01 21:14:56 UTC (rev 283401)
+++ trunk/Source/WebCore/ChangeLog 2021-10-01 21:24:36 UTC (rev 283402)
@@ -1,3 +1,29 @@
+2021-10-01 Chris Dumez <cdu...@apple.com>
+
+ GC may not know about memory used by AudioBufferSourceNode's buffer
+ https://bugs.webkit.org/show_bug.cgi?id=230966
+
+ Reviewed by Darin Adler.
+
+ AudioBufferSourceNode has a 'buffer' attribute settable by _javascript_ of type AudioBuffer.
+ An AudioBuffer may use a significant amount of memory and AudioBuffer.idl contains [ReportExtraMemoryCost]
+ extended attribute to report this memory to the GC. However, there is an issue if JS constructs a large
+ AudioBuffer, then sets it on an AudioBufferSourceNode and then the JSAudioBuffer wrapper gets garbage
+ collected. At this point, GC thinks it recovered the extra memory reported by the JSAudioBuffer but this
+ is not true since that AudioBuffer (and its internal memory) are still kept alive by AudioBufferSourceNode /
+ JSAudioBufferSourceNode. To address this issue, the proposal in this patch is to have JSAudioBufferSourceNode
+ mark its buffer when visiting childen during GC. As a result, the JSAudioBuffer wrapper will stay alive (and
+ report extra memory use) as long as its associated JSAudioBufferSourceNode wrapper is still alive.
+
+ Test: webaudio/AudioBufferSource/audiobuffersource-buffer-gc.html
+
+ * Modules/webaudio/AudioBuffer.idl:
+ * Modules/webaudio/AudioBufferSourceNode.h:
+ * Modules/webaudio/AudioBufferSourceNode.idl:
+ * Sources.txt:
+ * bindings/js/JSAudioBufferSourceNodeCustom.cpp: Added.
+ (WebCore::JSAudioBufferSourceNode::visitAdditionalChildren):
+
2021-10-01 Myles C. Maxfield <mmaxfi...@apple.com>
base-palette can accept "light" or "dark"
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBuffer.idl (283401 => 283402)
--- trunk/Source/WebCore/Modules/webaudio/AudioBuffer.idl 2021-10-01 21:14:56 UTC (rev 283401)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBuffer.idl 2021-10-01 21:24:36 UTC (rev 283402)
@@ -28,6 +28,7 @@
[
Conditional=WEB_AUDIO,
+ GenerateIsReachable=Impl,
ImplementationLacksVTable,
JSCustomMarkFunction,
JSGenerateToJSObject,
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h (283401 => 283402)
--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h 2021-10-01 21:14:56 UTC (rev 283401)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h 2021-10-01 21:24:36 UTC (rev 283402)
@@ -52,6 +52,9 @@
// setBufferForBindings() is called on the main thread. This is the buffer we use for playback.
ExceptionOr<void> setBufferForBindings(RefPtr<AudioBuffer>&&);
+ AudioBuffer* buffer() WTF_REQUIRES_LOCK(m_processLock) { return m_buffer.get(); }
+ Lock& processLock() WTF_RETURNS_LOCK(m_processLock) { return m_processLock; }
+
// This function does not lock before accessing the buffer and should therefore only be called on the main thread.
AudioBuffer* bufferForBindings() WTF_IGNORES_THREAD_SAFETY_ANALYSIS { ASSERT(isMainThread()); return m_buffer.get(); }
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.idl (283401 => 283402)
--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.idl 2021-10-01 21:14:56 UTC (rev 283401)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.idl 2021-10-01 21:24:36 UTC (rev 283402)
@@ -26,6 +26,7 @@
// A cached (non-streamed), memory-resident audio source
[
Conditional=WEB_AUDIO,
+ JSCustomMarkFunction,
JSGenerateToJSObject,
Exposed=Window
] interface AudioBufferSourceNode : AudioScheduledSourceNode {
Modified: trunk/Source/WebCore/Sources.txt (283401 => 283402)
--- trunk/Source/WebCore/Sources.txt 2021-10-01 21:14:56 UTC (rev 283401)
+++ trunk/Source/WebCore/Sources.txt 2021-10-01 21:24:36 UTC (rev 283402)
@@ -442,6 +442,7 @@
bindings/js/JSAnimationTimelineCustom.cpp
bindings/js/JSAttrCustom.cpp
bindings/js/JSAudioBufferCustom.cpp
+bindings/js/JSAudioBufferSourceNodeCustom.cpp
bindings/js/JSAudioNodeCustom.cpp
bindings/js/JSAudioWorkletProcessorCustom.cpp
bindings/js/JSAuthenticatorResponseCustom.cpp
Added: trunk/Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp (0 => 283402)
--- trunk/Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp (rev 0)
+++ trunk/Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp 2021-10-01 21:24:36 UTC (rev 283402)
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2017-2021 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "JSAudioBufferSourceNode.h"
+
+#if ENABLE(WEB_AUDIO)
+#include "AudioBufferSourceNode.h"
+
+namespace WebCore {
+
+using namespace JSC;
+
+template<typename Visitor>
+void JSAudioBufferSourceNode::visitAdditionalChildren(Visitor& visitor)
+{
+ Locker locker { wrapped().processLock() };
+ // The AudioBufferSourceNode's buffer may hold on to a large amount of memory. This memory is
+ // reported to GC via the JSAudioBuffer wrapper so we need to make sure that the buffer's
+ // wrapper stays alive as long as the buffer is used by the AudioBufferSourceNode.
+ visitor.addOpaqueRoot(wrapped().buffer());
+}
+
+DEFINE_VISIT_ADDITIONAL_CHILDREN(JSAudioBufferSourceNode);
+
+} // namespace WebCore
+
+#endif // ENABLE(WEB_AUDIO)
