Changeset 217240 in webkit


Ignore:
Timestamp:
May 22, 2017 12:48:38 PM (7 years ago)
Author:
keith_miller@apple.com
Message:

[Cocoa] An exported Objective C class’s prototype and constructor don't persist across JSContext deallocation
​https://bugs.webkit.org/show_bug.cgi?id=167708

Reviewed by Geoffrey Garen.

This patch moves the Objective C wrapper map to the global object. In order to make this work the JSWrapperMap
class no longer holds a reference to the JSContext. Instead, the context must be provided when getting a wrapper.

Also, this patch fixes a "bug" where we would observe changes to the Object property on the global object when
creating a wrapper for NSObject.

  • API/APICast.h:

(toJSGlobalObject):

  • API/JSContext.mm:

(-[JSContext ensureWrapperMap]):
(-[JSContext initWithVirtualMachine:]):
(-[JSContext dealloc]):
(-[JSContext wrapperMap]):
(-[JSContext initWithGlobalContextRef:]):
(-[JSContext wrapperForObjCObject:]):
(-[JSContext wrapperForJSObject:]):

  • API/JSWrapperMap.h:
  • API/JSWrapperMap.mm:

(-[JSObjCClassInfo initForClass:]):
(-[JSObjCClassInfo allocateConstructorAndPrototypeInContext:]):
(-[JSObjCClassInfo wrapperForObject:inContext:]):
(-[JSObjCClassInfo constructorInContext:]):
(-[JSObjCClassInfo prototypeInContext:]):
(-[JSWrapperMap initWithGlobalContextRef:]):
(-[JSWrapperMap classInfoForClass:]):
(-[JSWrapperMap jsWrapperForObject:inContext:]):
(-[JSWrapperMap objcWrapperForJSValueRef:inContext:]):
(-[JSObjCClassInfo initWithContext:forClass:]): Deleted.
(-[JSObjCClassInfo allocateConstructorAndPrototype]): Deleted.
(-[JSObjCClassInfo wrapperForObject:]): Deleted.
(-[JSObjCClassInfo constructor]): Deleted.
(-[JSObjCClassInfo prototype]): Deleted.
(-[JSWrapperMap initWithContext:]): Deleted.
(-[JSWrapperMap jsWrapperForObject:]): Deleted.
(-[JSWrapperMap objcWrapperForJSValueRef:]): Deleted.

  • API/tests/JSExportTests.mm:

(wrapperLifetimeIsTiedToGlobalObject):
(runJSExportTests):

  • API/tests/testapi.mm:
  • runtime/JSGlobalObject.h:

(JSC::JSGlobalObject::wrapperMap):
(JSC::JSGlobalObject::setWrapperMap):

