Changeset 167326 in webkit


Ignore:
Timestamp:
Apr 15, 2014 2:05:09 PM (10 years ago)
Author:
mhahnenberg@apple.com
Message:

Objective-C API external object graphs don't handle generational collection properly
https://bugs.webkit.org/show_bug.cgi?id=131634

Reviewed by Geoffrey Garen.

If the set of Objective-C objects transitively reachable through an object changes, we
need to update the set of opaque roots accordingly. If we don't, the next EdenCollection
won't rescan the external object graph, which would lead us to consider a newly allocated
JSManagedValue to be dead.

  • API/JSBase.cpp:

(JSSynchronousEdenCollectForDebugging):

  • API/JSVirtualMachine.mm:

(-[JSVirtualMachine initWithContextGroupRef:]):
(-[JSVirtualMachine dealloc]):
(-[JSVirtualMachine isOldExternalObject:]):
(-[JSVirtualMachine addExternalRememberedObject:]):
(-[JSVirtualMachine addManagedReference:withOwner:]):
(-[JSVirtualMachine removeManagedReference:withOwner:]):
(-[JSVirtualMachine externalRememberedSet]):
(scanExternalObjectGraph):
(scanExternalRememberedSet):

  • API/JSVirtualMachineInternal.h:
  • API/tests/testapi.mm:
  • heap/Heap.cpp:

(JSC::Heap::markRoots):

  • heap/Heap.h:

(JSC::Heap::slotVisitor):

  • heap/SlotVisitor.h:
  • heap/SlotVisitorInlines.h:

(JSC::SlotVisitor::containsOpaqueRoot):
(JSC::SlotVisitor::containsOpaqueRootTriState):

