Title: [151786] trunk/Source/_javascript_Core
- Revision
- 151786
- Author
- [email protected]
- Date
- 2013-06-20 11:36:27 -0700 (Thu, 20 Jun 2013)
Log Message
Improper deallocation of JSManagedValue causes crashes during autorelease pool draining
https://bugs.webkit.org/show_bug.cgi?id=117840
Reviewed by Geoffrey Garen.
Improperly managing a JSManagedValue can cause a crash when the JSC::Weak inside the
JSManagedValue is destroyed upon deallocation. We would rather have improperly maintained
JSManagedValues cause memory leaks than take down the whole app.
The fix is to use the callback to the JSC::Weak on the destruction of the VM so that we
can safely null it out. This will prevent ~Weak from crashing.
* API/JSManagedValue.mm:
(-[JSManagedValue JSC::JSC::]):
(JSManagedValueHandleOwner::finalize):
* API/tests/testapi.mm: Added a test that crashed prior to this fix due to a leaked
managed reference. Also fixed a small style nit I noticed in another test.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/JSManagedValue.mm (151785 => 151786)
--- trunk/Source/_javascript_Core/API/JSManagedValue.mm 2013-06-20 18:24:08 UTC (rev 151785)
+++ trunk/Source/_javascript_Core/API/JSManagedValue.mm 2013-06-20 18:36:27 UTC (rev 151786)
@@ -40,6 +40,7 @@
class JSManagedValueHandleOwner : public JSC::WeakHandleOwner {
public:
+ virtual void finalize(JSC::Handle<JSC::Unknown>, void* context);
virtual bool isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void* context, JSC::SlotVisitor&);
};
@@ -90,12 +91,27 @@
return [JSValue valueWithJSValueRef:toRef(object) inContext:context];
}
+- (void)disconnectValue
+{
+ m_value.clear();
+}
+
@end
+@interface JSManagedValue (PrivateMethods)
+- (void)disconnectValue;
+@end
+
bool JSManagedValueHandleOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void* context, JSC::SlotVisitor& visitor)
{
JSManagedValue *managedValue = static_cast<JSManagedValue *>(context);
return visitor.containsOpaqueRoot(managedValue);
}
+void JSManagedValueHandleOwner::finalize(JSC::Handle<JSC::Unknown>, void* context)
+{
+ JSManagedValue *managedValue = static_cast<JSManagedValue *>(context);
+ [managedValue disconnectValue];
+}
+
#endif // JSC_OBJC_API_ENABLED
Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (151785 => 151786)
--- trunk/Source/_javascript_Core/API/tests/testapi.mm 2013-06-20 18:24:08 UTC (rev 151785)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm 2013-06-20 18:36:27 UTC (rev 151786)
@@ -814,11 +814,22 @@
@autoreleasepool {
JSContext *context = [[JSContext alloc] init];
- TestObject *testObject = TestObject.testObject;
+ TestObject *testObject = [TestObject testObject];
context[@"testObject"] = testObject;
[context evaluateScript:@"testObject.__lookupGetter__('variable').call({})"];
checkResult(@"Make sure we throw an exception when calling getter on incorrect |this|", context.exception);
}
+
+ @autoreleasepool {
+ TestObject *testObject = [TestObject testObject];
+ JSManagedValue *managedTestObject;
+ @autoreleasepool {
+ JSContext *context = [[JSContext alloc] init];
+ context[@"testObject"] = testObject;
+ managedTestObject = [JSManagedValue managedValueWithValue:context[@"testObject"]];
+ [context.virtualMachine addManagedReference:managedTestObject withOwner:testObject];
+ }
+ }
}
#else
Modified: trunk/Source/_javascript_Core/ChangeLog (151785 => 151786)
--- trunk/Source/_javascript_Core/ChangeLog 2013-06-20 18:24:08 UTC (rev 151785)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-06-20 18:36:27 UTC (rev 151786)
@@ -1,3 +1,23 @@
+2013-06-20 Mark Hahnenberg <[email protected]>
+
+ Improper deallocation of JSManagedValue causes crashes during autorelease pool draining
+ https://bugs.webkit.org/show_bug.cgi?id=117840
+
+ Reviewed by Geoffrey Garen.
+
+ Improperly managing a JSManagedValue can cause a crash when the JSC::Weak inside the
+ JSManagedValue is destroyed upon deallocation. We would rather have improperly maintained
+ JSManagedValues cause memory leaks than take down the whole app.
+
+ The fix is to use the callback to the JSC::Weak on the destruction of the VM so that we
+ can safely null it out. This will prevent ~Weak from crashing.
+
+ * API/JSManagedValue.mm:
+ (-[JSManagedValue JSC::JSC::]):
+ (JSManagedValueHandleOwner::finalize):
+ * API/tests/testapi.mm: Added a test that crashed prior to this fix due to a leaked
+ managed reference. Also fixed a small style nit I noticed in another test.
+
2013-06-18 Oliver Hunt <[email protected]>
Going to google.com/trends causes a crash
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
