Leaks creating many JSKitInterpreter

[ddribin]

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]

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]

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]

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]

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