- Revision
- 246578
- Author
- ysuz...@apple.com
- Date
- 2019-06-18 18:19:41 -0700 (Tue, 18 Jun 2019)
Log Message
[JSC] JSLock should be WebThread aware
https://bugs.webkit.org/show_bug.cgi?id=198911
Reviewed by Geoffrey Garen.
Source/_javascript_Core:
Since WebKitLegacy content rendering is done in WebThread instead of the main thread in iOS, user of WebKitLegacy (e.g. UIWebView) needs
to grab the WebThread lock (which is a recursive lock) in the main thread when touching the WebKitLegacy content.
But, WebKitLegacy can expose JSContext for the web view. And we can interact with the JS content through _javascript_Core APIs. However,
since WebThread is a concept in WebCore, _javascript_Core APIs do not grab the WebThread lock. As a result, WebKitLegacy web content can be
modified from the main thread without grabbing the WebThread lock through _javascript_Core APIs.
This patch makes JSC aware of WebThread: JSLock grabs the WebThread lock before grabbing JS's lock. While this seems layering violation,
we already have many USE(WEB_THREAD) and WebThread aware code in WTF. Eventually, we should move WebThread code from WebCore to WTF since
JSC and WTF need to be aware of WebThread. But, for now, we just use the function pointer exposed by WebCore.
Since both JSLock and the WebThread lock are recursive locks, nested locking is totally OK. The possible problem is the order of locking.
We ensure that we always grab locks in (1) the WebThread lock and (2) JSLock order.
In JSLock, we take the WebThread lock, but we do not unlock it. This is how we use the WebThread lock: the WebThread lock is released
automatically when RunLoop finishes the current cycle, and in WebKitLegacy, we do not call unlocking function of the WebThread lock except
for some edge cases.
* API/JSVirtualMachine.mm:
(-[JSVirtualMachine isWebThreadAware]):
* API/JSVirtualMachineInternal.h:
* runtime/JSLock.cpp:
(JSC::JSLockHolder::JSLockHolder):
(JSC::JSLock::lock):
(JSC::JSLockHolder::init): Deleted.
* runtime/JSLock.h:
(JSC::JSLock::makeWebThreadAware):
(JSC::JSLock::isWebThreadAware const):
Source/WebCore:
* bindings/js/CommonVM.cpp:
(WebCore::commonVMSlow):
Tools:
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm: Added.
(TestWebKitAPI::TEST):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/API/JSVirtualMachine.mm (246577 => 246578)
--- trunk/Source/_javascript_Core/API/JSVirtualMachine.mm 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Source/_javascript_Core/API/JSVirtualMachine.mm 2019-06-19 01:19:41 UTC (rev 246578)
@@ -248,6 +248,11 @@
return *toJS(m_group);
}
+- (BOOL)isWebThreadAware
+{
+ return [self vm].apiLock().isWebThreadAware();
+}
+
+ (void)setCrashOnVMCreation:(BOOL)shouldCrash
{
JSC::VM::setCrashOnVMCreation(shouldCrash);
Modified: trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h (246577 => 246578)
--- trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h 2019-06-19 01:19:41 UTC (rev 246578)
@@ -46,6 +46,8 @@
- (JSC::VM&)vm;
+- (BOOL)isWebThreadAware;
+
@end
#endif // defined(__OBJC__)
Modified: trunk/Source/_javascript_Core/ChangeLog (246577 => 246578)
--- trunk/Source/_javascript_Core/ChangeLog 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-06-19 01:19:41 UTC (rev 246578)
@@ -1,3 +1,38 @@
+2019-06-18 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] JSLock should be WebThread aware
+ https://bugs.webkit.org/show_bug.cgi?id=198911
+
+ Reviewed by Geoffrey Garen.
+
+ Since WebKitLegacy content rendering is done in WebThread instead of the main thread in iOS, user of WebKitLegacy (e.g. UIWebView) needs
+ to grab the WebThread lock (which is a recursive lock) in the main thread when touching the WebKitLegacy content.
+ But, WebKitLegacy can expose JSContext for the web view. And we can interact with the JS content through _javascript_Core APIs. However,
+ since WebThread is a concept in WebCore, _javascript_Core APIs do not grab the WebThread lock. As a result, WebKitLegacy web content can be
+ modified from the main thread without grabbing the WebThread lock through _javascript_Core APIs.
+
+ This patch makes JSC aware of WebThread: JSLock grabs the WebThread lock before grabbing JS's lock. While this seems layering violation,
+ we already have many USE(WEB_THREAD) and WebThread aware code in WTF. Eventually, we should move WebThread code from WebCore to WTF since
+ JSC and WTF need to be aware of WebThread. But, for now, we just use the function pointer exposed by WebCore.
+
+ Since both JSLock and the WebThread lock are recursive locks, nested locking is totally OK. The possible problem is the order of locking.
+ We ensure that we always grab locks in (1) the WebThread lock and (2) JSLock order.
+
+ In JSLock, we take the WebThread lock, but we do not unlock it. This is how we use the WebThread lock: the WebThread lock is released
+ automatically when RunLoop finishes the current cycle, and in WebKitLegacy, we do not call unlocking function of the WebThread lock except
+ for some edge cases.
+
+ * API/JSVirtualMachine.mm:
+ (-[JSVirtualMachine isWebThreadAware]):
+ * API/JSVirtualMachineInternal.h:
+ * runtime/JSLock.cpp:
+ (JSC::JSLockHolder::JSLockHolder):
+ (JSC::JSLock::lock):
+ (JSC::JSLockHolder::init): Deleted.
+ * runtime/JSLock.h:
+ (JSC::JSLock::makeWebThreadAware):
+ (JSC::JSLock::isWebThreadAware const):
+
2019-06-18 Justin Michaud <justin_mich...@apple.com>
[WASM-References] Add support for Table.size, grow and fill instructions
Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (246577 => 246578)
--- trunk/Source/_javascript_Core/runtime/JSLock.cpp 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp 2019-06-19 01:19:41 UTC (rev 246578)
@@ -35,6 +35,10 @@
#include <wtf/Threading.h>
#include <wtf/threads/Signals.h>
+#if USE(WEB_THREAD)
+#include <wtf/ios/WebCoreThread.h>
+#endif
+
namespace JSC {
Lock GlobalJSLock::s_sharedInstanceMutex;
@@ -50,25 +54,18 @@
}
JSLockHolder::JSLockHolder(ExecState* exec)
- : m_vm(&exec->vm())
+ : JSLockHolder(exec->vm())
{
- init();
}
JSLockHolder::JSLockHolder(VM* vm)
- : m_vm(vm)
+ : JSLockHolder(*vm)
{
- init();
}
JSLockHolder::JSLockHolder(VM& vm)
: m_vm(&vm)
{
- init();
-}
-
-void JSLockHolder::init()
-{
m_vm->apiLock().lock();
}
@@ -105,6 +102,13 @@
void JSLock::lock(intptr_t lockCount)
{
ASSERT(lockCount > 0);
+#if USE(WEB_THREAD)
+ if (m_isWebThreadAware) {
+ ASSERT(WebCoreWebThreadIsEnabled && WebCoreWebThreadIsEnabled());
+ WebCoreWebThreadLock();
+ }
+#endif
+
bool success = m_lock.tryLock();
if (UNLIKELY(!success)) {
if (currentThreadIsHoldingLock()) {
Modified: trunk/Source/_javascript_Core/runtime/JSLock.h (246577 => 246578)
--- trunk/Source/_javascript_Core/runtime/JSLock.h 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Source/_javascript_Core/runtime/JSLock.h 2019-06-19 01:19:41 UTC (rev 246578)
@@ -70,9 +70,8 @@
JS_EXPORT_PRIVATE JSLockHolder(ExecState*);
JS_EXPORT_PRIVATE ~JSLockHolder();
+
private:
- void init();
-
RefPtr<VM> m_vm;
};
@@ -119,6 +118,13 @@
unsigned m_dropDepth;
};
+ void makeWebThreadAware()
+ {
+ m_isWebThreadAware = true;
+ }
+
+ bool isWebThreadAware() const { return m_isWebThreadAware; }
+
private:
void lock(intptr_t lockCount);
void unlock(intptr_t unlockCount);
@@ -130,6 +136,7 @@
void grabAllLocks(DropAllLocks*, unsigned lockCount);
Lock m_lock;
+ bool m_isWebThreadAware { false };
// We cannot make m_ownerThread an optional (instead of pairing it with an explicit
// m_hasOwnerThread) because currentThreadIsHoldingLock() may be called from a
// different thread, and an optional is vulnerable to races.
Modified: trunk/Source/WebCore/ChangeLog (246577 => 246578)
--- trunk/Source/WebCore/ChangeLog 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Source/WebCore/ChangeLog 2019-06-19 01:19:41 UTC (rev 246578)
@@ -1,3 +1,13 @@
+2019-06-18 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] JSLock should be WebThread aware
+ https://bugs.webkit.org/show_bug.cgi?id=198911
+
+ Reviewed by Geoffrey Garen.
+
+ * bindings/js/CommonVM.cpp:
+ (WebCore::commonVMSlow):
+
2019-06-18 Joseph Pecoraro <pecor...@apple.com>
WebSocketDeflater uses an unnecessarily constrained compression memory level
Modified: trunk/Source/WebCore/bindings/js/CommonVM.cpp (246577 => 246578)
--- trunk/Source/WebCore/bindings/js/CommonVM.cpp 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Source/WebCore/bindings/js/CommonVM.cpp 2019-06-19 01:19:41 UTC (rev 246578)
@@ -59,6 +59,8 @@
vm.heap.acquireAccess(); // At any time, we may do things that affect the GC.
#if PLATFORM(IOS_FAMILY)
+ if (WebThreadIsEnabled())
+ vm.apiLock().makeWebThreadAware();
vm.setRunLoop(WebThreadRunLoop());
vm.heap.machineThreads().addCurrentThread();
#endif
Modified: trunk/Tools/ChangeLog (246577 => 246578)
--- trunk/Tools/ChangeLog 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Tools/ChangeLog 2019-06-19 01:19:41 UTC (rev 246578)
@@ -1,3 +1,14 @@
+2019-06-18 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] JSLock should be WebThread aware
+ https://bugs.webkit.org/show_bug.cgi?id=198911
+
+ Reviewed by Geoffrey Garen.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm: Added.
+ (TestWebKitAPI::TEST):
+
2019-06-18 Keith Miller <keith_mil...@apple.com>
webkit-patch should allow for a bugzilla url not just bugzilla id
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (246577 => 246578)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2019-06-19 01:18:43 UTC (rev 246577)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2019-06-19 01:19:41 UTC (rev 246578)
@@ -875,6 +875,7 @@
E194E1BD177E53C7009C4D4E /* StopLoadingFromDidReceiveResponse.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = E194E1BC177E534A009C4D4E /* StopLoadingFromDidReceiveResponse.html */; };
E324A6F02041C82000A76593 /* UniqueArray.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E398BC0F2041C76300387136 /* UniqueArray.cpp */; };
E32B549222810AC4008AD702 /* Packed.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E32B549122810AC0008AD702 /* Packed.cpp */; };
+ E35FC7B222B82A7300F32F98 /* JSLockTakesWebThreadLock.mm in Sources */ = {isa = PBXBuildFile; fileRef = E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */; };
E373D7911F2CF35200C6FAAF /* Signals.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E3953F951F2CF32100A76A2E /* Signals.cpp */; };
E38A0D351FD50CC300E98C8B /* Threading.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E38A0D341FD50CBC00E98C8B /* Threading.cpp */; };
E3A1E77F21B25B39008C6007 /* URLParserTextEncoding.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E3A1E77E21B25B39008C6007 /* URLParserTextEncoding.cpp */; };
@@ -2272,6 +2273,7 @@
E194E1BC177E534A009C4D4E /* StopLoadingFromDidReceiveResponse.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = StopLoadingFromDidReceiveResponse.html; sourceTree = "<group>"; };
E19DB9781B32137C00DB38D4 /* NavigatorLanguage.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = NavigatorLanguage.mm; sourceTree = "<group>"; };
E32B549122810AC0008AD702 /* Packed.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = Packed.cpp; sourceTree = "<group>"; };
+ E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = JSLockTakesWebThreadLock.mm; sourceTree = "<group>"; };
E388887020C9098100E632BC /* WorkerPool.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkerPool.cpp; sourceTree = "<group>"; };
E38A0D341FD50CBC00E98C8B /* Threading.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Threading.cpp; sourceTree = "<group>"; };
E3953F951F2CF32100A76A2E /* Signals.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Signals.cpp; sourceTree = "<group>"; };
@@ -3807,6 +3809,7 @@
children = (
CDC8E49A1BC728FE00594FEC /* Resources */,
CDC8E4851BC5B19400594FEC /* AudioSessionCategoryIOS.mm */,
+ E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */,
CDC0932A21C872C10030C4B0 /* ScrollingDoesNotPauseMedia.mm */,
0F4FFA9D1ED3AA8500F7111F /* SnapshotViaRenderInContext.mm */,
);
@@ -4303,6 +4306,7 @@
7CCE7EAD1A411A3400447C4C /* _javascript_Test.cpp in Sources */,
7CCE7EA51A411A0800447C4C /* _javascript_TestMac.mm in Sources */,
5C0160C121A132460077FA32 /* JITEnabled.mm in Sources */,
+ E35FC7B222B82A7300F32F98 /* JSLockTakesWebThreadLock.mm in Sources */,
7CCE7EC41A411A7E00447C4C /* JSWrapperForNodeInWebFrame.mm in Sources */,
F45E15732112CE2900307E82 /* KeyboardInputTestsIOS.mm in Sources */,
7CCE7F061A411AE600447C4C /* LayoutMilestonesWithAllContentInFrame.cpp in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm (0 => 246578)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm 2019-06-19 01:19:41 UTC (rev 246578)
@@ -0,0 +1,64 @@
+/*
+ * 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 COMPUTER, 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 COMPUTER, 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.
+ */
+
+#import "config.h"
+
+#if PLATFORM(IOS_FAMILY)
+
+#import "PlatformUtilities.h"
+#import <_javascript_Core/JSVirtualMachine.h>
+#import <_javascript_Core/JSVirtualMachineInternal.h>
+#import <UIKit/UIKit.h>
+#import <WebCore/WebCoreThread.h>
+#import <stdlib.h>
+#import <wtf/DataLog.h>
+#import <wtf/RetainPtr.h>
+
+namespace TestWebKitAPI {
+
+TEST(WebKitLegacy, JSLockTakesWebThreadLock)
+{
+ RetainPtr<UIWindow> uiWindow = adoptNS([[UIWindow alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+ RetainPtr<UIWebView> uiWebView = adoptNS([[UIWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+ [uiWindow addSubview:uiWebView.get()];
+ [uiWebView loadHTMLString:@"<p>WebThreadLock and JSLock</p>" baseURL:nil];
+ RetainPtr<JSContext> jsContext = [uiWebView valueForKeyPath:@"documentView.webView.mainFrame._javascript_Context"];
+
+ EXPECT_TRUE(!!jsContext.get());
+
+ RetainPtr<JSVirtualMachine> jsVM = [jsContext virtualMachine];
+ EXPECT_TRUE([jsVM isWebThreadAware]);
+
+ // Release WebThread lock.
+ Util::spinRunLoop(1);
+
+ EXPECT_FALSE(WebThreadIsLocked());
+ // XMLHttpRequest has Timer, which has thread afinity.
+ [jsContext evaluateScript:@"for (var i = 0; i < 1e3; ++i) { var request = new XMLHttpRequest; request.property = new Array(10000); }"];
+}
+
+}
+
+#endif
