Title: [146558] trunk/Source/_javascript_Core
Revision
146558
Author
mhahnenb...@apple.com
Date
2013-03-21 20:12:49 -0700 (Thu, 21 Mar 2013)

Log Message

Objective-C API: Need a good way to preserve custom properties on JS wrappers
https://bugs.webkit.org/show_bug.cgi?id=112608

Reviewed by Geoffrey Garen.

Currently, we just use a weak map, which means that garbage collection can cause a wrapper to
disappear if it isn't directly exported to _javascript_.

The most straightforward and safe way (with respect to garbage collection and concurrency) is to have
clients add and remove their external references along with their owners. Effectively, the client is
recording the structure of the external object graph so that the garbage collector can make sure to
mark any wrappers that are reachable through either the JS object graph of the external Obj-C object
graph. By keeping these wrappers alive, this has the effect that custom properties on these wrappers
will also remain alive.

The rule for if an object needs to be tracked by the runtime (and therefore whether the client should report it) is as follows:
For a particular object, its references to its children should be added if:
1. The child is referenced from _javascript_.
2. The child contains references to other objects for which (1) or (2) are true.

* API/JSAPIWrapperObject.mm:
(JSAPIWrapperObjectHandleOwner::finalize):
(JSAPIWrapperObjectHandleOwner::isReachableFromOpaqueRoots): A wrapper object is kept alive only if its JSGlobalObject
is marked and its corresponding Objective-C object was added to the set of opaque roots.
(JSC::JSAPIWrapperObject::visitChildren): We now call out to scanExternalObjectGraph, which handles adding all Objective-C
objects to the set of opaque roots.
* API/JSAPIWrapperObject.h:
(JSAPIWrapperObject):
* API/JSContext.mm: Moved dealloc to its proper place in the main implementation.
(-[JSContext dealloc]):
* API/JSVirtualMachine.h:
* API/JSVirtualMachine.mm:
(-[JSVirtualMachine initWithContextGroupRef:]):
(-[JSVirtualMachine dealloc]):
(getInternalObjcObject): Helper funciton to get the Objective-C object out of JSManagedValues or JSValues if there is one.
(-[JSVirtualMachine addManagedReference:withOwner:]): Adds the Objective-C object to the set of objects
owned by the owner object in that particular virtual machine.
(-[JSVirtualMachine removeManagedReference:withOwner:]): Removes the relationship between the two objects.
(-[JSVirtualMachine externalObjectGraph]):
(scanExternalObjectGraph): Does a depth-first search of the external object graph in a particular virtual machine starting at
the specified root. Each new object it encounters it adds to the set of opaque roots. These opaque roots will keep their
corresponding wrapper objects alive if they have them.
* API/JSManagedReferenceInternal.h: Added.
* API/JSVirtualMachine.mm: Added the per-JSVirtualMachine map between objects and the objects they own, which is more formally
known as that virtual machine's external object graph.
* API/JSWrapperMap.mm:
(-[JSWrapperMap dealloc]): We were leaking this before :-(
(-[JSVirtualMachine initWithContextGroupRef:]):
(-[JSVirtualMachine dealloc]):
(-[JSVirtualMachine externalObjectGraph]):
* API/JSVirtualMachineInternal.h:
* API/tests/testapi.mm: Added two new tests using the TinyDOMNode class. The first tests that a custom property added to a wrapper
doesn't vanish after GC, even though that wrapper isn't directly accessible to the JS garbage collector but is accessible through
the external Objective-C object graph. The second test makes sure that adding an object to the external object graph with the same
owner doesn't cause any sort of problems.
(+[TinyDOMNode sharedVirtualMachine]):
(-[TinyDOMNode init]):
(-[TinyDOMNode dealloc]):
(-[TinyDOMNode appendChild:]):
(-[TinyDOMNode numberOfChildren]):
(-[TinyDOMNode childAtIndex:]):
(-[TinyDOMNode removeChildAtIndex:]):
* _javascript_Core.xcodeproj/project.pbxproj:
* heap/SlotVisitor.h:
(SlotVisitor):
* heap/SlotVisitorInlines.h:
(JSC::SlotVisitor::containsOpaqueRootTriState): Added a new method to SlotVisitor to allow scanExternalObjectGraph to have a
thread-safe view of opaque roots during parallel marking. The set of opaque roots available to any one SlotVisitor isn't guaranteed
to be 100% correct, but that just results in a small duplication of work in scanExternalObjectGraph. To indicate this change for
false negatives we return a TriState that's either true or mixed, but never false.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/API/JSAPIWrapperObject.h (146557 => 146558)


--- trunk/Source/_javascript_Core/API/JSAPIWrapperObject.h	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/JSAPIWrapperObject.h	2013-03-22 03:12:49 UTC (rev 146558)
@@ -55,6 +55,6 @@
 
 } // namespace JSC
 
