Changeset 279179 in webkit for trunk/Source/JavaScriptCore/API/JSVirtualMachine.mm

Timestamp:

Jun 23, 2021, 12:20:02 PM (4 years ago)

Author:

[email protected]

Message:

add/removeManagedReference:withOwner: should have autoreleasepools
​https://bugs.webkit.org/show_bug.cgi?id=227308

Reviewed by Darin Adler.

Since these APIs create autoreleased objects as an implementation detail
but don't return any to the caller there's no indication such autoreleased
objects could be accumulating. Additionally, it's entirely reasonable to
call these methods in a loop an a large set of objects, which further
exacerbates the problem.

• API/JSVirtualMachine.mm:

(-[JSVirtualMachine addManagedReference:withOwner:]):
(-[JSVirtualMachine removeManagedReference:withOwner:]):

File:

• trunk/Source/JavaScriptCore/API/JSVirtualMachine.mm (modified) (1 diff)

trunk/Source/JavaScriptCore/API/JSVirtualMachine.mm

@@ -163 +163 @@
 
 - (void)addManagedReference:(id)object withOwner:(id)owner
-{    
-    if ([object isKindOfClass:[JSManagedValue class]])
-        [object didAddOwner:owner];
-        
-    object = getInternalObjcObject(object);
-    owner = getInternalObjcObject(owner);
-    
-    if (!object || !owner)
-        return;
-    
-    JSC::JSLockHolder locker(toJS(m_group));
-    if ([self isOldExternalObject:owner] && ![self isOldExternalObject:object])
-        [self addExternalRememberedObject:owner];
- 
-    Locker externalDataMutexLocker { m_externalDataMutex };
-    RetainPtr<NSMapTable> ownedObjects = [m_externalObjectGraph objectForKey:owner];
-    if (!ownedObjects) {
-        NSPointerFunctionsOptions weakIDOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
-        NSPointerFunctionsOptions integerOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsIntegerPersonality;
-        ownedObjects = adoptNS([[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:integerOptions capacity:1]);
-
-        [m_externalObjectGraph setObject:ownedObjects.get() forKey:owner];
-    }
-
-    size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects.get(), (__bridge void*)object));
-    NSMapInsert(ownedObjects.get(), (__bridge void*)object, reinterpret_cast<void*>(count + 1));
+{
+    @autoreleasepool {
+        if ([object isKindOfClass:[JSManagedValue class]])
+            [object didAddOwner:owner];
+
+        object = getInternalObjcObject(object);
+        owner = getInternalObjcObject(owner);
+
+        if (!object || !owner)
+            return;
+
+        JSC::JSLockHolder locker(toJS(m_group));
+        if ([self isOldExternalObject:owner] && ![self isOldExternalObject:object])
+            [self addExternalRememberedObject:owner];
+
+        Locker externalDataMutexLocker { m_externalDataMutex };
+        RetainPtr<NSMapTable> ownedObjects = [m_externalObjectGraph objectForKey:owner];
+        if (!ownedObjects) {
+            NSPointerFunctionsOptions weakIDOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
+            NSPointerFunctionsOptions integerOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsIntegerPersonality;
+            ownedObjects = adoptNS([[NSMapTable alloc] initWithKeyOptions:weakIDOptions valueOptions:integerOptions capacity:1]);
+
+            [m_externalObjectGraph setObject:ownedObjects.get() forKey:owner];
+        }
+
+        size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects.get(), (__bridge void*)object));
+        NSMapInsert(ownedObjects.get(), (__bridge void*)object, reinterpret_cast<void*>(count + 1));
+    }
 }
 
 - (void)removeManagedReference:(id)object withOwner:(id)owner
 {
-    if ([object isKindOfClass:[JSManagedValue class]])
-        [object didRemoveOwner:owner];
-
-    object = getInternalObjcObject(object);
-    owner = getInternalObjcObject(owner);
-    
-    if (!object || !owner)
-        return;
-    
-    JSC::JSLockHolder locker(toJS(m_group));
-    
-    Locker externalDataMutexLocker { m_externalDataMutex };
-    NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
-    if (!ownedObjects)
-        return;
-   
-    size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects, (__bridge void*)object));
-    if (count > 1) {
-        NSMapInsert(ownedObjects, (__bridge void*)object, reinterpret_cast<void*>(count - 1));
-        return;
-    }
-    
-    if (count == 1)
-        NSMapRemove(ownedObjects, (__bridge void*)object);
-
-    if (![ownedObjects count]) {
-        [m_externalObjectGraph removeObjectForKey:owner];
-        [m_externalRememberedSet removeObjectForKey:owner];
+    @autoreleasepool {
+        if ([object isKindOfClass:[JSManagedValue class]])
+            [object didRemoveOwner:owner];
+
+        object = getInternalObjcObject(object);
+        owner = getInternalObjcObject(owner);
+
+        if (!object || !owner)
+            return;
+
+        JSC::JSLockHolder locker(toJS(m_group));
+
+        Locker externalDataMutexLocker { m_externalDataMutex };
+        NSMapTable *ownedObjects = [m_externalObjectGraph objectForKey:owner];
+        if (!ownedObjects)
+            return;
+
+        size_t count = reinterpret_cast<size_t>(NSMapGet(ownedObjects, (__bridge void*)object));
+        if (count > 1) {
+            NSMapInsert(ownedObjects, (__bridge void*)object, reinterpret_cast<void*>(count - 1));
+            return;
+        }
+
+        if (count == 1)
+            NSMapRemove(ownedObjects, (__bridge void*)object);
+
+        if (![ownedObjects count]) {
+            [m_externalObjectGraph removeObjectForKey:owner];
+            [m_externalRememberedSet removeObjectForKey:owner];
+        }
     }
 }