AbortSignal does not always emit the abort signal
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Oct 2019 16:31:21 +0000 (16:31 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Oct 2019 16:31:21 +0000 (16:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201871
<rdar://problem/55451712>

Reviewed by Chris Dumez.

Source/WebCore:

Make sure a JSAbortSignal is not GCed until objects that can abort it are gone.
This includes a followed signal and an AbortController.
Current WebKit implementation only uses following of one signal at a time.

Test: http/tests/fetch/abort-signal-gc.html

* Sources.txt:
* WebCore.xcodeproj/project.pbxproj:
* bindings/js/JSAbortControllerCustom.cpp: Added.
(WebCore::JSAbortController::visitAdditionalChildren):
* bindings/js/JSAbortSignalCustom.cpp: Added.
(WebCore::JSAbortSignalOwner::isReachableFromOpaqueRoots):
* bindings/js/JSTypedOMCSSStyleValueCustom.cpp:
* dom/AbortController.idl:
* dom/AbortSignal.cpp:
(WebCore::AbortSignal::follow):
* dom/AbortSignal.h:
* dom/AbortSignal.idl:

LayoutTests:

* http/tests/fetch/abort-signal-gc-expected.txt: Added.
* http/tests/fetch/abort-signal-gc.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250727 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/fetch/abort-signal-gc-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/fetch/abort-signal-gc.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Sources.txt
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/bindings/js/JSAbortControllerCustom.cpp [new file with mode: 0644]
Source/WebCore/bindings/js/JSAbortSignalCustom.cpp [new file with mode: 0644]
Source/WebCore/bindings/js/JSTypedOMCSSStyleValueCustom.cpp
Source/WebCore/dom/AbortController.idl
Source/WebCore/dom/AbortSignal.cpp
Source/WebCore/dom/AbortSignal.h
Source/WebCore/dom/AbortSignal.idl

index d005457..26b331e 100644 (file)
@@ -1,5 +1,16 @@
 2019-10-04  youenn fablet  <youenn@apple.com>
 
+        AbortSignal does not always emit the abort signal
+        https://bugs.webkit.org/show_bug.cgi?id=201871
+        <rdar://problem/55451712>
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/fetch/abort-signal-gc-expected.txt: Added.
+        * http/tests/fetch/abort-signal-gc.html: Added.
+
+2019-10-04  youenn fablet  <youenn@apple.com>
+
         Allow to suspend RTCPeerConnection when not connected
         https://bugs.webkit.org/show_bug.cgi?id=202403
 
diff --git a/LayoutTests/http/tests/fetch/abort-signal-gc-expected.txt b/LayoutTests/http/tests/fetch/abort-signal-gc-expected.txt
new file mode 100644 (file)
index 0000000..a8fcf06
--- /dev/null
@@ -0,0 +1,4 @@
+
+PASS GC should collect a signal after its controller 
+PASS GC should collect a signal after its request 
+
diff --git a/LayoutTests/http/tests/fetch/abort-signal-gc.html b/LayoutTests/http/tests/fetch/abort-signal-gc.html
new file mode 100644 (file)
index 0000000..ba233ef
--- /dev/null
@@ -0,0 +1,44 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Abort Signal GC</title>
+        <script src="/js-test-resources/gc.js"></script>
+        <script src="/resources/testharness.js"></script>
+        <script src="/resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <script>
+function testAbortControllerSignal(callback)
+{
+     const controller = new AbortController();
+     controller.signal.addEventListener("abort", callback);
+     setTimeout(() => controller.abort(), 100);
+}
+
+promise_test(() => {
+    let resolve;
+    const promise = new Promise(r => resolve = r);
+    testAbortControllerSignal(resolve);
+    window.gc();
+    return promise;
+}, "GC should collect a signal after its controller");
+
+function testFetchRequestSignal(callback)
+{
+     const controller = new AbortController();
+     const request = new Request("/", {signal : controller.signal});
+     request.signal.addEventListener("abort", callback);
+     setTimeout(() => controller.abort(), 100);
+}
+
+promise_test(() => {
+    let resolve;
+    const promise = new Promise(r => resolve = r);
+    testFetchRequestSignal(resolve);
+    window.gc();
+    return promise;
+}, "GC should collect a signal after its request");
+        </script>
+    </body>
+</html>
index fa4ccfd..2bf7d44 100644 (file)
@@ -1,5 +1,32 @@
 2019-10-04  youenn fablet  <youenn@apple.com>
 
+        AbortSignal does not always emit the abort signal
+        https://bugs.webkit.org/show_bug.cgi?id=201871
+        <rdar://problem/55451712>
+
+        Reviewed by Chris Dumez.
+
+        Make sure a JSAbortSignal is not GCed until objects that can abort it are gone.
+        This includes a followed signal and an AbortController.
+        Current WebKit implementation only uses following of one signal at a time.
+
+        Test: http/tests/fetch/abort-signal-gc.html
+
+        * Sources.txt:
+        * WebCore.xcodeproj/project.pbxproj:
+        * bindings/js/JSAbortControllerCustom.cpp: Added.
+        (WebCore::JSAbortController::visitAdditionalChildren):
+        * bindings/js/JSAbortSignalCustom.cpp: Added.
+        (WebCore::JSAbortSignalOwner::isReachableFromOpaqueRoots):
+        * bindings/js/JSTypedOMCSSStyleValueCustom.cpp:
+        * dom/AbortController.idl:
+        * dom/AbortSignal.cpp:
+        (WebCore::AbortSignal::follow):
+        * dom/AbortSignal.h:
+        * dom/AbortSignal.idl:
+
+2019-10-04  youenn fablet  <youenn@apple.com>
+
         Allow to suspend RTCPeerConnection when not connected
         https://bugs.webkit.org/show_bug.cgi?id=202403
 
index 9c171e5..264653c 100644 (file)
@@ -453,6 +453,8 @@ bindings/js/DOMGCOutputConstraint.cpp
 bindings/js/DOMWrapperWorld.cpp
 bindings/js/GCController.cpp
 bindings/js/IDBBindingUtilities.cpp
+bindings/js/JSAbortControllerCustom.cpp
+bindings/js/JSAbortSignalCustom.cpp
 bindings/js/JSAuthenticatorResponseCustom.cpp
 bindings/js/JSAnimationEffectCustom.cpp
 bindings/js/JSAnimationTimelineCustom.cpp
index 08a4692..45c1a30 100644 (file)
                3FFFF9AC159D9B060020BBD5 /* ViewportStyleResolver.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ViewportStyleResolver.h; sourceTree = "<group>"; };
                410626A72280A22A006D1B59 /* JSAudioNodeCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSAudioNodeCustom.cpp; sourceTree = "<group>"; };
                4107908A1FC3E4F20061B27A /* ClientOrigin.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ClientOrigin.h; sourceTree = "<group>"; };
+               410938282347799A009428BA /* JSAbortControllerCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSAbortControllerCustom.cpp; sourceTree = "<group>"; };
+               4109382C2347850E009428BA /* JSAbortSignalCustom.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = JSAbortSignalCustom.cpp; sourceTree = "<group>"; };
                410B7E711045FAB000D8224F /* JSMessageEventCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSMessageEventCustom.cpp; sourceTree = "<group>"; };
                410E445F234373AD000173D4 /* LibWebRTCSocketIdentifier.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = LibWebRTCSocketIdentifier.h; path = libwebrtc/LibWebRTCSocketIdentifier.h; sourceTree = "<group>"; };
                41103AA71E39790A00769F03 /* RealtimeOutgoingAudioSource.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RealtimeOutgoingAudioSource.cpp; sourceTree = "<group>"; };
                7C3D8EE41E08BABE0023B084 /* GC / Wrapping Only */ = {
                        isa = PBXGroup;
                        children = (
+                               410938282347799A009428BA /* JSAbortControllerCustom.cpp */,
+                               4109382C2347850E009428BA /* JSAbortSignalCustom.cpp */,
                                71EFCEDE202B39C700D7C411 /* JSAnimationEffectCustom.cpp */,
                                71025ED51F99F147004A250C /* JSAnimationTimelineCustom.cpp */,
                                BC2ED6BB0C6BD2F000920BFF /* JSAttrCustom.cpp */,
                                E34EE49F1DC2D57500EAA9D3 /* JSEventCustom.h */,
                                BC60901E0E91B8EC000C68B5 /* JSEventTargetCustom.cpp */,
                                46B63F6B1C6E8CDF002E914B /* JSEventTargetCustom.h */,
+                               4131F3B11F9552810059995A /* JSFetchEventCustom.cpp */,
                                835B680E1F548BDE0071F7F6 /* JSFileSystemEntryCustom.cpp */,
                                BCCBAD3A0C18BFF800CE890F /* JSHTMLCollectionCustom.cpp */,
                                BC51580A0C03D404008BB0EE /* JSHTMLDocumentCustom.cpp */,
                        children = (
                                DEC2975D1B4DEB2A005F5945 /* JSCustomEventCustom.cpp */,
                                831C46C31F9EE5E000EBD450 /* JSExtendableMessageEventCustom.cpp */,
-                               4131F3B11F9552810059995A /* JSFetchEventCustom.cpp */,
                                BCE7B1920D4E86960075A539 /* JSHistoryCustom.cpp */,
                                CA6C1537220D1EB30055CBFC /* JSIDBCursorCustom.cpp */,
                                CA6C1538220D1EB30055CBFC /* JSIDBCursorWithValueCustom.cpp */,
diff --git a/Source/WebCore/bindings/js/JSAbortControllerCustom.cpp b/Source/WebCore/bindings/js/JSAbortControllerCustom.cpp
new file mode 100644 (file)
index 0000000..b91e3c7
--- /dev/null
@@ -0,0 +1,36 @@
+/*
+* Copyright (C) 2019 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. ``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
+* 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 "JSAbortController.h"
+
+namespace WebCore {
+
+void JSAbortController::visitAdditionalChildren(JSC::SlotVisitor& visitor)
+{
+    visitor.addOpaqueRoot(&wrapped().signal());
+}
+
+} // namespace WebCore
diff --git a/Source/WebCore/bindings/js/JSAbortSignalCustom.cpp b/Source/WebCore/bindings/js/JSAbortSignalCustom.cpp
new file mode 100644 (file)
index 0000000..3dad637
--- /dev/null
@@ -0,0 +1,49 @@
+/*
+* Copyright (C) 2019 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. ``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
+* 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 "JSAbortSignal.h"
+
+namespace WebCore {
+
+bool JSAbortSignalOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, JSC::SlotVisitor& visitor, const char** reason)
+{
+    auto& abortSignal = JSC::jsCast<JSAbortSignal*>(handle.slot()->asCell())->wrapped();
+    if (abortSignal.isFiringEventListeners()) {
+        if (UNLIKELY(reason))
+            *reason = "EventTarget firing event listeners";
+        return true;
+    }
+
+    if (abortSignal.aborted())
+        return false;
+
+    if (abortSignal.isFollowingSignal())
+        return true;
+
+    return visitor.containsOpaqueRoot(&abortSignal);
+}
+
+} // namespace WebCore
index 135aaee..bf26e80 100644 (file)
@@ -28,6 +28,7 @@
 
 #if ENABLE(CSS_TYPED_OM)
 
+#include "JSDOMWrapperCache.h"
 #include "JSTypedOMCSSImageValue.h"
 #include "JSTypedOMCSSUnitValue.h"
 #include "JSTypedOMCSSUnparsedValue.h"
index 2971c54..7f4ce59 100644 (file)
@@ -27,7 +27,8 @@
     Constructor,
     ConstructorCallWith=ScriptExecutionContext,
     Exposed=(Window,Worker),
-    ImplementationLacksVTable
+    ImplementationLacksVTable,
+    JSCustomMarkFunction
 ] interface AbortController {
     [SameObject] readonly attribute AbortSignal signal;
 
index 4a7f7c6..4c75fd1 100644 (file)
@@ -75,6 +75,8 @@ void AbortSignal::follow(AbortSignal& signal)
         return;
     }
 
+    ASSERT(!m_followingSignal);
+    m_followingSignal = makeWeakPtr(signal);
     signal.addAlgorithm([weakThis = makeWeakPtr(this)] {
         if (!weakThis)
             return;
index cc05913..d42de67 100644 (file)
@@ -53,6 +53,8 @@ public:
 
     void follow(AbortSignal&);
 
+    bool isFollowingSignal() const { return !!m_followingSignal; }
+
 private:
     explicit AbortSignal(ScriptExecutionContext&);
 
@@ -63,6 +65,7 @@ private:
     
     bool m_aborted { false };
     Vector<Algorithm> m_algorithms;
+    WeakPtr<AbortSignal> m_followingSignal;
 };
 
 }
index bb7d49d..5fd9963 100644 (file)
@@ -24,6 +24,7 @@
  */
 
 [
+    CustomIsReachable,
     Exposed=(Window,Worker)
 ] interface AbortSignal : EventTarget {
     readonly attribute boolean aborted;