Changeset 180452 in webkit


Ignore:
Timestamp:
Feb 20, 2015 1:51:37 PM (9 years ago)
Author:
mark.lam@apple.com
Message:

[JSObjCClassInfo reallocateConstructorAndOrPrototype] should also reallocate super class prototype chain.
<​https://webkit.org/b/141809>

Reviewed by Geoffrey Garen.

A ObjC class that implement the JSExport protocol will have a JS prototype
chain and constructor automatically synthesized for its JS wrapper object.
However, if there are no more instances of that ObjC class reachable by a
JS GC root scan, then its synthesized prototype chain and constructors may
be released by the GC. If a new instance of that ObjC class is subsequently
instantiated, then [JSObjCClassInfo reallocateConstructorAndOrPrototype]
should re-construct the prototype chain and constructor (if they were
previously released). However, the current implementation only
re-constructs the immediate prototype, but not every other prototype
object upstream in the prototype chain.

To fix this, we do the following:

  1. We no longer allocate the JSObjCClassInfo's prototype and constructor eagerly. Hence, -initWithContext:forClass: will no longer call -allocateConstructorAndPrototypeWithSuperClassInfo:.
  2. Instead, we'll always access the prototype and constructor thru accessor methods. The accessor methods will call -allocateConstructorAndPrototype: if needed.
  3. -allocateConstructorAndPrototype: will fetch the needed superClassInfo from the JSWrapperMap itself. This makes it so that we no longer need to pass the superClassInfo all over.
  4. -allocateConstructorAndPrototype: will get the super class prototype by invoking -prototype: on the superClassInfo, thereby allowing the super class to allocate its prototype and constructor if needed and fixing the issue in this bug.
  1. Also removed the GC warning comments, and ensured that needed JS objects are kept alive by having a local var pointing to it from the stack (which makes a GC root).
  • API/JSWrapperMap.mm:

(-[JSObjCClassInfo initWithContext:forClass:]):
(-[JSObjCClassInfo allocateConstructorAndPrototype]):
(-[JSObjCClassInfo wrapperForObject:]):
(-[JSObjCClassInfo constructor]):
(-[JSObjCClassInfo prototype]):
(-[JSWrapperMap classInfoForClass:]):
(-[JSObjCClassInfo initWithContext:forClass:superClassInfo:]): Deleted.
(-[JSObjCClassInfo allocateConstructorAndPrototypeWithSuperClassInfo:]): Deleted.
(-[JSObjCClassInfo reallocateConstructorAndOrPrototype]): Deleted.

  • API/tests/Regress141809.h: Added.
  • API/tests/Regress141809.mm: Added.

(-[TestClassB name]):
(-[TestClassC name]):
(runRegress141809):

