Mocha icon indicating copy to clipboard operation
Mocha copied to clipboard

[MOObjectDescription addInstanceMethodWithSelector:function:] causes crashes when argument list isn't exact

Open matt-curtis opened this issue 9 years ago • 7 comments

Calling this method behaves oddly in regards to arguments:

1.) When the JavaScript function is called, the arguments variable is only ever as populated as the predefined arguments. E.g, when assigning the function function(){ arguments } to a method selector such as "my:method:selector:" arguments.length will always equal 0, regardless of the actual parameters sent along with the message.

2.) The function specified can only have the exact same (or less) number of arguments as the selector has parameters. Any greater number of arguments causes the runtime to crash. For the selector "my:method:selector:" this function will work: function(a, b, c){} but function(a, b, c, d){ arguments } causes a crash. This breaks and defies the fluid nature of JavaScript Function arguments.

It also appears that calling this method (and it's companion addClassMethod version) can cause the JSValue and its JSContext to be retained.

matt-curtis avatar Apr 12 '15 17:04 matt-curtis

Do you have any stand-alone example code which illustrates the problem? It would be easier to debug if there was a nice example.

afedor avatar Apr 16 '15 14:04 afedor

Sure - here's one I just constructed. Tested in Sketch.

COScript.currentCOScript().setShouldKeepAround_(true);

//  Create class

var uniqueClassName = "Mocha_DynamicClass_"+new Date().getTime();

var delegateClassDesc = MOClassDescription.allocateDescriptionForClassWithName_superclass_(uniqueClassName, NSObject);

var selector = "webView:didFinishLoadForFrame:";

var onLoaded = function(){
    var app = [NSApplication sharedApplication];
    [app displayDialog:"WebView Loaded!" withTitle:"Alert"];

    win.close();
    COScript.currentCOScript().setShouldKeepAround_(false);
};

//  !! -- BEGIN Crashing Code -- !!

//  This example works fine

delegateClassDesc.addInstanceMethodWithSelector_function_(selector, function(webView, webFrame){
    onLoaded();
});

//  This example works fine too, but arguments only equals [webView], instead of [webView, webFrame]

delegateClassDesc.addInstanceMethodWithSelector_function_(selector, function(webView){
    onLoaded();
});

//  This example works fine too, but arguments only equals [], instead of [webView, webFrame]

delegateClassDesc.addInstanceMethodWithSelector_function_(selector, function(){
    onLoaded();
});

//  This example crashes - anything more than the exact number of arguments crashes

delegateClassDesc.addInstanceMethodWithSelector_function_(selector, function(webView, webFrame, extraArg){
    onLoaded();
});

//  !! -- END Crashing Code -- !!

delegateClassDesc.registerClass();

//  Create Window

var frame = NSMakeRect(0, 0, 500, 400);
var styleMask = NSTitledWindowMask | NSClosableWindowMask | NSResizableWindowMask;
var win  = [[NSWindow alloc] initWithContentRect:frame styleMask:styleMask backing:NSBackingStoreBuffered defer:false];

//  Create WebView

var webView = WebView.new();

webView.setFrameLoadDelegate_(NSClassFromString(uniqueClassName).new());
webView.setMainFrameURL_("http://google.com");

webView.setFrame_(frame);
win.contentView().addSubview_(webView);

//  Show Window

win.makeKeyAndOrderFront_(NSApp);
win.center();

matt-curtis avatar Apr 16 '15 16:04 matt-curtis

Well, unfortunately, this appears to be a tough nut to crack. When the delegate method implementation is called, there doesn't appear to be any information we can use to find the correct number of arguments. The block implementation provides the object, but NOT the selector (surprise!), and the venerable C varargs list does not give any information about how many arguments are provided.

Possibly the only work around would be to keep some global list of function to selector mappings. That's pretty ugly though.

afedor avatar Apr 17 '15 03:04 afedor

Maybe I'm not understanding, but wouldn't this solve that?

matt-curtis avatar Apr 17 '15 04:04 matt-curtis

Nope. That's why I put in the little "surprise!" text. You might look at http://landonf.bikemonkey.org/code/objc/imp_implementationWithBlock.20110413.html to understand why that is. i.e. _cmd does not exist in the block implementation that Mocha uses to generate the class methods

afedor avatar Apr 17 '15 14:04 afedor

Ah, OK. I reviewed the source more thoroughly and that article and I see now...what a bother, haha. va_list makes this more hopeless... (I also just realized my 'fix' in my library won't handle va_list...awesome.)

OK, here's another route: what about using forwardInvocation: and [NSInvocation getArgument:atIndex:] somehow? I realize this would mean using a shim class to catch the invocation then forwarding it to the actual class or something...just spitballing here. This is an interesting problem

matt-curtis avatar Apr 17 '15 18:04 matt-curtis

Well your still left with having to store the mapping between the function and the selector somewhere - although in this case you could probably put it in the shim class itself

afedor avatar Apr 17 '15 20:04 afedor