Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.mm (146391 => 146392)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2013-03-20 21:36:25 UTC (rev 146391)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2013-03-20 21:41:55 UTC (rev 146392)
@@ -135,7 +135,7 @@
// Make an object that is in all ways a completely vanilla _javascript_ object,
// other than that it has a native brand set that will be displayed by the default
// Object.prototype.toString conversion.
-static JSValue *createObjectWithCustomBrand(JSContext *context, NSString *brand, JSClassRef parentClass = 0, Class cls = 0)
+static JSValue *objectWithCustomBrand(JSContext *context, NSString *brand, JSClassRef parentClass = 0, Class cls = 0)
{
JSClassDefinition definition;
definition = kJSClassDefinitionEmpty;
@@ -144,7 +144,7 @@
JSClassRef classRef = JSClassCreate(&definition);
JSObjectRef result = makeWrapper([context globalContextRef], classRef, cls);
JSClassRelease(classRef);
- return [[JSValue alloc] initWithValue:result inContext:context];
+ return [JSValue valueWithValue:result inContext:context];
}
// Look for @optional properties in the prototype containing a selector to property
@@ -379,12 +379,12 @@
if (m_prototype)
prototype = [JSValue valueWithValue:toRef(m_prototype.get()) inContext:m_context];
else
- prototype = createObjectWithCustomBrand(m_context, [NSString stringWithFormat:@"%sPrototype", className]);
+ prototype = objectWithCustomBrand(m_context, [NSString stringWithFormat:@"%sPrototype", className]);
if (m_constructor)
constructor = [JSValue valueWithValue:toRef(m_constructor.get()) inContext:m_context];
else
- constructor = createObjectWithCustomBrand(m_context, [NSString stringWithFormat:@"%sConstructor", className], wrapperClass(), [m_class retain]);
+ constructor = objectWithCustomBrand(m_context, [NSString stringWithFormat:@"%sConstructor", className], wrapperClass(), [m_class retain]);
JSContextRef cContext = [m_context globalContextRef];
m_prototype = toJS(JSValueToObject(cContext, valueInternalValue(prototype), 0));
@@ -401,9 +401,6 @@
// Set [Prototype].
JSObjectSetPrototype([m_context globalContextRef], toRef(m_prototype.get()), toRef(superClassInfo->m_prototype.get()));
-
- [constructor release];
- [prototype release];
}
}
Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (146391 => 146392)
--- trunk/Source/_javascript_Core/API/tests/testapi.mm 2013-03-20 21:36:25 UTC (rev 146391)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm 2013-03-20 21:41:55 UTC (rev 146392)
@@ -562,6 +562,22 @@
@autoreleasepool {
JSContext *context = [[JSContext alloc] init];
+ TestObject *testObject = [TestObject testObject];
+ @autoreleasepool {
+ context[@"testObject"] = testObject;
+ [context evaluateScript:@"var constructor = Object.getPrototypeOf(testObject).constructor; constructor.prototype = undefined;"];
+ [context evaluateScript:@"testObject = undefined"];
+ }
+
+ JSSynchronousGarbageCollectForDebugging([context globalContextRef]);
+
+ @autoreleasepool {
+ context[@"testObject"] = testObject;
+ }
+ }
+
+ @autoreleasepool {
+ JSContext *context = [[JSContext alloc] init];
TextXYZ *testXYZ = [[TextXYZ alloc] init];
@autoreleasepool {
Modified: trunk/Source/_javascript_Core/ChangeLog (146391 => 146392)
--- trunk/Source/_javascript_Core/ChangeLog 2013-03-20 21:36:25 UTC (rev 146391)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-03-20 21:41:55 UTC (rev 146392)
@@ -1,3 +1,23 @@
+2013-03-20 Mark Hahnenberg <[email protected]>
+
+ Objective-C API: Fix over-releasing in allocateConstructorAndPrototypeWithSuperClassInfo:
+ https://bugs.webkit.org/show_bug.cgi?id=112832
+
+ Reviewed by Geoffrey Garen.
+
+ If either the m_constructor or m_prototype (but not both) is collected, we will call
+ allocateConstructorAndPrototypeWithSuperClassInfo, which will create a new object to replace the one
+ that was collected, but at the end of the method we call release on both of them.
+ This is incorrect since we autorelease the JSValue in the case that the object doesn't need to be
+ reallocated. Thus we'll end up overreleasing later during the drain of the autorelease pool.
+
+ * API/JSWrapperMap.mm:
+ (objectWithCustomBrand): We no longer alloc here. We instead call the JSValue valueWithValue class method,
+ which autoreleases for us.
+ (-[JSObjCClassInfo allocateConstructorAndPrototypeWithSuperClassInfo:]): We no longer call release on the
+ constructor or prototype JSValues.
+ * API/tests/testapi.mm: Added a new test that crashes on ToT due to over-releasing.
+
2013-03-19 Filip Pizlo <[email protected]>
It's called "Hash Consing" not "Hash Consting"