⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 246578 in webkit


Ignore:
Timestamp:
Jun 18, 2019, 6:19:41 PM (7 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] JSLock should be WebThread aware
​https://bugs.webkit.org/show_bug.cgi?id=198911

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

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 JavaScriptCore APIs. However,
since WebThread is a concept in WebCore, JavaScriptCore 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 JavaScriptCore 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):

Location:
trunk
Files:
1 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/API/JSVirtualMachine.mm

    r243617 r246578  
    249249}
    250250
     251- (BOOL)isWebThreadAware
     252{
     253    return [self vm].apiLock().isWebThreadAware();
     254}
     255
    251256+ (void)setCrashOnVMCreation:(BOOL)shouldCrash
    252257{

  • trunk/Source/JavaScriptCore/API/JSVirtualMachineInternal.h

    r243617 r246578  
    4747- (JSC::VM&)vm;
    4848
     49- (BOOL)isWebThreadAware;
     50
    4951@end
    5052

  • trunk/Source/JavaScriptCore/ChangeLog

    r246577 r246578  
     12019-06-18  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] JSLock should be WebThread aware
     4        https://bugs.webkit.org/show_bug.cgi?id=198911
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Since WebKitLegacy content rendering is done in WebThread instead of the main thread in iOS, user of WebKitLegacy (e.g. UIWebView) needs
     9        to grab the WebThread lock (which is a recursive lock) in the main thread when touching the WebKitLegacy content.
     10        But, WebKitLegacy can expose JSContext for the web view. And we can interact with the JS content through JavaScriptCore APIs. However,
     11        since WebThread is a concept in WebCore, JavaScriptCore APIs do not grab the WebThread lock. As a result, WebKitLegacy web content can be
     12        modified from the main thread without grabbing the WebThread lock through JavaScriptCore APIs.
     13
     14        This patch makes JSC aware of WebThread: JSLock grabs the WebThread lock before grabbing JS's lock. While this seems layering violation,
     15        we already have many USE(WEB_THREAD) and WebThread aware code in WTF. Eventually, we should move WebThread code from WebCore to WTF since
     16        JSC and WTF need to be aware of WebThread. But, for now, we just use the function pointer exposed by WebCore.
     17
     18        Since both JSLock and the WebThread lock are recursive locks, nested locking is totally OK. The possible problem is the order of locking.
     19        We ensure that we always grab locks in (1) the WebThread lock and (2) JSLock order.
     20
     21        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
     22        automatically when RunLoop finishes the current cycle, and in WebKitLegacy, we do not call unlocking function of the WebThread lock except
     23        for some edge cases.
     24
     25        * API/JSVirtualMachine.mm:
     26        (-[JSVirtualMachine isWebThreadAware]):
     27        * API/JSVirtualMachineInternal.h:
     28        * runtime/JSLock.cpp:
     29        (JSC::JSLockHolder::JSLockHolder):
     30        (JSC::JSLock::lock):
     31        (JSC::JSLockHolder::init): Deleted.
     32        * runtime/JSLock.h:
     33        (JSC::JSLock::makeWebThreadAware):
     34        (JSC::JSLock::isWebThreadAware const):
     35
    1362019-06-18  Justin Michaud  <justin_michaud@apple.com>
    237

  • trunk/Source/JavaScriptCore/runtime/JSLock.cpp

    r246490 r246578  
    3636#include <wtf/threads/Signals.h>
    3737
     38#if USE(WEB_THREAD)
     39#include <wtf/ios/WebCoreThread.h>
     40#endif
     41
    3842namespace JSC {
    3943
    … …  
    5155
    5256JSLockHolder::JSLockHolder(ExecState* exec)
    53     : m_vm(&exec->vm())
    54 {
    55     init();
     57    : JSLockHolder(exec->vm())
     58{
    5659}
    5760
    5861JSLockHolder::JSLockHolder(VM* vm)
    59     : m_vm(vm)
    60 {
    61     init();
     62    : JSLockHolder(*vm)
     63{
    6264}
    6365
    6466JSLockHolder::JSLockHolder(VM& vm)
    6567    : m_vm(&vm)
    66 {
    67     init();
    68 }
    69 
    70 void JSLockHolder::init()
    7168{
    7269    m_vm->apiLock().lock();
    … …  
    106103{
    107104    ASSERT(lockCount > 0);
     105#if USE(WEB_THREAD)
     106    if (m_isWebThreadAware) {
     107        ASSERT(WebCoreWebThreadIsEnabled && WebCoreWebThreadIsEnabled());
     108        WebCoreWebThreadLock();
     109    }
     110#endif
     111
    108112    bool success = m_lock.tryLock();
    109113    if (UNLIKELY(!success)) {

  • trunk/Source/JavaScriptCore/runtime/JSLock.h

    r246490 r246578  
    7171
    7272    JS_EXPORT_PRIVATE ~JSLockHolder();
     73
    7374private:
    74     void init();
    75 
    7675    RefPtr<VM> m_vm;
    7776};
    … …  
    120119    };
    121120
     121    void makeWebThreadAware()
     122    {
     123        m_isWebThreadAware = true;
     124    }
     125
     126    bool isWebThreadAware() const { return m_isWebThreadAware; }
     127
    122128private:
    123129    void lock(intptr_t lockCount);
    … …  
    131137
    132138    Lock m_lock;
     139    bool m_isWebThreadAware { false };
    133140    // We cannot make m_ownerThread an optional (instead of pairing it with an explicit
    134141    // m_hasOwnerThread) because currentThreadIsHoldingLock() may be called from a

  • trunk/Source/WebCore/ChangeLog

    r246573 r246578  
     12019-06-18  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] JSLock should be WebThread aware
     4        https://bugs.webkit.org/show_bug.cgi?id=198911
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        * bindings/js/CommonVM.cpp:
     9        (WebCore::commonVMSlow):
     10
    1112019-06-18  Joseph Pecoraro  <pecoraro@apple.com>
    212

  • trunk/Source/WebCore/bindings/js/CommonVM.cpp

    r246490 r246578  
    6060
    6161#if PLATFORM(IOS_FAMILY)
     62    if (WebThreadIsEnabled())
     63        vm.apiLock().makeWebThreadAware();
    6264    vm.setRunLoop(WebThreadRunLoop());
    6365    vm.heap.machineThreads().addCurrentThread();

  • trunk/Tools/ChangeLog

    r246566 r246578  
     12019-06-18  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] JSLock should be WebThread aware
     4        https://bugs.webkit.org/show_bug.cgi?id=198911
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     9        * TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm: Added.
     10        (TestWebKitAPI::TEST):
     11
    1122019-06-18  Keith Miller  <keith_miller@apple.com>
    213

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r246514 r246578  
    876876                E324A6F02041C82000A76593 /* UniqueArray.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E398BC0F2041C76300387136 /* UniqueArray.cpp */; };
    877877                E32B549222810AC4008AD702 /* Packed.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E32B549122810AC0008AD702 /* Packed.cpp */; };
     878                E35FC7B222B82A7300F32F98 /* JSLockTakesWebThreadLock.mm in Sources */ = {isa = PBXBuildFile; fileRef = E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */; };
    878879                E373D7911F2CF35200C6FAAF /* Signals.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E3953F951F2CF32100A76A2E /* Signals.cpp */; };
    879880                E38A0D351FD50CC300E98C8B /* Threading.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E38A0D341FD50CBC00E98C8B /* Threading.cpp */; };
    … …  
    22732274                E19DB9781B32137C00DB38D4 /* NavigatorLanguage.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = NavigatorLanguage.mm; sourceTree = "<group>"; };
    22742275                E32B549122810AC0008AD702 /* Packed.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = Packed.cpp; sourceTree = "<group>"; };
     2276                E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = JSLockTakesWebThreadLock.mm; sourceTree = "<group>"; };
    22752277                E388887020C9098100E632BC /* WorkerPool.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkerPool.cpp; sourceTree = "<group>"; };
    22762278                E38A0D341FD50CBC00E98C8B /* Threading.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Threading.cpp; sourceTree = "<group>"; };
    … …  
    38083810                                CDC8E49A1BC728FE00594FEC /* Resources */,
    38093811                                CDC8E4851BC5B19400594FEC /* AudioSessionCategoryIOS.mm */,
     3812                                E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */,
    38103813                                CDC0932A21C872C10030C4B0 /* ScrollingDoesNotPauseMedia.mm */,
    38113814                                0F4FFA9D1ED3AA8500F7111F /* SnapshotViaRenderInContext.mm */,
    … …  
    43044307                                7CCE7EA51A411A0800447C4C /* JavaScriptTestMac.mm in Sources */,
    43054308                                5C0160C121A132460077FA32 /* JITEnabled.mm in Sources */,
     4309                                E35FC7B222B82A7300F32F98 /* JSLockTakesWebThreadLock.mm in Sources */,
    43064310                                7CCE7EC41A411A7E00447C4C /* JSWrapperForNodeInWebFrame.mm in Sources */,
    43074311                                F45E15732112CE2900307E82 /* KeyboardInputTestsIOS.mm in Sources */,
Note: See TracChangeset for help on using the changeset viewer.