-#endif
+#endif // JSC_OBJC_API_ENABLED
 
 #endif // JSAPIWrapperObject_h

Modified: trunk/Source/_javascript_Core/API/JSAPIWrapperObject.mm (146557 => 146558)


--- trunk/Source/_javascript_Core/API/JSAPIWrapperObject.mm	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/JSAPIWrapperObject.mm	2013-03-22 03:12:49 UTC (rev 146558)
@@ -29,6 +29,7 @@
 #include "JSCJSValueInlines.h"
 #include "JSCallbackObject.h"
 #include "JSCellInlines.h"
+#include "JSVirtualMachineInternal.h"
 #include "SlotVisitorInlines.h"
 #include "Structure.h"
 #include "StructureInlines.h"
@@ -38,6 +39,7 @@
 class JSAPIWrapperObjectHandleOwner : public JSC::WeakHandleOwner {
 public:
     virtual void finalize(JSC::Handle<JSC::Unknown>, void*);
+    virtual bool isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void* context, JSC::SlotVisitor&);
 };
 
 static JSAPIWrapperObjectHandleOwner* jsAPIWrapperObjectHandleOwner()
@@ -48,13 +50,23 @@
 
 void JSAPIWrapperObjectHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void*)
 {
-    JSC::WeakSet::deallocate(JSC::WeakImpl::asWeakImpl(handle.slot()));
     JSC::JSAPIWrapperObject* wrapperObject = JSC::jsCast<JSC::JSAPIWrapperObject*>(handle.get().asCell());
     if (!wrapperObject->wrappedObject())
         return;
     [static_cast<id>(wrapperObject->wrappedObject()) release];
+    JSC::WeakSet::deallocate(JSC::WeakImpl::asWeakImpl(handle.slot()));
 }
 
+bool JSAPIWrapperObjectHandleOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, JSC::SlotVisitor& visitor)
+{
+    JSC::JSAPIWrapperObject* wrapperObject = JSC::jsCast<JSC::JSAPIWrapperObject*>(handle.get().asCell());
+    // We use the JSGlobalObject when processing weak handles to prevent the situation where using
+    // the same Objective-C object in multiple global objects keeps all of the global objects alive.
+    if (!wrapperObject->wrappedObject())
+        return false;
+    return JSC::Heap::isMarked(wrapperObject->structure()->globalObject()) && visitor.containsOpaqueRoot(wrapperObject->wrappedObject());
+}
+
 namespace JSC {
     
 template <> const ClassInfo JSCallbackObject<JSAPIWrapperObject>::s_info = { "JSAPIWrapperObject", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(JSCallbackObject) };
@@ -92,7 +104,7 @@
     Base::visitChildren(cell, visitor);
 
     if (thisObject->wrappedObject())
-        visitor.addOpaqueRoot(thisObject->wrappedObject());
+        scanExternalObjectGraph(cell->structure()->globalObject()->globalData(), visitor, thisObject->wrappedObject());
 }
 
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/API/JSContext.mm (146557 => 146558)


--- trunk/Source/_javascript_Core/API/JSContext.mm	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/JSContext.mm	2013-03-22 03:12:49 UTC (rev 146558)
@@ -78,6 +78,15 @@
     return self;
 }
 
