Leaks creating many JSKitInterpreter

Discuss the JavaScriptCore Objective-C wrapper/bridge
Post Reply
ddribin
Posts: 19
Joined: Mon May 12, 2008 9:21 am

Leaks creating many JSKitInterpreter

Post by ddribin » Mon May 12, 2008 9:35 am

I was benchmarking some of my test code, and I noticed that even though I retain/release JSKitInterpreter properly, all instances were being leaked. I tracked it down to the calls to addFunction:target:selector: in -init for the print, log, introspect, and include global functions. The problem is that the target for the functions get added to the global gGlobalFuncMap. This retains all instances of any created interpreter, and thus leaks them since they are never removed from the global function map.

The way I got around this was to comment out the addFunction: calls in -init. I'm not sure of the "right" way to fix this. Perhaps, we need to unregister the global functions in -dealloc. But this can be dangerous in garbage collected environments.

Thoughts?

-Dave

gandreas
Immortal
Posts: 1464
Joined: Wed Feb 04, 2004 6:02 pm
Contact:

Post by gandreas » Mon May 12, 2008 12:10 pm

I've yet to play with it with GC on (since, amongst other things, it's unclear how well JavaScriptCore works with GC on), but yeah, that does seem to leak.

One possible work around is to change addFunction:target:selector: to have a non-retained object for the target:

Code: Select all

static JSValueRef globalFunctionCallback(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
{
    NSDictionary *funcInfo = [gGlobalFuncMap objectForKey: [NSValue valueWithPointer:function]];
    NSCAssert1(funcInfo != nil, @"Missing function info for function %p", function);
    NSArray *args = [JSKitInterpreter convertJSValueRefs:arguments count:argumentCount context:ctx];
    SEL sel = NSSelectorFromString([funcInfo objectForKey: @"selector"]);
    id target = [[funcInfo objectForKey: @"target"] nonretainedObjectValue];
    id retval = [target performSelector:sel withObject:args];
    if (retval == nil)
	return JSValueMakeNull(ctx);
    return [retval convertToJSValueRefWithContext:(JSGlobalContextRef) ctx];
}

- (void) addFunction: (NSString *) name target: (id) target selector: (SEL) sel
{
    JSStringRef funcName = JSStringCreateWithCFString((CFStringRef)name);
    JSObjectRef func = JSObjectMakeFunctionWithCallback(myContext, funcName, globalFunctionCallback);
    JSObjectSetProperty(myContext, JSContextGetGlobalObject(myContext), funcName, func, 0, NULL);
    JSKitObject *obj = [JSKitObject objectWithObject:func context:myContext];
    NSDictionary *entry = [NSDictionary dictionaryWithObjectsAndKeys:
			   obj, @"object",
			   name, @"name",
			   [NSValue valueWithNonretainedObject: target], @"target",
			   NSStringFromSelector(sel), @"selector",
			   nil];
    if (!gGlobalFuncMap) {
	gGlobalFuncMap = [[NSMutableDictionary dictionary] retain];
    }
    [gGlobalFuncMap setObject: entry
			  forKey: [NSValue valueWithPointer:func]];
    JSStringRelease(funcName);
}


ddribin
Posts: 19
Joined: Mon May 12, 2008 9:21 am

Post by ddribin » Tue May 13, 2008 1:36 am

gandreas wrote:I've yet to play with it with GC on (since, amongst other things, it's unclear how well JavaScriptCore works with GC on), but yeah, that does seem to leak.
I'm not using GC, yet, but JavaScriptCore, being in CoreFoundation doesn't interact with GC at all. The JSKit Objective-C wrappers just need to be GC-compatible.
One possible work around is to change addFunction:target:selector: to have a non-retained object for the target:
That works... sort of. The problem is that the gGlobalFuncMap still grows forever, and does not release functions as interpreters are released. I made a simple work around by converting myGlobalFunctions into an array. Then I add all functions to this array. And finally, in dealloc, I call:

Code: Select all

    [gGlobalFuncMap removeObjectsForKeys:myGlobalFunctions];
I tried using JSObjectSetPrivate to set the interpreter as private data on the global object, but that didn't work. If it worked, it would mean that a global variable isn't necessary, as we could store the functions in per-interpreter dictionaries. Unfortunately, I couldn't get it working.

Also, is there an email address I can send patches too? This board doesn't allow attaching files, so it makes sharing patches a PITA.

-Dave

ddribin
Posts: 19
Joined: Mon May 12, 2008 9:21 am

Post by ddribin » Tue May 13, 2008 9:35 am

ddribin wrote:I tried using JSObjectSetPrivate to set the interpreter as private data on the global object, but that didn't work. If it worked, it would mean that a global variable isn't necessary, as we could store the functions in per-interpreter dictionaries. Unfortunately, I couldn't get it working.
Okay, I got it working. You have to use a non-NULL class when creating the global context in order to use private data. :-/ Anyhow, it now works without gGlobalFuncMap. I stash a pointer to the interpreter in the private data. This also removes the need for gCtxMap. As a nice side benefit, it's now thread-safe as there's no shared globals between instances of the interpreter.

-Dave

ddribin
Posts: 19
Joined: Mon May 12, 2008 9:21 am

Post by ddribin » Wed May 14, 2008 1:12 am

Here's a patch that fixes the leaks:

http://www.dribin.org/dave/tmp/interpre ... 1.patch.gz

It also removes the need for any global variables.

-Dave

Post Reply

Who is online

Users browsing this forum: No registered users and 2 guests