Changeset 247346 in webkit


Ignore:
Timestamp:
Jul 11, 2019 2:42:22 AM (5 years ago)
Author:
ysuzuki@apple.com
Message:

Unreviewed, revert r243617.
​https://bugs.webkit.org/show_bug.cgi?id=196341

Mark pointed out that JSVirtualMachine can be gone in the other thread while we are executing GC constraint-solving.
This patch does not account that JavaScriptCore.framework is multi-thread safe: JSVirtualMachine wrapper can be destroyed,
and [JSVirtualMachine dealloc] can be executed in any threads while the VM is retained and used in the other thread (e.g.
destroyed from AutoReleasePool in some thread).

  • API/JSContext.mm:

(-[JSContext initWithVirtualMachine:]):
(-[JSContext dealloc]):
(-[JSContext initWithGlobalContextRef:]):
(-[JSContext wrapperMap]):
(+[JSContext contextWithJSGlobalContextRef:]):

  • API/JSVirtualMachine.mm:

(initWrapperCache):
(wrapperCache):
(+[JSVMWrapperCache addWrapper:forJSContextGroupRef:]):
(+[JSVMWrapperCache wrapperForJSContextGroupRef:]):
(-[JSVirtualMachine initWithContextGroupRef:]):
(-[JSVirtualMachine dealloc]):
(+[JSVirtualMachine virtualMachineWithContextGroupRef:]):
(-[JSVirtualMachine contextForGlobalContextRef:]):
(-[JSVirtualMachine addContext:forGlobalContextRef:]):
(scanExternalObjectGraph):
(scanExternalRememberedSet):

  • API/JSVirtualMachineInternal.h:
  • runtime/JSGlobalObject.h:

(JSC::JSGlobalObject::setWrapperMap):
(JSC::JSGlobalObject::setAPIWrapper): Deleted.
(JSC::JSGlobalObject::apiWrapper const): Deleted.

  • runtime/VM.h:
Location:
trunk/Source/JavaScriptCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

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

    r244323 r247346  
    8686
    8787    [self ensureWrapperMap];
    88 
    89     toJSGlobalObject(m_context)->setAPIWrapper((__bridge void*)self);
     88    [m_virtualMachine addContext:self forGlobalContextRef:m_context];
    9089
    9190    return self;
    … …  
    9493- (void)dealloc
    9594{
    96     toJSGlobalObject(m_context)->setAPIWrapper((__bridge void*)nil);
    9795    m_exception.clear();
    9896    JSGlobalContextRelease(m_context);
    … …  
    311309    };
    312310
    313     toJSGlobalObject(m_context)->setAPIWrapper((__bridge void*)self);
     311    [m_virtualMachine addContext:self forGlobalContextRef:m_context];
    314312
    315313    return self;
    … …  
    361359- (JSWrapperMap *)wrapperMap
    362360{
    363     return toJSGlobalObject(m_context)->wrapperMap();
     361    return toJS(m_context)->lexicalGlobalObject()->wrapperMap();
    364362}
    365363
    … …  
    372370+ (JSContext *)contextWithJSGlobalContextRef:(JSGlobalContextRef)globalContext
    373371{
    374     JSContext *context = (__bridge JSContext *)toJSGlobalObject(globalContext)->apiWrapper();
     372    JSVirtualMachine *virtualMachine = [JSVirtualMachine virtualMachineWithContextGroupRef:toRef(&toJS(globalContext)->vm())];
     373    JSContext *context = [virtualMachine contextForGlobalContextRef:globalContext];
    375374    if (!context)
    376375        context = [[[JSContext alloc] initWithGlobalContextRef:globalContext] autorelease];

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

    r246578 r247346  
    4242#import <wtf/Lock.h>
    4343
     44static NSMapTable *globalWrapperCache = 0;
     45
     46static Lock wrapperCacheMutex;
     47
     48static void initWrapperCache()
     49{
     50    ASSERT(!globalWrapperCache);
     51    NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
     52    NSPointerFunctionsOptions valueOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
     53    globalWrapperCache = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
     54}
     55
     56static NSMapTable *wrapperCache()
     57{
     58    if (!globalWrapperCache)
     59        initWrapperCache();
     60    return globalWrapperCache;
     61}
     62
     63@interface JSVMWrapperCache : NSObject
     64+ (void)addWrapper:(JSVirtualMachine *)wrapper forJSContextGroupRef:(JSContextGroupRef)group;
     65+ (JSVirtualMachine *)wrapperForJSContextGroupRef:(JSContextGroupRef)group;
     66@end
     67
     68@implementation JSVMWrapperCache
     69
     70+ (void)addWrapper:(JSVirtualMachine *)wrapper forJSContextGroupRef:(JSContextGroupRef)group
     71{
     72    std::lock_guard<Lock> lock(wrapperCacheMutex);
     73    NSMapInsert(wrapperCache(), group, (__bridge void*)wrapper);
     74}
     75
     76+ (JSVirtualMachine *)wrapperForJSContextGroupRef:(JSContextGroupRef)group
     77{
     78    std::lock_guard<Lock> lock(wrapperCacheMutex);
     79    return (__bridge JSVirtualMachine *)NSMapGet(wrapperCache(), group);
     80}
     81
     82@end
     83
    4484@implementation JSVirtualMachine {
    4585    JSContextGroupRef m_group;
    4686    Lock m_externalDataMutex;
     87    NSMapTable *m_contextCache;
    4788    NSMapTable *m_externalObjectGraph;
    4889    NSMapTable *m_externalRememberedSet;
    … …  
    66107    m_group = JSContextGroupRetain(group);
    67108   
     109    NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
     110    NSPointerFunctionsOptions valueOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
     111    m_contextCache = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
     112   
    68113    NSPointerFunctionsOptions weakIDOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
    69114    NSPointerFunctionsOptions strongIDOptions = NSPointerFunctionsStrongMemory | NSPointerFunctionsObjectPersonality;
    … …  
    72117    NSPointerFunctionsOptions integerOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsIntegerPersonality;
    73118    m_externalRememberedSet = [[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:integerOptions capacity:0];
    74 
    75     toJS(group)->m_apiWrapper = (__bridge void*)self;
     119   
     120    [JSVMWrapperCache addWrapper:self forJSContextGroupRef:group];
    76121 
    77122    return self;
    … …  
    80125- (void)dealloc
    81126{
    82     toJS(m_group)->m_apiWrapper = (__bridge void*)nil;
    83127    JSContextGroupRelease(m_group);
     128    [m_contextCache release];
    84129    [m_externalObjectGraph release];
    85130    [m_externalRememberedSet release];
    … …  
    193238+ (JSVirtualMachine *)virtualMachineWithContextGroupRef:(JSContextGroupRef)group
    194239{
    195     auto* vm = toJS(group);
    196     JSVirtualMachine *virtualMachine = (__bridge JSVirtualMachine *)vm->m_apiWrapper;
     240    JSVirtualMachine *virtualMachine = [JSVMWrapperCache wrapperForJSContextGroupRef:group];
    197241    if (!virtualMachine)
    198242        virtualMachine = [[[JSVirtualMachine alloc] initWithContextGroupRef:group] autorelease];
    199243    return virtualMachine;
     244}
     245
     246- (JSContext *)contextForGlobalContextRef:(JSGlobalContextRef)globalContext
     247{
     248    return (__bridge JSContext *)NSMapGet(m_contextCache, globalContext);
     249}
     250
     251- (void)addContext:(JSContext *)wrapper forGlobalContextRef:(JSGlobalContextRef)globalContext
     252{
     253    NSMapInsert(m_contextCache, globalContext, (__bridge void*)wrapper);
    200254}
    201255
    … …  
    264318{
    265319    @autoreleasepool {
    266         JSVirtualMachine *virtualMachine = (__bridge JSVirtualMachine *)vm.m_apiWrapper;
     320        JSVirtualMachine *virtualMachine = [JSVMWrapperCache wrapperForJSContextGroupRef:toRef(&vm)];
    267321        if (!virtualMachine)
    268322            return;
    … …  
    302356{
    303357    @autoreleasepool {
    304         JSVirtualMachine *virtualMachine = (__bridge JSVirtualMachine *)vm.m_apiWrapper;
     358        JSVirtualMachine *virtualMachine = [JSVMWrapperCache wrapperForJSContextGroupRef:toRef(&vm)];
    305359        if (!virtualMachine)
    306360            return;

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

    r246578 r247346  
    4545+ (JSVirtualMachine *)virtualMachineWithContextGroupRef:(JSContextGroupRef)group;
    4646
     47- (JSContext *)contextForGlobalContextRef:(JSGlobalContextRef)globalContext;
     48- (void)addContext:(JSContext *)wrapper forGlobalContextRef:(JSGlobalContextRef)globalContext;
    4749- (JSC::VM&)vm;
    4850

  • trunk/Source/JavaScriptCore/ChangeLog

    r247296 r247346  
     12019-07-11  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        Unreviewed, revert r243617.
     4        https://bugs.webkit.org/show_bug.cgi?id=196341
     5
     6        Mark pointed out that JSVirtualMachine can be gone in the other thread while we are executing GC constraint-solving.
     7        This patch does not account that JavaScriptCore.framework is multi-thread safe: JSVirtualMachine wrapper can be destroyed,
     8        and [JSVirtualMachine dealloc] can be executed in any threads while the VM is retained and used in the other thread (e.g.
     9        destroyed from AutoReleasePool in some thread).
     10
     11        * API/JSContext.mm:
     12        (-[JSContext initWithVirtualMachine:]):
     13        (-[JSContext dealloc]):
     14        (-[JSContext initWithGlobalContextRef:]):
     15        (-[JSContext wrapperMap]):
     16        (+[JSContext contextWithJSGlobalContextRef:]):
     17        * API/JSVirtualMachine.mm:
     18        (initWrapperCache):
     19        (wrapperCache):
     20        (+[JSVMWrapperCache addWrapper:forJSContextGroupRef:]):
     21        (+[JSVMWrapperCache wrapperForJSContextGroupRef:]):
     22        (-[JSVirtualMachine initWithContextGroupRef:]):
     23        (-[JSVirtualMachine dealloc]):
     24        (+[JSVirtualMachine virtualMachineWithContextGroupRef:]):
     25        (-[JSVirtualMachine contextForGlobalContextRef:]):
     26        (-[JSVirtualMachine addContext:forGlobalContextRef:]):
     27        (scanExternalObjectGraph):
     28        (scanExternalRememberedSet):
     29        * API/JSVirtualMachineInternal.h:
     30        * runtime/JSGlobalObject.h:
     31        (JSC::JSGlobalObject::setWrapperMap):
     32        (JSC::JSGlobalObject::setAPIWrapper): Deleted.
     33        (JSC::JSGlobalObject::apiWrapper const): Deleted.
     34        * runtime/VM.h:
     35
    1362019-07-10  Tadeu Zagallo  <tzagallo@apple.com>
    237

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

    r246565 r247346  
    10111011    JSWrapperMap* wrapperMap() const { return m_wrapperMap.get(); }
    10121012    void setWrapperMap(JSWrapperMap* map) { m_wrapperMap = map; }
    1013     void setAPIWrapper(void* apiWrapper) { m_apiWrapper = apiWrapper; }
    1014     void* apiWrapper() const { return m_apiWrapper; }
    10151013#endif
    10161014#ifdef JSC_GLIB_API_ENABLED
    … …  
    10551053#if JSC_OBJC_API_ENABLED
    10561054    RetainPtr<JSWrapperMap> m_wrapperMap;
    1057     void* m_apiWrapper { nullptr };
    10581055#endif
    10591056#ifdef JSC_GLIB_API_ENABLED

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

    r246572 r247346  
    834834#endif
    835835
    836 #if JSC_OBJC_API_ENABLED
    837     void* m_apiWrapper { nullptr };
    838 #endif
    839 
    840836    JS_EXPORT_PRIVATE void resetDateCache();
    841837
Note: See TracChangeset for help on using the changeset viewer.