+- (void)dealloc
+{
+    [m_wrapperMap release];
+    JSGlobalContextRelease(m_context);
+    [m_virtualMachine release];
+    [self.exceptionHandler release];
+    [super dealloc];
+}
+
 - (JSValue *)evaluateScript:(NSString *)script
 {
     JSValueRef exceptionValue = 0;
@@ -195,15 +204,6 @@
     return self;
 }
 
-- (void)dealloc
-{
-    [m_wrapperMap release];
-    JSGlobalContextRelease(m_context);
-    [m_virtualMachine release];
-    [self.exceptionHandler release];
-    [super dealloc];
-}
-
 - (void)notifyException:(JSValueRef)exceptionValue
 {
     self.exceptionHandler(self, [JSValue valueWithValue:exceptionValue inContext:self]);

Modified: trunk/Source/_javascript_Core/API/JSVirtualMachine.h (146557 => 146558)


--- trunk/Source/_javascript_Core/API/JSVirtualMachine.h	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/JSVirtualMachine.h	2013-03-22 03:12:49 UTC (rev 146558)
@@ -37,6 +37,9 @@
 
 - (id)init;
 
+- (void)addManagedReference:(id)object withOwner:(id)owner;
+- (void)removeManagedReference:(id)object withOwner:(id)owner;
+
 @end
 
 #endif

Modified: trunk/Source/_javascript_Core/API/JSVirtualMachine.mm (146557 => 146558)


--- trunk/Source/_javascript_Core/API/JSVirtualMachine.mm	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/JSVirtualMachine.mm	2013-03-22 03:12:49 UTC (rev 146558)
@@ -30,7 +30,10 @@
 #if JSC_OBJC_API_ENABLED
 
 #import "APICast.h"
+#import "APIShims.h"
+#import "JSVirtualMachine.h"
 #import "JSVirtualMachineInternal.h"
+#import "JSWrapperMap.h"
 
 static NSMapTable *globalWrapperCache = 0;
 
@@ -79,6 +82,7 @@
 @implementation JSVirtualMachine {
     JSContextGroupRef m_group;
     NSMapTable *m_contextCache;
+    NSMapTable *m_externalObjectGraph;
 }
 
 - (id)init
@@ -101,22 +105,90 @@
     NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
     NSPointerFunctionsOptions valueOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
     m_contextCache = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
+    
+    NSPointerFunctionsOptions weakIDOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
+    NSPointerFunctionsOptions strongIDOptions = NSPointerFunctionsStrongMemory | NSPointerFunctionsObjectPersonality;
+    m_externalObjectGraph = [[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:strongIDOptions capacity:0];
    
     [JSVMWrapperCache addWrapper:self forJSContextGroupRef:group];
  
     return self;
 }
 
-@end
-
-@implementation JSVirtualMachine(Internal)
-
 - (void)dealloc
 {
     JSContextGroupRelease(m_group);
+    [m_contextCache release];
+    [m_externalObjectGraph release];
     [super dealloc];
 }
 
+static id getInternalObjcObject(id object)
+{
+    if ([object isKindOfClass:[JSManagedValue class]])
+        object = [static_cast<JSManagedValue *>(object) value];
+    
+    if ([object isKindOfClass:[JSValue class]]) {
+        JSValue *value = static_cast<JSValue *>(object);
+        object = tryUnwrapObjcObject([value.context globalContextRef], [value JSValueRef]);
+    }
+    
+    return object;
+}
+
+- (void)addManagedReference:(id)object withOwner:(id)owner
+{    
+    object = getInternalObjcObject(object);
+    owner = getInternalObjcObject(owner);
+    
+    if (!object || !owner)
+        return;
+    
+    JSC::APIEntryShim shim(toJS(m_group));
+    
+    NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
+    if (!ownedObjects) {
+        NSPointerFunctionsOptions weakIDOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
+        NSPointerFunctionsOptions integerOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsIntegerPersonality;
+        ownedObjects = [[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:integerOptions capacity:1];
+
+        [m_externalObjectGraph setObject:ownedObjects forKey:owner];
+        [ownedObjects release];
+    }
+    NSMapInsert(ownedObjects, object, reinterpret_cast<void*>(reinterpret_cast<size_t>(NSMapGet(ownedObjects, object)) + 1));
+}
+
+- (void)removeManagedReference:(id)object withOwner:(id)owner
+{
+    object = getInternalObjcObject(object);
+    owner = getInternalObjcObject(owner);
+    
+    if (!object || !owner)
+        return;
+    
+    JSC::APIEntryShim shim(toJS(m_group));
+    
+    NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
+    if (!ownedObjects)
+        return;
+   
+    size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects, object));
+    if (count > 1) {
+        NSMapInsert(ownedObjects, object, reinterpret_cast<void*>(count - 1));
+        return;
+    }
+    
+    if (count == 1)
+        NSMapRemove(ownedObjects, object);
+
+    if (![ownedObjects count])
+        [m_externalObjectGraph removeObjectForKey:owner];
+}
+
+@end
+
+@implementation JSVirtualMachine(Internal)
+
 JSContextGroupRef getGroupFromVirtualMachine(JSVirtualMachine *virtualMachine)
 {
     return virtualMachine->m_group;
@@ -140,8 +212,37 @@
     NSMapInsert(m_contextCache, globalContext, wrapper);
 }
 