Location:
trunk/Source/JavaScriptCore
Files:
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/API/JSBase.cpp

    r165676 r167326  
    143143
    144144extern "C" JS_EXPORT void JSSynchronousGarbageCollectForDebugging(JSContextRef);
     145extern "C" JS_EXPORT void JSSynchronousEdenCollectForDebugging(JSContextRef);
    145146
    146147void JSSynchronousGarbageCollectForDebugging(JSContextRef ctx)
     
    152153    JSLockHolder locker(exec);
    153154    exec->vm().heap.collectAllGarbage();
     155}
     156
     157void JSSynchronousEdenCollectForDebugging(JSContextRef ctx)
     158{
     159    if (!ctx)
     160        return;
     161
     162    ExecState* exec = toJS(ctx);
     163    JSLockHolder locker(exec);
     164    exec->vm().heap.collect(EdenCollection);
    154165}
    155166
  • trunk/Source/JavaScriptCore/API/JSVirtualMachine.mm

    r166835 r167326  
    8888    NSMapTable *m_contextCache;
    8989    NSMapTable *m_externalObjectGraph;
     90    NSMapTable *m_externalRememberedSet;
    9091}
    9192
     
    114115    NSPointerFunctionsOptions strongIDOptions = NSPointerFunctionsStrongMemory | NSPointerFunctionsObjectPersonality;
    115116    m_externalObjectGraph = [[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:strongIDOptions capacity:0];
     117
     118    NSPointerFunctionsOptions integerOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsIntegerPersonality;
     119    m_externalRememberedSet = [[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:integerOptions capacity:0];
    116120   
    117121    [JSVMWrapperCache addWrapper:self forJSContextGroupRef:group];
     
    125129    [m_contextCache release];
    126130    [m_externalObjectGraph release];
     131    [m_externalRememberedSet release];
    127132    [super dealloc];
    128133}
     
    144149
    145150    return object;
     151}
     152
     153- (bool)isOldExternalObject:(id)object
     154{
     155    JSC::VM* vm = toJS(m_group);
     156    return vm->heap.slotVisitor().containsOpaqueRoot(object);
     157}
     158
     159- (void)addExternalRememberedObject:(id)object
     160{
     161    ASSERT([self isOldExternalObject:object]);
     162    [m_externalRememberedSet setObject:[NSNumber numberWithBool:true] forKey:object];
    146163}
    147164
     
    158175   
    159176    JSC::JSLockHolder locker(toJS(m_group));
    160    
     177    if ([self isOldExternalObject:owner] && ![self isOldExternalObject:object])
     178        [self addExternalRememberedObject:owner];
     179 
    161180    NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
    162181    if (!ownedObjects) {
     
    199218        NSMapRemove(ownedObjects, object);
    200219
    201     if (![ownedObjects count])
     220    if (![ownedObjects count]) {
    202221        [m_externalObjectGraph removeObjectForKey:owner];
     222        [m_externalRememberedSet removeObjectForKey:owner];
     223    }
    203224}
    204225
     
    233254{
    234255    return m_externalObjectGraph;
     256}
     257
     258- (NSMapTable *)externalRememberedSet
     259{
     260    return m_externalRememberedSet;
    235261}
    236262
     
    254280           
    255281            NSMapTable *ownedObjects = [externalObjectGraph objectForKey:static_cast<id>(nextRoot)];
    256             id ownedObject;
    257             NSEnumerator *enumerator = [ownedObjects keyEnumerator];
    258             while ((ownedObject = [enumerator nextObject]))
     282            for (id ownedObject in ownedObjects)
    259283                stack.append(static_cast<void*>(ownedObject));
    260284        }
     
    262286}
    263287
    264 #endif
    265 
     288void scanExternalRememberedSet(JSC::VM& vm, JSC::SlotVisitor& visitor)
     289{
     290    @autoreleasepool {
     291        JSVirtualMachine *virtualMachine = [JSVMWrapperCache wrapperForJSContextGroupRef:toRef(&vm)];
     292        if (!virtualMachine)
     293            return;
     294        NSMapTable *externalObjectGraph = [virtualMachine externalObjectGraph];
     295        NSMapTable *externalRememberedSet = [virtualMachine externalRememberedSet];
     296        for (id key in externalRememberedSet) {
     297            NSMapTable *ownedObjects = [externalObjectGraph objectForKey:key];
     298            for (id ownedObject in ownedObjects)
     299                scanExternalObjectGraph(vm, visitor, ownedObject);
     300        }
     301        [externalRememberedSet removeAllObjects];
     302    }
     303}
     304
     305#endif // JSC_OBJC_API_ENABLED
  • trunk/Source/JavaScriptCore/API/JSVirtualMachineInternal.h

    r148696 r167326  
    2727#define JSVirtualMachineInternal_h
    2828
     29#if JSC_OBJC_API_ENABLED
     30
    2931#import <JavaScriptCore/JavaScriptCore.h>
    30 
    31 #if JSC_OBJC_API_ENABLED
    3232
    3333namespace JSC {
     
    5252
    5353void scanExternalObjectGraph(JSC::VM&, JSC::SlotVisitor&, void* root);
     54void scanExternalRememberedSet(JSC::VM&, JSC::SlotVisitor&);
    5455
    55 #endif
     56#endif // JSC_OBJC_API_ENABLED
    5657
    5758#endif // JSVirtualMachineInternal_h
  • trunk/Source/JavaScriptCore/API/tests/testapi.mm

    r166835 r167326  
    3131
    3232extern "C" void JSSynchronousGarbageCollectForDebugging(JSContextRef);
     33extern "C" void JSSynchronousEdenCollectForDebugging(JSContextRef);
    3334
    3435extern "C" bool _Block_has_signature(id);
     
    13271328    }
    13281329
     1330    @autoreleasepool {
     1331        JSContext *context = [[JSContext alloc] init];
     1332
     1333        // Create the root, make it reachable from JS, and force an EdenCollection
     1334        // so that we scan the external object graph.
     1335        TestObject *root = [TestObject testObject];
     1336        @autoreleasepool {
     1337            context[@"root"] = root;
     1338        }
     1339        JSSynchronousEdenCollectForDebugging([context JSGlobalContextRef]);
     1340
     1341        // Create a new Obj-C object only reachable via the external object graph
     1342        // through the object we already scanned during the EdenCollection.
     1343        TestObject *child = [TestObject testObject];
     1344        [context.virtualMachine addManagedReference:child withOwner:root];
     1345
     1346        // Create a new managed JSValue that will only be kept alive if we properly rescan
     1347        // the external object graph.
     1348        JSManagedValue *managedJSObject = nil;
     1349        @autoreleasepool {
     1350            JSValue *jsObject = [JSValue valueWithObject:@"hello" inContext:context];
     1351            managedJSObject = [JSManagedValue managedValueWithValue:jsObject];
     1352            [context.virtualMachine addManagedReference:managedJSObject withOwner:child];
     1353        }
     1354
     1355        // Force another EdenCollection. It should rescan the new part of the external object graph.
     1356        JSSynchronousEdenCollectForDebugging([context JSGlobalContextRef]);
     1357       
     1358        // Check that the managed JSValue is still alive.
     1359        checkResult(@"EdenCollection doesn't reclaim new managed values", [managedJSObject value] != nil);
     1360    }
     1361
    13291362    currentThisInsideBlockGetterTest();
    13301363    runDateTests();
  • trunk/Source/JavaScriptCore/ChangeLog

    r167325 r167326  
     12014-04-15  Mark Hahnenberg  <mhahnenberg@apple.com>
     2
     3        Objective-C API external object graphs don't handle generational collection properly
     4        https://bugs.webkit.org/show_bug.cgi?id=131634
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        If the set of Objective-C objects transitively reachable through an object changes, we
     9        need to update the set of opaque roots accordingly. If we don't, the next EdenCollection
     10        won't rescan the external object graph, which would lead us to consider a newly allocated
     11        JSManagedValue to be dead.
     12
     13        * API/JSBase.cpp:
     14        (JSSynchronousEdenCollectForDebugging):
     15        * API/JSVirtualMachine.mm:
     16        (-[JSVirtualMachine initWithContextGroupRef:]):
     17        (-[JSVirtualMachine dealloc]):
     18        (-[JSVirtualMachine isOldExternalObject:]):
     19        (-[JSVirtualMachine addExternalRememberedObject:]):
     20        (-[JSVirtualMachine addManagedReference:withOwner:]):
     21        (-[JSVirtualMachine removeManagedReference:withOwner:]):
     22        (-[JSVirtualMachine externalRememberedSet]):
     23        (scanExternalObjectGraph):
     24        (scanExternalRememberedSet):
     25        * API/JSVirtualMachineInternal.h:
     26        * API/tests/testapi.mm:
     27        * heap/Heap.cpp:
     28        (JSC::Heap::markRoots):
     29        * heap/Heap.h:
     30        (JSC::Heap::slotVisitor):
     31        * heap/SlotVisitor.h:
     32        * heap/SlotVisitorInlines.h:
     33        (JSC::SlotVisitor::containsOpaqueRoot):
     34        (JSC::SlotVisitor::containsOpaqueRootTriState):
     35
    1362014-04-15  Filip Pizlo  <fpizlo@apple.com>
    237
  • trunk/Source/JavaScriptCore/heap/Heap.cpp

    r167293 r167326  
    4242#include "JSONObject.h"
    4343#include "JSCInlines.h"
     44#include "JSVirtualMachineInternal.h"
    4445#include "RecursiveAllocationScope.h"
    4546#include "Tracing.h"
     
    513514        ParallelModeEnabler enabler(m_slotVisitor);
    514515
     516        visitExternalRememberedSet();
    515517        visitSmallStrings();
    516518        visitConservativeRoots(conservativeRoots);
     
    590592    m_objectSpace.clearNewlyAllocated();
    591593    m_objectSpace.clearMarks();
     594}
     595
     596void Heap::visitExternalRememberedSet()
     597{
     598#if JSC_OBJC_API_ENABLED
     599    scanExternalRememberedSet(*m_vm, m_slotVisitor);
     600#endif
    592601}
    593602
  • trunk/Source/JavaScriptCore/heap/Heap.h

    r166837 r167326  
    115115    MarkedSpace& objectSpace() { return m_objectSpace; }
    116116    MachineThreads& machineThreads() { return m_machineThreads; }
     117
     118    const SlotVisitor& slotVisitor() const { return m_slotVisitor; }
    117119
    118120    JS_EXPORT_PRIVATE GCActivityCallback* fullActivityCallback();
     
    268270    void gatherScratchBufferRoots(ConservativeRoots&);
    269271    void clearLivenessData();
     272    void visitExternalRememberedSet();
    270273    void visitSmallStrings();
    271274    void visitConservativeRoots(ConservativeRoots&);
  • trunk/Source/JavaScriptCore/heap/SlotVisitor.h

    r166837 r167326  
    7979   
    8080    void addOpaqueRoot(void*);
    81     bool containsOpaqueRoot(void*);
    82     TriState containsOpaqueRootTriState(void*);
     81    bool containsOpaqueRoot(void*) const;
     82    TriState containsOpaqueRootTriState(void*) const;
    8383    int opaqueRootCount();
    8484
  • trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h

    r166837 r167326  
    174174}
    175175
    176 inline bool SlotVisitor::containsOpaqueRoot(void* root)
     176inline bool SlotVisitor::containsOpaqueRoot(void* root) const
    177177{
    178178    ASSERT(!m_isInParallelMode);
     
    185185}
    186186
    187 inline TriState SlotVisitor::containsOpaqueRootTriState(void* root)
     187inline TriState SlotVisitor::containsOpaqueRootTriState(void* root) const
    188188{
    189189    if (m_opaqueRoots.contains(root))
Note: See TracChangeset for help on using the changeset viewer.