Location:
trunk/Source/JavaScriptCore
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

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

    r177091 r180452  
    11/*
    2  * Copyright (C) 2013 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    365365}
    366366
    367 - (id)initWithContext:(JSContext *)context forClass:(Class)cls superClassInfo:(JSObjCClassInfo*)superClassInfo;
     367- (id)initWithContext:(JSContext *)context forClass:(Class)cls;
    368368- (JSValue *)wrapperForObject:(id)object;
    369369- (JSValue *)constructor;
     370- (JSC::JSObject *)prototype;
    370371
    371372@end
    … …  
    373374@implementation JSObjCClassInfo
    374375
    375 - (id)initWithContext:(JSContext *)context forClass:(Class)cls superClassInfo:(JSObjCClassInfo*)superClassInfo
     376- (id)initWithContext:(JSContext *)context forClass:(Class)cls
    376377{
    377378    self = [super init];
    … …  
    387388    definition.className = className;
    388389    m_classRef = JSClassCreate(&definition);
    389 
    390     [self allocateConstructorAndPrototypeWithSuperClassInfo:superClassInfo];
    391390
    392391    return self;
    … …  
    451450}
    452451
    453 - (void)allocateConstructorAndPrototypeWithSuperClassInfo:(JSObjCClassInfo*)superClassInfo
    454 {
     452typedef std::pair<JSC::JSObject*, JSC::JSObject*> ConstructorPrototypePair;
     453
     454- (ConstructorPrototypePair)allocateConstructorAndPrototype
     455{
     456    JSObjCClassInfo* superClassInfo = [m_context.wrapperMap classInfoForClass:class_getSuperclass(m_class)];
     457
    455458    ASSERT(!m_constructor || !m_prototype);
    456459    ASSERT((m_class == [NSObject class]) == !superClassInfo);
    … …  
    466469        }
    467470    } else {
    468         // We need to hold a reference to the superclass prototype here on the stack
    469         // to that it won't get GC'ed while we do allocations between now and when we
    470         // set it in this class' prototype below.
    471         JSC::JSObject* superClassPrototype = superClassInfo->m_prototype.get();
    472 
    473471        const char* className = class_getName(m_class);
    474472
    … …  
    500498
    501499        // Set [Prototype].
     500        JSC::JSObject* superClassPrototype = [superClassInfo prototype];
    502501        JSObjectSetPrototype([m_context JSGlobalContextRef], toRef(m_prototype.get()), toRef(superClassPrototype));
    503502    }
    504 }
    505 
    506 - (void)reallocateConstructorAndOrPrototype
    507 {
    508     [self allocateConstructorAndPrototypeWithSuperClassInfo:[m_context.wrapperMap classInfoForClass:class_getSuperclass(m_class)]];
    509     // We should not add any code here that can trigger a GC or the prototype and
    510     // constructor that we just created may be collected before they can be used.
     503    return ConstructorPrototypePair(m_constructor.get(), m_prototype.get());
    511504}
    512505
    … …  
    525518    }
    526519
    527     if (!m_prototype)
    528         [self reallocateConstructorAndOrPrototype];
    529     ASSERT(!!m_prototype);
    530     // We need to hold a reference to the prototype here on the stack to that it won't
    531     // get GC'ed while we create the wrapper below.
    532     JSC::JSObject* prototype = m_prototype.get();
     520    JSC::JSObject* prototype = [self prototype];
    533521
    534522    JSObjectRef wrapper = makeWrapper([m_context JSGlobalContextRef], m_classRef, object);
    … …  
    539527- (JSValue *)constructor
    540528{
    541     if (!m_constructor)
    542         [self reallocateConstructorAndOrPrototype];
    543     ASSERT(!!m_constructor);
    544     // If we need to add any code here in the future that can trigger a GC, we should
    545     // cache the constructor pointer in a stack local var first so that it is protected
    546     // from the GC until it gets used below.
    547     return [JSValue valueWithJSValueRef:toRef(m_constructor.get()) inContext:m_context];
     529    JSC::JSObject* constructor = m_constructor.get();
     530    if (!constructor)
     531        constructor = [self allocateConstructorAndPrototype].first;
     532    ASSERT(!!constructor);
     533    return [JSValue valueWithJSValueRef:toRef(constructor) inContext:m_context];
     534}
     535
     536- (JSC::JSObject*)prototype
     537{
     538    JSC::JSObject* prototype = m_prototype.get();
     539    if (!prototype)
     540        prototype = [self allocateConstructorAndPrototype].second;
     541    ASSERT(!!prototype);
     542    return prototype;
    548543}
    549544
    … …  
    592587        return m_classMap[cls] = [self classInfoForClass:class_getSuperclass(cls)];
    593588
    594     return m_classMap[cls] = [[[JSObjCClassInfo alloc] initWithContext:m_context forClass:cls superClassInfo:[self classInfoForClass:class_getSuperclass(cls)]] autorelease];
     589    return m_classMap[cls] = [[[JSObjCClassInfo alloc] initWithContext:m_context forClass:cls] autorelease];
    595590}
    596591

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

    r179753 r180452  
    11/*
    2  * Copyright (C) 2013 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2015 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2929#import "DateTests.h"
    3030#import "JSExportTests.h"
     31#import "Regress141809.h"
    3132
    3233#import <pthread.h>
    … …  
    14321433    runDateTests();
    14331434    runJSExportTests();
     1435    runRegress141809();
    14341436}
    14351437

  • trunk/Source/JavaScriptCore/ChangeLog

    r180441 r180452  
     12015-02-20  Mark Lam  <mark.lam@apple.com>
     2
     3        [JSObjCClassInfo reallocateConstructorAndOrPrototype] should also reallocate super class prototype chain.
     4        <https://webkit.org/b/141809>
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        A ObjC class that implement the JSExport protocol will have a JS prototype
     9        chain and constructor automatically synthesized for its JS wrapper object.
     10        However, if there are no more instances of that ObjC class reachable by a
     11        JS GC root scan, then its synthesized prototype chain and constructors may
     12        be released by the GC.  If a new instance of that ObjC class is subsequently
     13        instantiated, then [JSObjCClassInfo reallocateConstructorAndOrPrototype]
     14        should re-construct the prototype chain and constructor (if they were
     15        previously released).  However, the current implementation only
     16        re-constructs the immediate prototype, but not every other prototype
     17        object upstream in the prototype chain.
     18
     19        To fix this, we do the following:
     20        1. We no longer allocate the JSObjCClassInfo's prototype and constructor
     21           eagerly.  Hence, -initWithContext:forClass: will no longer call
     22           -allocateConstructorAndPrototypeWithSuperClassInfo:.
     23        2. Instead, we'll always access the prototype and constructor thru
     24           accessor methods.  The accessor methods will call
     25           -allocateConstructorAndPrototype: if needed.
     26        3. -allocateConstructorAndPrototype: will fetch the needed superClassInfo
     27           from the JSWrapperMap itself.  This makes it so that we no longer
     28           need to pass the superClassInfo all over.
     29        4. -allocateConstructorAndPrototype: will get the super class prototype
     30           by invoking -prototype: on the superClassInfo, thereby allowing the
     31           super class to allocate its prototype and constructor if needed and
     32           fixing the issue in this bug.
     33
     34        5. Also removed the GC warning comments, and ensured that needed JS
     35           objects are kept alive by having a local var pointing to it from the
     36           stack (which makes a GC root).
     37
     38        * API/JSWrapperMap.mm:
     39        (-[JSObjCClassInfo initWithContext:forClass:]):
     40        (-[JSObjCClassInfo allocateConstructorAndPrototype]):
     41        (-[JSObjCClassInfo wrapperForObject:]):
     42        (-[JSObjCClassInfo constructor]):
     43        (-[JSObjCClassInfo prototype]):
     44        (-[JSWrapperMap classInfoForClass:]):
     45        (-[JSObjCClassInfo initWithContext:forClass:superClassInfo:]): Deleted.
     46        (-[JSObjCClassInfo allocateConstructorAndPrototypeWithSuperClassInfo:]): Deleted.
     47        (-[JSObjCClassInfo reallocateConstructorAndOrPrototype]): Deleted.
     48        * API/tests/Regress141809.h: Added.
     49        * API/tests/Regress141809.mm: Added.
     50        (-[TestClassB name]):
     51        (-[TestClassC name]):
     52        (runRegress141809):
     53        * API/tests/testapi.mm:
     54        * JavaScriptCore.xcodeproj/project.pbxproj:
     55
    1562015-02-20  Alexey Proskuryakov  <ap@apple.com>
    257

  • trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r180279 r180452  
    16131613                FEA08620182B7A0400F6D851 /* Breakpoint.h in Headers */ = {isa = PBXBuildFile; fileRef = FEA0861E182B7A0400F6D851 /* Breakpoint.h */; settings = {ATTRIBUTES = (Private, ); }; };
    16141614                FEA08621182B7A0400F6D851 /* DebuggerPrimitives.h in Headers */ = {isa = PBXBuildFile; fileRef = FEA0861F182B7A0400F6D851 /* DebuggerPrimitives.h */; settings = {ATTRIBUTES = (Private, ); }; };
     1615                FEB51F6C1A97B688001F921C /* Regress141809.mm in Sources */ = {isa = PBXBuildFile; fileRef = FEB51F6B1A97B688001F921C /* Regress141809.mm */; };
    16151616                FEB58C14187B8B160098EF0B /* ErrorHandlingScope.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FEB58C12187B8B160098EF0B /* ErrorHandlingScope.cpp */; };
    16161617                FEB58C15187B8B160098EF0B /* ErrorHandlingScope.h in Headers */ = {isa = PBXBuildFile; fileRef = FEB58C13187B8B160098EF0B /* ErrorHandlingScope.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    33463347                FEA0861E182B7A0400F6D851 /* Breakpoint.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Breakpoint.h; sourceTree = "<group>"; };
    33473348                FEA0861F182B7A0400F6D851 /* DebuggerPrimitives.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DebuggerPrimitives.h; sourceTree = "<group>"; };
     3349                FEB51F6A1A97B688001F921C /* Regress141809.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = Regress141809.h; path = API/tests/Regress141809.h; sourceTree = "<group>"; };
     3350                FEB51F6B1A97B688001F921C /* Regress141809.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = Regress141809.mm; path = API/tests/Regress141809.mm; sourceTree = "<group>"; };
    33483351                FEB58C12187B8B160098EF0B /* ErrorHandlingScope.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ErrorHandlingScope.cpp; sourceTree = "<group>"; };
    33493352                FEB58C13187B8B160098EF0B /* ErrorHandlingScope.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ErrorHandlingScope.h; sourceTree = "<group>"; };
    … …  
    37163719                                C2181FC018A948FB0025A235 /* JSExportTests.h */,
    37173720                                C2181FC118A948FB0025A235 /* JSExportTests.mm */,
     3721                                FEB51F6A1A97B688001F921C /* Regress141809.h */,
     3722                                FEB51F6B1A97B688001F921C /* Regress141809.mm */,
    37183723                                144005170A531CB50005F061 /* minidom */,
    37193724                                14BD5A2D0A3E91F600BAF59C /* testapi.c */,
    … …  
    67366741                        files = (
    67376742                                C29ECB031804D0ED00D2CBB4 /* CurrentThisInsideBlockGetterTest.mm in Sources */,
     6743                                FEB51F6C1A97B688001F921C /* Regress141809.mm in Sources */,
    67386744                                C20328201981979D0088B499 /* CustomGlobalObjectClassTest.c in Sources */,
    67396745                                C288B2DE18A54D3E007BE40B /* DateTests.mm in Sources */,
Note: See TracChangeset for help on using the changeset viewer.