+- (NSMapTable *)externalObjectGraph
+{
+    return m_externalObjectGraph;
+}
+
 @end
 
+void scanExternalObjectGraph(JSC::JSGlobalData& globalData, JSC::SlotVisitor& visitor, void* root)
+{
+    @autoreleasepool {
+        JSVirtualMachine *virtualMachine = [JSVirtualMachine virtualMachineWithContextGroupRef:toRef(&globalData)];
+        NSMapTable *externalObjectGraph = [virtualMachine externalObjectGraph];
+        Vector<void*> stack;
+        stack.append(root);
+        while (!stack.isEmpty()) {
+            void* nextRoot = stack.last();
+            stack.removeLast();
+            if (visitor.containsOpaqueRootTriState(nextRoot) == TrueTriState)
+                continue;
+            visitor.addOpaqueRoot(nextRoot);
+            
+            NSMapTable *ownedObjects = [externalObjectGraph objectForKey:static_cast<id>(nextRoot)];
+            id ownedObject;
+            NSEnumerator *enumerator = [ownedObjects keyEnumerator];
+            while ((ownedObject = [enumerator nextObject])) {
+                ASSERT(reinterpret_cast<size_t>(NSMapGet(ownedObjects, ownedObject)) == 1);
+                stack.append(static_cast<void*>(ownedObject));
+            }
+        }
+    }
+}
 
 #endif
 

Modified: trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h (146557 => 146558)


--- trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h	2013-03-22 03:12:49 UTC (rev 146558)
@@ -26,11 +26,16 @@
 #ifndef JSVirtualMachineInternal_h
 #define JSVirtualMachineInternal_h
 
-#import <_javascript_Core/JSVirtualMachine.h>
 #import <_javascript_Core/_javascript_Core.h>
 
 #if JSC_OBJC_API_ENABLED
 
+namespace JSC {
+class JSGlobalData;
+class SlotVisitor;
+}
+
+#if defined(__OBJC__)
 @interface JSVirtualMachine(Internal)
 
 JSContextGroupRef getGroupFromVirtualMachine(JSVirtualMachine *);
@@ -40,8 +45,13 @@
 - (JSContext *)contextForGlobalContextRef:(JSGlobalContextRef)globalContext;
 - (void)addContext:(JSContext *)wrapper forGlobalContextRef:(JSGlobalContextRef)globalContext;
 
+- (NSMapTable *)externalObjectGraph;
+
 @end
+#endif // defined(__OBJC__)
 
+void scanExternalObjectGraph(JSC::JSGlobalData&, JSC::SlotVisitor&, void* root);
+
 #endif
 
 #endif // JSVirtualMachineInternal_h

Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.mm (146557 => 146558)


--- trunk/Source/_javascript_Core/API/JSWrapperMap.mm	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.mm	2013-03-22 03:12:49 UTC (rev 146558)
@@ -434,6 +434,7 @@
 
 - (void)dealloc
 {
+    [m_cachedObjCWrappers release];
     [m_classMap release];
     [super dealloc];
 }

Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (146557 => 146558)