Location:
trunk/Source/JavaScriptCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

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

    r165676 r217240  
    5959    ASSERT(c);
    6060    return reinterpret_cast<JSC::ExecState*>(c);
     61}
     62
     63inline JSC::JSGlobalObject* toJSGlobalObject(JSGlobalContextRef context)
     64{
     65    return toJS(context)->lexicalGlobalObject();
    6166}
    6267

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

    r211740 r217240  
    4444    JSVirtualMachine *m_virtualMachine;
    4545    JSGlobalContextRef m_context;
    46     JSWrapperMap *m_wrapperMap;
    4746    JSC::Strong<JSC::JSObject> m_exception;
    4847}
    … …  
    5150{
    5251    return m_context;
     52}
     53
     54- (void)ensureWrapperMap
     55{
     56    if (!toJS([self JSGlobalContextRef])->lexicalGlobalObject()->wrapperMap())
     57        [[JSWrapperMap alloc] initWithGlobalContextRef:[self JSGlobalContextRef]];
    5358}
    5459
    … …  
    6671    m_virtualMachine = [virtualMachine retain];
    6772    m_context = JSGlobalContextCreateInGroup(getGroupFromVirtualMachine(virtualMachine), 0);
    68     m_wrapperMap = [[JSWrapperMap alloc] initWithContext:self];
    6973
    7074    self.exceptionHandler = ^(JSContext *context, JSValue *exceptionValue) {
    … …  
    7276    };
    7377
     78    [self ensureWrapperMap];
    7479    [m_virtualMachine addContext:self forGlobalContextRef:m_context];
    7580
    … …  
    8085{
    8186    m_exception.clear();
    82     [m_wrapperMap release];
    8387    JSGlobalContextRelease(m_context);
    8488    [m_virtualMachine release];
    … …  
    126130- (JSWrapperMap *)wrapperMap
    127131{
    128     return m_wrapperMap;
     132    return toJS(m_context)->lexicalGlobalObject()->wrapperMap();
    129133}
    130134
    … …  
    259263    ASSERT(m_virtualMachine);
    260264    m_context = JSGlobalContextRetain(context);
    261     m_wrapperMap = [[JSWrapperMap alloc] initWithContext:self];
     265    [self ensureWrapperMap];
    262266
    263267    self.exceptionHandler = ^(JSContext *context, JSValue *exceptionValue) {
    … …  
    310314{
    311315    JSC::JSLockHolder locker(toJS(m_context));
    312     return [m_wrapperMap jsWrapperForObject:object];
     316    return [[self wrapperMap] jsWrapperForObject:object inContext:self];
    313317}
    314318
    … …  
    316320{
    317321    JSC::JSLockHolder locker(toJS(m_context));
    318     return [m_wrapperMap objcWrapperForJSValueRef:value];
     322    return [[self wrapperMap] objcWrapperForJSValueRef:value inContext:self];
    319323}
    320324

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

    r158762 r217240  
    3232@interface JSWrapperMap : NSObject
    3333
    34 - (id)initWithContext:(JSContext *)context;
     34- (id)initWithGlobalContextRef:(JSGlobalContextRef)context;
    3535
    36 - (JSValue *)jsWrapperForObject:(id)object;
     36- (JSValue *)jsWrapperForObject:(id)object inContext:(JSContext *)context;
    3737
    38 - (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value;
     38- (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value inContext:(JSContext *)context;
    3939
    4040@end

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

    r211247 r217240  
    3636#import "ObjCCallbackFunction.h"
    3737#import "ObjcRuntimeExtras.h"
     38#import "ObjectConstructor.h"
    3839#import "WeakGCMap.h"
    3940#import "WeakGCMapInlines.h"
    … …  
    364365
    365366@interface JSObjCClassInfo : NSObject {
    366     JSContext *m_context;
    367367    Class m_class;
    368368    bool m_block;
    … …  
    372372}
    373373
    374 - (id)initWithContext:(JSContext *)context forClass:(Class)cls;
    375 - (JSC::JSObject *)wrapperForObject:(id)object;
    376 - (JSC::JSObject *)constructor;
    377 - (JSC::JSObject *)prototype;
     374- (id)initForClass:(Class)cls;
     375- (JSC::JSObject *)wrapperForObject:(id)object inContext:(JSContext *)context;
     376- (JSC::JSObject *)constructorInContext:(JSContext *)context;
     377- (JSC::JSObject *)prototypeInContext:(JSContext *)context;
    378378
    379379@end
    … …  
    381381@implementation JSObjCClassInfo
    382382
    383 - (id)initWithContext:(JSContext *)context forClass:(Class)cls
     383- (id)initForClass:(Class)cls
    384384{
    385385    self = [super init];
    … …  
    388388
    389389    const char* className = class_getName(cls);
    390     m_context = context;
    391390    m_class = cls;
    392391    m_block = [cls isSubclassOfClass:getNSBlockClass()];
    … …  
    459458typedef std::pair<JSC::JSObject*, JSC::JSObject*> ConstructorPrototypePair;
    460459
    461 - (ConstructorPrototypePair)allocateConstructorAndPrototype
    462 {
    463     JSObjCClassInfo* superClassInfo = [m_context.wrapperMap classInfoForClass:class_getSuperclass(m_class)];
     460- (ConstructorPrototypePair)allocateConstructorAndPrototypeInContext:(JSContext *)context
     461{
     462    JSObjCClassInfo* superClassInfo = [context.wrapperMap classInfoForClass:class_getSuperclass(m_class)];
    464463
    465464    ASSERT(!m_constructor || !m_prototype);
    … …  
    470469
    471470    if (!superClassInfo) {
    472         JSContextRef cContext = [m_context JSGlobalContextRef];
    473         JSValue *constructor = m_context[@"Object"];
     471        JSC::JSGlobalObject* globalObject = toJSGlobalObject([context JSGlobalContextRef]);
    474472        if (!jsConstructor)
    475             jsConstructor = toJS(JSValueToObject(cContext, valueInternalValue(constructor), 0));
    476 
    477         if (!jsPrototype) {
    478             JSValue *prototype = constructor[@"prototype"];
    479             jsPrototype = toJS(JSValueToObject(cContext, valueInternalValue(prototype), 0));
    480         }
     473            jsConstructor = globalObject->objectConstructor();
     474
     475        if (!jsPrototype)
     476            jsPrototype = globalObject->objectPrototype();
    481477    } else {
    482478        const char* className = class_getName(m_class);
    … …  
    484480        // Create or grab the prototype/constructor pair.
    485481        if (!jsPrototype)
    486             jsPrototype = objectWithCustomBrand(m_context, [NSString stringWithFormat:@"%sPrototype", className]);
     482            jsPrototype = objectWithCustomBrand(context, [NSString stringWithFormat:@"%sPrototype", className]);
    487483
    488484        if (!jsConstructor)
    489             jsConstructor = allocateConstructorForCustomClass(m_context, className, m_class);
    490 
    491         JSValue* prototype = [JSValue valueWithJSValueRef:toRef(jsPrototype) inContext:m_context];
    492         JSValue* constructor = [JSValue valueWithJSValueRef:toRef(jsConstructor) inContext:m_context];
     485            jsConstructor = allocateConstructorForCustomClass(context, className, m_class);
     486
     487        JSValue* prototype = [JSValue valueWithJSValueRef:toRef(jsPrototype) inContext:context];
     488        JSValue* constructor = [JSValue valueWithJSValueRef:toRef(jsConstructor) inContext:context];
    493489        putNonEnumerable(prototype, @"constructor", constructor);
    494490        putNonEnumerable(constructor, @"prototype", prototype);
    … …  
    496492        Protocol *exportProtocol = getJSExportProtocol();
    497493        forEachProtocolImplementingProtocol(m_class, exportProtocol, ^(Protocol *protocol){
    498             copyPrototypeProperties(m_context, m_class, protocol, prototype);
    499             copyMethodsToObject(m_context, m_class, protocol, NO, constructor);
     494            copyPrototypeProperties(context, m_class, protocol, prototype);
     495            copyMethodsToObject(context, m_class, protocol, NO, constructor);
    500496        });
    501497
    502498        // Set [Prototype].
    503         JSC::JSObject* superClassPrototype = [superClassInfo prototype];
    504         JSObjectSetPrototype([m_context JSGlobalContextRef], toRef(jsPrototype), toRef(superClassPrototype));
     499        JSC::JSObject* superClassPrototype = [superClassInfo prototypeInContext:context];
     500        JSObjectSetPrototype([context JSGlobalContextRef], toRef(jsPrototype), toRef(superClassPrototype));
    505501    }
    506502
    … …  
    510506}
    511507
    512 - (JSC::JSObject*)wrapperForObject:(id)object
     508- (JSC::JSObject*)wrapperForObject:(id)object inContext:(JSContext *)context
    513509{
    514510    ASSERT([object isKindOfClass:m_class]);
    515511    ASSERT(m_block == [object isKindOfClass:getNSBlockClass()]);
    516512    if (m_block) {
    517         if (JSObjectRef method = objCCallbackFunctionForBlock(m_context, object)) {
    518             JSValue *constructor = [JSValue valueWithJSValueRef:method inContext:m_context];
    519             JSValue *prototype = [JSValue valueWithNewObjectInContext:m_context];
     513        if (JSObjectRef method = objCCallbackFunctionForBlock(context, object)) {
     514            JSValue *constructor = [JSValue valueWithJSValueRef:method inContext:context];
     515            JSValue *prototype = [JSValue valueWithNewObjectInContext:context];
    520516            putNonEnumerable(constructor, @"prototype", prototype);
    521517            putNonEnumerable(prototype, @"constructor", constructor);
    … …  
    524520    }
    525521
    526     JSC::JSObject* prototype = [self prototype];
    527 
    528     JSC::JSObject* wrapper = makeWrapper([m_context JSGlobalContextRef], m_classRef, object);
    529     JSObjectSetPrototype([m_context JSGlobalContextRef], toRef(wrapper), toRef(prototype));
     522    JSC::JSObject* prototype = [self prototypeInContext:context];
     523
     524    JSC::JSObject* wrapper = makeWrapper([context JSGlobalContextRef], m_classRef, object);
     525    JSObjectSetPrototype([context JSGlobalContextRef], toRef(wrapper), toRef(prototype));
    530526    return wrapper;
    531527}
    532528
    533 - (JSC::JSObject*)constructor
     529- (JSC::JSObject*)constructorInContext:(JSContext *)context
    534530{
    535531    JSC::JSObject* constructor = m_constructor.get();
    536532    if (!constructor)
    537         constructor = [self allocateConstructorAndPrototype].first;
     533        constructor = [self allocateConstructorAndPrototypeInContext:context].first;
    538534    ASSERT(!!constructor);
    539535    return constructor;
    540536}
    541537
    542 - (JSC::JSObject*)prototype
     538- (JSC::JSObject*)prototypeInContext:(JSContext *)context
    543539{
    544540    JSC::JSObject* prototype = m_prototype.get();
    545541    if (!prototype)
    546         prototype = [self allocateConstructorAndPrototype].second;
     542        prototype = [self allocateConstructorAndPrototypeInContext:context].second;
    547543    ASSERT(!!prototype);
    548544    return prototype;
    … …  
    552548
    553549@implementation JSWrapperMap {
    554     JSContext *m_context;
    555550    NSMutableDictionary *m_classMap;
    556551    std::unique_ptr<JSC::WeakGCMap<id, JSC::JSObject>> m_cachedJSWrappers;
    … …  
    558553}
    559554
    560 - (id)initWithContext:(JSContext *)context
     555- (id)initWithGlobalContextRef:(JSGlobalContextRef)context
    561556{
    562557    self = [super init];
    … …  
    568563    m_cachedObjCWrappers = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
    569564
    570     m_cachedJSWrappers = std::make_unique<JSC::WeakGCMap<id, JSC::JSObject>>(toJS([context JSGlobalContextRef])->vm());
    571 
    572     m_context = context;
     565    m_cachedJSWrappers = std::make_unique<JSC::WeakGCMap<id, JSC::JSObject>>(toJS(context)->vm());
     566
     567    ASSERT(!toJSGlobalObject(context)->wrapperMap());
     568    toJSGlobalObject(context)->setWrapperMap(self);
    573569    m_classMap = [[NSMutableDictionary alloc] init];
    574570    return self;
    … …  
    595591        return m_classMap[cls] = [self classInfoForClass:class_getSuperclass(cls)];
    596592
    597     return m_classMap[cls] = [[[JSObjCClassInfo alloc] initWithContext:m_context forClass:cls] autorelease];
    598 }
    599 
    600 - (JSValue *)jsWrapperForObject:(id)object
    601 {
     593    return m_classMap[cls] = [[[JSObjCClassInfo alloc] initForClass:cls] autorelease];
     594}
     595
     596- (JSValue *)jsWrapperForObject:(id)object inContext:(JSContext *)context
     597{
     598    ASSERT(toJSGlobalObject([context JSGlobalContextRef])->wrapperMap() == self);
    602599    JSC::JSObject* jsWrapper = m_cachedJSWrappers->get(object);
    603600    if (jsWrapper)
    604         return [JSValue valueWithJSValueRef:toRef(jsWrapper) inContext:m_context];
     601        return [JSValue valueWithJSValueRef:toRef(jsWrapper) inContext:context];
    605602
    606603    if (class_isMetaClass(object_getClass(object)))
    607         jsWrapper = [[self classInfoForClass:(Class)object] constructor];
     604        jsWrapper = [[self classInfoForClass:(Class)object] constructorInContext:context];
    608605    else {
    609606        JSObjCClassInfo* classInfo = [self classInfoForClass:[object class]];
    610         jsWrapper = [classInfo wrapperForObject:object];
     607        jsWrapper = [classInfo wrapperForObject:object inContext:context];
    611608    }
    612609
    … …  
    617614    //     but still, would probably nicer if we made it so that only one associated object was required, broadcasting object dealloc.
    618615    m_cachedJSWrappers->set(object, jsWrapper);
    619     return [JSValue valueWithJSValueRef:toRef(jsWrapper) inContext:m_context];
    620 }
    621 
    622 - (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value
    623 {
     616    return [JSValue valueWithJSValueRef:toRef(jsWrapper) inContext:context];
     617}
     618
     619- (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value inContext:context
     620{
     621    ASSERT(toJSGlobalObject([context JSGlobalContextRef])->wrapperMap() == self);
    624622    JSValue *wrapper = static_cast<JSValue *>(NSMapGet(m_cachedObjCWrappers, value));
    625623    if (!wrapper) {
    626         wrapper = [[[JSValue alloc] initWithValue:value inContext:m_context] autorelease];
     624        wrapper = [[[JSValue alloc] initWithValue:value inContext:context] autorelease];
    627625        NSMapInsert(m_cachedObjCWrappers, value, wrapper);
    628626    }

  • trunk/Source/JavaScriptCore/API/tests/JSExportTests.mm

    r164439 r217240  
    126126@end
    127127
     128@protocol AJSExport <JSExport>
     129- (instancetype)init;
     130@end
     131
     132@interface A : NSObject <AJSExport>
     133@end
     134
     135@implementation A
     136@end
     137
     138static void wrapperLifetimeIsTiedToGlobalObject()
     139{
     140    JSGlobalContextRef contextRef;
     141    @autoreleasepool {
     142        JSContext *context = [[JSContext alloc] init];
     143        contextRef = JSGlobalContextRetain(context.JSGlobalContextRef);
     144        context[@"A"] = A.class;
     145        checkResult(@"Initial wrapper's constructor is itself", [[context evaluateScript:@"new A().constructor === A"] toBool]);
     146    }
     147
     148    @autoreleasepool {
     149        JSContext *context = [JSContext contextWithJSGlobalContextRef:contextRef];
     150        checkResult(@"New context's wrapper's constructor is itself", [[context evaluateScript:@"new A().constructor === A"] toBool]);
     151    }
     152
     153    JSGlobalContextRelease(contextRef);
     154}
     155
     156static void wrapperForNSObjectisObject()
     157{
     158    @autoreleasepool {
     159        JSContext *context = [[JSContext alloc] init];
     160        context[@"Object"] = [[NSNull alloc] init];
     161        context.exception = nil;
     162
     163        context[@"A"] = NSObject.class;
     164        NSLog(@"here: %@", [context exception]);
     165    }
     166}
     167
    128168void runJSExportTests()
    129169{
    … …  
    133173        [JSExportTests exportDynamicallyGeneratedProtocolTest];
    134174    }
     175    wrapperLifetimeIsTiedToGlobalObject();
     176    wrapperForNSObjectisObject();
    135177}
    136178

  • trunk/Source/JavaScriptCore/API/tests/testapi.mm

    r206876 r217240  
    513513static void testObjectiveCAPIMain()
    514514{
     515    runJSExportTests();
     516
    515517    @autoreleasepool {
    516518        JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
    … …  
    14691471        checkResult(@"Ran code in five concurrent VMs that GC'd", ok);
    14701472    }
    1471    
     1473
    14721474    currentThisInsideBlockGetterTest();
    14731475    runDateTests();
    … …  
    15081510}
    15091511
     1512
    15101513void testObjectiveCAPI()
    15111514{

  • trunk/Source/JavaScriptCore/ChangeLog

    r217221 r217240  
     12017-05-22  Keith Miller  <keith_miller@apple.com>
     2
     3        [Cocoa] An exported Objective C class’s prototype and constructor don't persist across JSContext deallocation
     4        https://bugs.webkit.org/show_bug.cgi?id=167708
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        This patch moves the Objective C wrapper map to the global object. In order to make this work the JSWrapperMap
     9        class no longer holds a reference to the JSContext. Instead, the context must be provided when getting a wrapper.
     10
     11        Also, this patch fixes a "bug" where we would observe changes to the Object property on the global object when
     12        creating a wrapper for NSObject.
     13
     14        * API/APICast.h:
     15        (toJSGlobalObject):
     16        * API/JSContext.mm:
     17        (-[JSContext ensureWrapperMap]):
     18        (-[JSContext initWithVirtualMachine:]):
     19        (-[JSContext dealloc]):
     20        (-[JSContext wrapperMap]):
     21        (-[JSContext initWithGlobalContextRef:]):
     22        (-[JSContext wrapperForObjCObject:]):
     23        (-[JSContext wrapperForJSObject:]):
     24        * API/JSWrapperMap.h:
     25        * API/JSWrapperMap.mm:
     26        (-[JSObjCClassInfo initForClass:]):
     27        (-[JSObjCClassInfo allocateConstructorAndPrototypeInContext:]):
     28        (-[JSObjCClassInfo wrapperForObject:inContext:]):
     29        (-[JSObjCClassInfo constructorInContext:]):
     30        (-[JSObjCClassInfo prototypeInContext:]):
     31        (-[JSWrapperMap initWithGlobalContextRef:]):
     32        (-[JSWrapperMap classInfoForClass:]):
     33        (-[JSWrapperMap jsWrapperForObject:inContext:]):
     34        (-[JSWrapperMap objcWrapperForJSValueRef:inContext:]):
     35        (-[JSObjCClassInfo initWithContext:forClass:]): Deleted.
     36        (-[JSObjCClassInfo allocateConstructorAndPrototype]): Deleted.
     37        (-[JSObjCClassInfo wrapperForObject:]): Deleted.
     38        (-[JSObjCClassInfo constructor]): Deleted.
     39        (-[JSObjCClassInfo prototype]): Deleted.
     40        (-[JSWrapperMap initWithContext:]): Deleted.
     41        (-[JSWrapperMap jsWrapperForObject:]): Deleted.
     42        (-[JSWrapperMap objcWrapperForJSValueRef:]): Deleted.
     43        * API/tests/JSExportTests.mm:
     44        (wrapperLifetimeIsTiedToGlobalObject):
     45        (runJSExportTests):
     46        * API/tests/testapi.mm:
     47        * runtime/JSGlobalObject.h:
     48        (JSC::JSGlobalObject::wrapperMap):
     49        (JSC::JSGlobalObject::setWrapperMap):
     50
    1512017-05-22  Filip Pizlo  <fpizlo@apple.com>
    252

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

    r217060 r217240  
    4646#include <array>
    4747#include <wtf/HashSet.h>
     48#include <wtf/RetainPtr.h>
    4849
    4950struct OpaqueJSClass;
    5051struct OpaqueJSClassContextData;
     52OBJC_CLASS JSWrapperMap;
    5153
    5254namespace Inspector {
    … …  
    830832    bool needsSiteSpecificQuirks() const { return m_needsSiteSpecificQuirks; }
    831833
     834#if JSC_OBJC_API_ENABLED
     835    JSWrapperMap* wrapperMap() const { return m_wrapperMap.get(); }
     836    void setWrapperMap(JSWrapperMap* map) { m_wrapperMap = map; }
     837#endif
     838
    832839protected:
    833840    struct GlobalPropertyInfo {
    … …  
    859866
    860867    bool m_needsSiteSpecificQuirks { false };
     868#if JSC_OBJC_API_ENABLED
     869    RetainPtr<JSWrapperMap> m_wrapperMap;
     870#endif
    861871};
    862872
Note: See TracChangeset for help on using the changeset viewer.