Title: [139486] trunk/Source/_javascript_Core
- Revision
- 139486
- Author
- mhahnenb...@apple.com
- Date
- 2013-01-11 12:56:20 -0800 (Fri, 11 Jan 2013)
Log Message
Objective-C objects that are passed to _javascript_ leak (until the JSContext is destroyed)
https://bugs.webkit.org/show_bug.cgi?id=106056
Reviewed by Darin Adler.
* API/APIJSValue.h:
* API/JSValue.mm: Make the reference to the JSContext strong.
(-[JSValue context]):
(-[JSValue initWithValue:inContext:]):
(-[JSValue dealloc]):
* API/JSWrapperMap.mm: Make the reference back from wrappers to Obj-C objects weak instead of strong.
Also add an explicit WeakGCMap in the JSWrapperMap rather than using Obj-C associated object API which
was causing memory leaks.
(wrapperClass):
(-[JSObjCClassInfo wrapperForObject:]):
(-[JSWrapperMap initWithContext:]):
(-[JSWrapperMap dealloc]):
(-[JSWrapperMap wrapperForObject:]):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/APIJSValue.h (139485 => 139486)
--- trunk/Source/_javascript_Core/API/APIJSValue.h 2013-01-11 20:31:09 UTC (rev 139485)
+++ trunk/Source/_javascript_Core/API/APIJSValue.h 2013-01-11 20:56:20 UTC (rev 139486)
@@ -29,7 +29,7 @@
// A JSValue is a reference to a value within the _javascript_ object space of a
// JSVirtualMachine. All instances of JSValue originate from a JSContext, and
-// hold a weak reference to this JSContext. Where an instance method is invoked
+// hold a strong reference to this JSContext. Where an instance method is invoked
// upon a JSValue, and this returns another JSValue, the returned JSValue will
// originate from the same JSContext as the JSValue on which the method was
// invoked.
Modified: trunk/Source/_javascript_Core/API/JSValue.mm (139485 => 139486)
--- trunk/Source/_javascript_Core/API/JSValue.mm 2013-01-11 20:31:09 UTC (rev 139485)
+++ trunk/Source/_javascript_Core/API/JSValue.mm 2013-01-11 20:56:20 UTC (rev 139486)
@@ -53,7 +53,7 @@
@implementation JSValue {
JSValueRef m_value;
- JSContext *m_weakContext;
+ JSContext *m_context;
}
+ (JSValue *)valueWithObject:(id)value inContext:(JSContext *)context
@@ -521,7 +521,7 @@
- (JSContext *)context
{
- return objc_loadWeak(&m_weakContext);
+ return m_context;
}
@end
@@ -1049,7 +1049,7 @@
return nil;
ASSERT(value);
- objc_initWeak(&m_weakContext, context);
+ m_context = [context retain];
[context protect:value];
m_value = value;
return self;
@@ -1172,7 +1172,8 @@
JSContext *context = [self context];
if (context)
[context unprotect:m_value];
- objc_destroyWeak(&m_weakContext);
+ [m_context release];
+ m_context = nil;
[super dealloc];
}
Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.mm (139485 => 139486)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2013-01-11 20:31:09 UTC (rev 139485)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2013-01-11 20:56:20 UTC (rev 139486)
@@ -28,10 +28,12 @@
#if JS_OBJC_API_ENABLED
+#import "APICast.h"
#import "JSContextInternal.h"
#import "JSWrapperMap.h"
#import "ObjCCallbackFunction.h"
#import "ObjcRuntimeExtras.h"
+#import "WeakGCMap.h"
#import <wtf/TCSpinLock.h>
#import "wtf/Vector.h"
@@ -376,6 +378,7 @@
@implementation JSWrapperMap {
JSContext *m_context;
NSMutableDictionary *m_classMap;
+ JSC::WeakGCMap<id, JSC::JSObject> m_cachedWrappers;
}
- (id)initWithContext:(JSContext *)context
@@ -413,10 +416,11 @@
- (JSValue *)wrapperForObject:(id)object
{
- JSValue *wrapper = objc_getAssociatedObject(object, m_context);
- if (wrapper && wrapper.context)
- return wrapper;
+ JSC::JSObject* jsWrapper = m_cachedWrappers.get(object);
+ if (jsWrapper)
+ return [JSValue valueWithValue:toRef(jsWrapper) inContext:m_context];
+ JSValue *wrapper;
if (class_isMetaClass(object_getClass(object)))
wrapper = [[self classInfoForClass:(Class)object] constructor];
else {
@@ -429,7 +433,9 @@
// (1) For immortal objects JSValues will effectively leak and this results in error output being logged - we should avoid adding associated objects to immortal objects.
// (2) A long lived object may rack up many JSValues. When the contexts are released these will unproctect the associated _javascript_ objects,
// but still, would probably nicer if we made it so that only one associated object was required, broadcasting object dealloc.
- objc_setAssociatedObject(object, m_context, wrapper, OBJC_ASSOCIATION_RETAIN);
+ JSC::ExecState* exec = toJS(contextInternalContext(m_context));
+ jsWrapper = toJS(exec, valueInternalValue(wrapper)).toObject(exec);
+ m_cachedWrappers.set(exec->globalData(), object, jsWrapper);
return wrapper;
}
Modified: trunk/Source/_javascript_Core/ChangeLog (139485 => 139486)
--- trunk/Source/_javascript_Core/ChangeLog 2013-01-11 20:31:09 UTC (rev 139485)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-01-11 20:56:20 UTC (rev 139486)
@@ -1,3 +1,24 @@
+2013-01-11 Mark Hahnenberg <mhahnenb...@apple.com>
+
+ Objective-C objects that are passed to _javascript_ leak (until the JSContext is destroyed)
+ https://bugs.webkit.org/show_bug.cgi?id=106056
+
+ Reviewed by Darin Adler.
+
+ * API/APIJSValue.h:
+ * API/JSValue.mm: Make the reference to the JSContext strong.
+ (-[JSValue context]):
+ (-[JSValue initWithValue:inContext:]):
+ (-[JSValue dealloc]):
+ * API/JSWrapperMap.mm: Make the reference back from wrappers to Obj-C objects weak instead of strong.
+ Also add an explicit WeakGCMap in the JSWrapperMap rather than using Obj-C associated object API which
+ was causing memory leaks.
+ (wrapperClass):
+ (-[JSObjCClassInfo wrapperForObject:]):
+ (-[JSWrapperMap initWithContext:]):
+ (-[JSWrapperMap dealloc]):
+ (-[JSWrapperMap wrapperForObject:]):
+
2013-01-11 Geoffrey Garen <gga...@apple.com>
Fixed some bogus PropertyOffset ASSERTs
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