--- trunk/Source/_javascript_Core/API/tests/testapi.mm	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm	2013-03-22 03:12:49 UTC (rev 146558)
@@ -153,6 +153,85 @@
 }
 @end
 
+@class TinyDOMNode;
+
+@protocol TinyDOMNode <JSExport>
+- (void)appendChild:(TinyDOMNode *)child;
+- (NSUInteger)numberOfChildren;
+- (TinyDOMNode *)childAtIndex:(NSUInteger)index;
+- (void)removeChildAtIndex:(NSUInteger)index;
+@end
+
+@interface TinyDOMNode : NSObject<TinyDOMNode>
++ (JSVirtualMachine *)sharedVirtualMachine;
++ (void)clearSharedVirtualMachine;
+@end
+
+@implementation TinyDOMNode {
+    NSMutableArray *m_children;
+}
+
+static JSVirtualMachine *sharedInstance = nil;
+
++ (JSVirtualMachine *)sharedVirtualMachine
+{
+    if (!sharedInstance)
+        sharedInstance = [[JSVirtualMachine alloc] init];
+    return sharedInstance;
+}
+
++ (void)clearSharedVirtualMachine
+{
+    sharedInstance = nil;
+}
+
+- (id)init
+{
+    self = [super init];
+    if (!self)
+        return nil;
+
+    m_children = [[NSMutableArray alloc] initWithCapacity:0];
+
+    return self;
+}
+
+- (void)dealloc
+{
+    NSEnumerator *enumerator = [m_children objectEnumerator];
+    id nextChild;
+    while ((nextChild = [enumerator nextObject]))
+        [[TinyDOMNode sharedVirtualMachine] removeManagedReference:nextChild withOwner:self];
+}
+
+- (void)appendChild:(TinyDOMNode *)child
+{
+    [[TinyDOMNode sharedVirtualMachine] addManagedReference:child withOwner:self];
+    [m_children addObject:child];
+}
+
+- (NSUInteger)numberOfChildren
+{
+    return [m_children count];
+}
+
+- (TinyDOMNode *)childAtIndex:(NSUInteger)index
+{
+    if (index >= [m_children count])
+        return nil;
+    return [m_children objectAtIndex:index];
+}
+
+- (void)removeChildAtIndex:(NSUInteger)index
+{
+    if (index >= [m_children count])
+        return;
+    [[TinyDOMNode sharedVirtualMachine] removeManagedReference:[m_children objectAtIndex:index] withOwner:self];
+    [m_children removeObjectAtIndex:index];
+}
+
+@end
+
 static void checkResult(NSString *description, bool passed)
 {
     NSLog(@"TEST: \"%@\": %@", description, passed ? @"PASSED" : @"FAILED");
@@ -648,6 +727,72 @@
         }
     }
 
