Modified: trunk/Source/_javascript_Core/API/JSVirtualMachine.mm (210457 => 210458)
--- trunk/Source/_javascript_Core/API/JSVirtualMachine.mm 2017-01-06 23:30:57 UTC (rev 210457)
+++ trunk/Source/_javascript_Core/API/JSVirtualMachine.mm 2017-01-06 23:38:31 UTC (rev 210458)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -81,6 +81,7 @@
@implementation JSVirtualMachine {
JSContextGroupRef m_group;
+ Lock m_externalDataMutex;
NSMapTable *m_contextCache;
NSMapTable *m_externalObjectGraph;
NSMapTable *m_externalRememberedSet;
@@ -156,6 +157,7 @@
- (void)addExternalRememberedObject:(id)object
{
+ auto locker = holdLock(m_externalDataMutex);
ASSERT([self isOldExternalObject:object]);
[m_externalRememberedSet setObject:@YES forKey:object];
}
@@ -175,6 +177,7 @@
if ([self isOldExternalObject:owner] && ![self isOldExternalObject:object])
[self addExternalRememberedObject:owner];
+ auto externalDataMutexLocker = holdLock(m_externalDataMutex);
NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
if (!ownedObjects) {
NSPointerFunctionsOptions weakIDOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
@@ -202,6 +205,7 @@
JSC::JSLockHolder locker(toJS(m_group));
+ auto externalDataMutexLocker = holdLock(m_externalDataMutex);
NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
if (!ownedObjects)
return;
@@ -248,6 +252,11 @@
NSMapInsert(m_contextCache, globalContext, wrapper);
}
+- (Lock&)externalDataMutex
+{
+ return m_externalDataMutex;
+}
+
- (NSMapTable *)externalObjectGraph
{
return m_externalObjectGraph;
@@ -260,7 +269,7 @@
@end
-void scanExternalObjectGraph(JSC::VM& vm, JSC::SlotVisitor& visitor, void* root)
+static void scanExternalObjectGraph(JSC::VM& vm, JSC::SlotVisitor& visitor, void* root, bool lockAcquired)
{
@autoreleasepool {
JSVirtualMachine *virtualMachine = [JSVMWrapperCache wrapperForJSContextGroupRef:toRef(&vm)];
@@ -267,6 +276,7 @@
if (!virtualMachine)
return;
NSMapTable *externalObjectGraph = [virtualMachine externalObjectGraph];
+ Lock& externalDataMutex = [virtualMachine externalDataMutex];
Vector<void*> stack;
stack.append(root);
while (!stack.isEmpty()) {
@@ -275,14 +285,29 @@
if (visitor.containsOpaqueRootTriState(nextRoot) == TrueTriState)
continue;
visitor.addOpaqueRoot(nextRoot);
-
- NSMapTable *ownedObjects = [externalObjectGraph objectForKey:static_cast<id>(nextRoot)];
- for (id ownedObject in ownedObjects)
- stack.append(static_cast<void*>(ownedObject));
+
+ auto appendOwnedObjects = [&] {
+ NSMapTable *ownedObjects = [externalObjectGraph objectForKey:static_cast<id>(nextRoot)];
+ for (id ownedObject in ownedObjects)
+ stack.append(static_cast<void*>(ownedObject));
+ };
+
+ if (lockAcquired)
+ appendOwnedObjects();
+ else {
+ auto locker = holdLock(externalDataMutex);
+ appendOwnedObjects();
+ }
}
}
}
+void scanExternalObjectGraph(JSC::VM& vm, JSC::SlotVisitor& visitor, void* root)
+{
+ bool lockAcquired = false;
+ scanExternalObjectGraph(vm, visitor, root, lockAcquired);
+}
+
void scanExternalRememberedSet(JSC::VM& vm, JSC::SlotVisitor& visitor)
{
@autoreleasepool {
@@ -289,12 +314,15 @@
JSVirtualMachine *virtualMachine = [JSVMWrapperCache wrapperForJSContextGroupRef:toRef(&vm)];
if (!virtualMachine)
return;
+ Lock& externalDataMutex = [virtualMachine externalDataMutex];
+ auto locker = holdLock(externalDataMutex);
NSMapTable *externalObjectGraph = [virtualMachine externalObjectGraph];
NSMapTable *externalRememberedSet = [virtualMachine externalRememberedSet];
for (id key in externalRememberedSet) {
NSMapTable *ownedObjects = [externalObjectGraph objectForKey:key];
+ bool lockAcquired = true;
for (id ownedObject in ownedObjects)
- scanExternalObjectGraph(vm, visitor, ownedObject);
+ scanExternalObjectGraph(vm, visitor, ownedObject, lockAcquired);
}
[externalRememberedSet removeAllObjects];
}
Modified: trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h (210457 => 210458)
--- trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h 2017-01-06 23:30:57 UTC (rev 210457)
+++ trunk/Source/_javascript_Core/API/JSVirtualMachineInternal.h 2017-01-06 23:38:31 UTC (rev 210458)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -47,8 +47,6 @@
- (JSContext *)contextForGlobalContextRef:(JSGlobalContextRef)globalContext;
- (void)addContext:(JSContext *)wrapper forGlobalContextRef:(JSGlobalContextRef)globalContext;
-- (NSMapTable *)externalObjectGraph;
-
@end
#endif // defined(__OBJC__)
Modified: trunk/Source/_javascript_Core/ChangeLog (210457 => 210458)
--- trunk/Source/_javascript_Core/ChangeLog 2017-01-06 23:30:57 UTC (rev 210457)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-01-06 23:38:31 UTC (rev 210458)
@@ -1,3 +1,27 @@
+2017-01-06 Mark Lam <[email protected]>
+
+ The ObjC API's JSVirtualMachine's map tables need to be guarded by a lock.
+ https://bugs.webkit.org/show_bug.cgi?id=166778
+ <rdar://problem/29761198>
+
+ Reviewed by Filip Pizlo.
+
+ Now that we have a concurrent GC, access to JSVirtualMachine's
+ m_externalObjectGraph and m_externalRememberedSet need to be guarded by a lock
+ since both the GC marker thread and the mutator thread may access them at the
+ same time.
+
+ * API/JSVirtualMachine.mm:
+ (-[JSVirtualMachine addExternalRememberedObject:]):
+ (-[JSVirtualMachine addManagedReference:withOwner:]):
+ (-[JSVirtualMachine removeManagedReference:withOwner:]):
+ (-[JSVirtualMachine externalDataMutex]):
+ (scanExternalObjectGraph):
+ (scanExternalRememberedSet):
+
+ * API/JSVirtualMachineInternal.h:
+ - Deleted externalObjectGraph method. There's no need to expose this.
+
2017-01-06 Michael Saboff <[email protected]>
@putByValDirect in Array.of and Array.from overwrites non-writable/configurable properties