+    @autoreleasepool {
+        JSVirtualMachine *vm = [TinyDOMNode sharedVirtualMachine];
+        JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
+        TinyDOMNode *root = [[TinyDOMNode alloc] init];
+        TinyDOMNode *lastNode = root;
+        for (NSUInteger i = 0; i < 3; i++) {
+            TinyDOMNode *newNode = [[TinyDOMNode alloc] init];
+            [lastNode appendChild:newNode];
+            lastNode = newNode;
+        }
+
+        @autoreleasepool {
+            context[@"root"] = root;
+            context[@"getLastNodeInChain"] = ^(TinyDOMNode *head){
+                TinyDOMNode *lastNode = nil;
+                while (head) {
+                    lastNode = head;
+                    head = [lastNode childAtIndex:0];
+                }
+                return lastNode;
+            };
+            [context evaluateScript:@"getLastNodeInChain(root).myCustomProperty = 42;"];
+        }
+
+        JSSynchronousGarbageCollectForDebugging([context globalContextRef]);
+
+        JSValue *myCustomProperty = [context evaluateScript:@"getLastNodeInChain(root).myCustomProperty"];
+        checkResult(@"My custom property == 42", [myCustomProperty isNumber] && [myCustomProperty toInt32] == 42);
+
+        [TinyDOMNode clearSharedVirtualMachine];
+    }
+
+    @autoreleasepool {
+        JSVirtualMachine *vm = [TinyDOMNode sharedVirtualMachine];
+        JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
+        TinyDOMNode *root = [[TinyDOMNode alloc] init];
+        TinyDOMNode *lastNode = root;
+        for (NSUInteger i = 0; i < 3; i++) {
+            TinyDOMNode *newNode = [[TinyDOMNode alloc] init];
+            [lastNode appendChild:newNode];
+            lastNode = newNode;
+        }
+
+        @autoreleasepool {
+            context[@"root"] = root;
+            context[@"getLastNodeInChain"] = ^(TinyDOMNode *head){
+                TinyDOMNode *lastNode = nil;
+                while (head) {
+                    lastNode = head;
+                    head = [lastNode childAtIndex:0];
+                }
+                return lastNode;
+            };
+            [context evaluateScript:@"getLastNodeInChain(root).myCustomProperty = 42;"];
+
+            [root appendChild:[root childAtIndex:0]];
+            [root removeChildAtIndex:0];
+        }
+
+        JSSynchronousGarbageCollectForDebugging([context globalContextRef]);
+
+        JSValue *myCustomProperty = [context evaluateScript:@"getLastNodeInChain(root).myCustomProperty"];
+        checkResult(@"duplicate calls to addManagedReference don't cause things to die", [myCustomProperty isNumber] && [myCustomProperty toInt32] == 42);
+
+        [TinyDOMNode clearSharedVirtualMachine];
+    }
 }
 
 #else

Modified: trunk/Source/_javascript_Core/ChangeLog (146557 => 146558)


--- trunk/Source/_javascript_Core/ChangeLog	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-03-22 03:12:49 UTC (rev 146558)
@@ -1,3 +1,76 @@
+2013-03-21  Mark Hahnenberg  <mhahnenb...@apple.com>
+
+        Objective-C API: Need a good way to preserve custom properties on JS wrappers
+        https://bugs.webkit.org/show_bug.cgi?id=112608
+
+        Reviewed by Geoffrey Garen.
+
+        Currently, we just use a weak map, which means that garbage collection can cause a wrapper to 
+        disappear if it isn't directly exported to _javascript_.
+
+        The most straightforward and safe way (with respect to garbage collection and concurrency) is to have 
+        clients add and remove their external references along with their owners. Effectively, the client is 
+        recording the structure of the external object graph so that the garbage collector can make sure to 
+        mark any wrappers that are reachable through either the JS object graph of the external Obj-C object 
+        graph. By keeping these wrappers alive, this has the effect that custom properties on these wrappers 
+        will also remain alive.
+
+        The rule for if an object needs to be tracked by the runtime (and therefore whether the client should report it) is as follows:
+        For a particular object, its references to its children should be added if:
+        1. The child is referenced from _javascript_.
+        2. The child contains references to other objects for which (1) or (2) are true.
+
+        * API/JSAPIWrapperObject.mm:
+        (JSAPIWrapperObjectHandleOwner::finalize):
+        (JSAPIWrapperObjectHandleOwner::isReachableFromOpaqueRoots): A wrapper object is kept alive only if its JSGlobalObject
+        is marked and its corresponding Objective-C object was added to the set of opaque roots.
+        (JSC::JSAPIWrapperObject::visitChildren): We now call out to scanExternalObjectGraph, which handles adding all Objective-C
+        objects to the set of opaque roots.
+        * API/JSAPIWrapperObject.h:
+        (JSAPIWrapperObject):
+        * API/JSContext.mm: Moved dealloc to its proper place in the main implementation.
+        (-[JSContext dealloc]):
+        * API/JSVirtualMachine.h:
+        * API/JSVirtualMachine.mm:
+        (-[JSVirtualMachine initWithContextGroupRef:]):
+        (-[JSVirtualMachine dealloc]):
+        (getInternalObjcObject): Helper funciton to get the Objective-C object out of JSManagedValues or JSValues if there is one.
+        (-[JSVirtualMachine addManagedReference:withOwner:]): Adds the Objective-C object to the set of objects 
+        owned by the owner object in that particular virtual machine.
+        (-[JSVirtualMachine removeManagedReference:withOwner:]): Removes the relationship between the two objects.
+        (-[JSVirtualMachine externalObjectGraph]):
+        (scanExternalObjectGraph): Does a depth-first search of the external object graph in a particular virtual machine starting at
+        the specified root. Each new object it encounters it adds to the set of opaque roots. These opaque roots will keep their 
+        corresponding wrapper objects alive if they have them. 
+        * API/JSManagedReferenceInternal.h: Added.
+        * API/JSVirtualMachine.mm: Added the per-JSVirtualMachine map between objects and the objects they own, which is more formally
+        known as that virtual machine's external object graph.
+        * API/JSWrapperMap.mm:
+        (-[JSWrapperMap dealloc]): We were leaking this before :-(
+        (-[JSVirtualMachine initWithContextGroupRef:]):
+        (-[JSVirtualMachine dealloc]):
+        (-[JSVirtualMachine externalObjectGraph]):
+        * API/JSVirtualMachineInternal.h:
+        * API/tests/testapi.mm: Added two new tests using the TinyDOMNode class. The first tests that a custom property added to a wrapper 
+        doesn't vanish after GC, even though that wrapper isn't directly accessible to the JS garbage collector but is accessible through 
+        the external Objective-C object graph. The second test makes sure that adding an object to the external object graph with the same 
+        owner doesn't cause any sort of problems.
+        (+[TinyDOMNode sharedVirtualMachine]):
+        (-[TinyDOMNode init]):
+        (-[TinyDOMNode dealloc]):
+        (-[TinyDOMNode appendChild:]):
+        (-[TinyDOMNode numberOfChildren]):
+        (-[TinyDOMNode childAtIndex:]):
+        (-[TinyDOMNode removeChildAtIndex:]):
+        * _javascript_Core.xcodeproj/project.pbxproj:
+        * heap/SlotVisitor.h:
+        (SlotVisitor):
+        * heap/SlotVisitorInlines.h:
+        (JSC::SlotVisitor::containsOpaqueRootTriState): Added a new method to SlotVisitor to allow scanExternalObjectGraph to have a 
+        thread-safe view of opaque roots during parallel marking. The set of opaque roots available to any one SlotVisitor isn't guaranteed
+        to be 100% correct, but that just results in a small duplication of work in scanExternalObjectGraph. To indicate this change for
+        false negatives we return a TriState that's either true or mixed, but never false.
+
 2013-03-21  Mark Lam  <mark....@apple.com>
 
         Fix O(n^2) op_debug bytecode charPosition to column computation.

Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.h (146557 => 146558)


--- trunk/Source/_javascript_Core/heap/SlotVisitor.h	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h	2013-03-22 03:12:49 UTC (rev 146558)
@@ -62,6 +62,7 @@
     
     void addOpaqueRoot(void*);
     bool containsOpaqueRoot(void*);
+    TriState containsOpaqueRootTriState(void*);
     int opaqueRootCount();
 
     GCThreadSharedData& sharedData() { return m_shared; }

Modified: trunk/Source/_javascript_Core/heap/SlotVisitorInlines.h (146557 => 146558)


--- trunk/Source/_javascript_Core/heap/SlotVisitorInlines.h	2013-03-22 03:11:39 UTC (rev 146557)
+++ trunk/Source/_javascript_Core/heap/SlotVisitorInlines.h	2013-03-22 03:12:49 UTC (rev 146558)
@@ -121,6 +121,16 @@
 #endif
 }
 
+inline TriState SlotVisitor::containsOpaqueRootTriState(void* root)
+{
+    if (m_opaqueRoots.contains(root))
+        return TrueTriState;
+    MutexLocker locker(m_shared.m_opaqueRootsLock);
+    if (m_shared.m_opaqueRoots.contains(root))
+        return TrueTriState;
+    return MixedTriState;
+}
+
 inline int SlotVisitor::opaqueRootCount()
 {
     ASSERT(!m_isInParallelMode);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to