828319 - FF18 IonMonkey regression with someFunction.caller being null affecting most ExtJS applications

Status: RESOLVED DUPLICATE of bug 818023

(Core :: JavaScript Engine, defect)

Description

[Steve Storey]

User Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20121231071231

Steps to reproduce:

The issue is visible in one of our internal applications that uses the ExtJS library. I'm trying to work up a simpler test case to demonstrate the problem that can be published more easily. In order to get a sample trace of what happened, I modified ext-all-dev.js right at the top to create:

        // This is the "$previous" method of a hook function on an instance. When called, it
        // calls through the class prototype by the name of the called method.
        callOverrideParent = function () {
            if (callOverrideParent.caller == null) {
                try {
                throw new Error("Caller is null");
                } catch (ex) {
                        console.log(ex.stack);
                        throw ex;
                }
            }
            var method = callOverrideParent.caller.caller; // skip callParent (our caller)
            return method.$owner.prototype[method.$name].apply(this, arguments);
        },

It seems that there are other ExtJS users affected by the problem: http://www.sencha.com/forum/showthread.php?253345-FF-18-problem&p=927553

And there is a vaguley similar bugzilla report: https://bugzilla.mozilla.org/show_bug.cgi?id=818023 but since that's not noted as a regression, I thought that I should point out that this behaviour is affecting live applications.


Actual results:

I received a caller is null error message in the Firefox built-in logging console, with the following trace:

[13:44:21.647] callOverrideParent@http://localhost:8080/myapp/js/ext/ext-all-dev.js:57
.callParent@http://localhost:8080/myapp/js/ext/ext-all-dev.js:6279
.statics.flushLayouts/<.runComplete@http://localhost:8080/myapp/js/ext/ext-all-dev.js:31620
.run@http://localhost:8080/myapp/js/ext/ext-all-dev.js:148629
.statics.flushLayouts@http://localhost:8080/myapp/js/ext/ext-all-dev.js:31624
.statics.resumeLayouts@http://localhost:8080/myapp/js/ext/ext-all-dev.js:31640
Ext.resumeLayouts@http://localhost:8080/myapp/js/ext/ext-all-dev.js:34971
.refreshSize@http://localhost:8080/myapp/js/ext/ext-all-dev.js:133137
.onAdd@http://localhost:8080/myapp/js/ext/ext-all-dev.js:122405
.callParent@http://localhost:8080/myapp/js/ext/ext-all-dev.js:6279
.onAdd@http://localhost:8080/myapp/js/ext/ext-all-dev.js:133807
Ext.util.Event<.fire@http://localhost:8080/myapp/js/ext/ext-all-dev.js:14666
.continueFireEvent@http://localhost:8080/myapp/js/ext/ext-all-dev.js:15035
.fireEvent@http://localhost:8080/myapp/js/ext/ext-all-dev.js:15006
.insert@http://localhost:8080/myapp/js/ext/ext-all-dev.js:93555
.onNodeAppend@http://localhost:8080/myapp/js/ext/ext-all-dev.js:98504
Ext.util.Event<.fire@http://localhost:8080/myapp/js/ext/ext-all-dev.js:14666
.continueFireEvent@http://localhost:8080/myapp/js/ext/ext-all-dev.js:15035
.fireEvent@http://localhost:8080/myapp/js/ext/ext-all-dev.js:15006
.statics.getPrototypeBody/<.appendChild@http://localhost:8080/myapp/js/ext/ext-all-dev.js:97575
.fillNode@http://localhost:8080/myapp/js/ext/ext-all-dev.js:99523
.onProxyLoad@http://localhost:8080/myapp/js/ext/ext-all-dev.js:99553
MyApplication.BranchTreeEntryProxy<.doRequest/<.callback@http://localhost:8080/myapp/js/myapp/BranchPanel.js:24
MyApplication.Service<.request/<.success@http://localhost:8080/myapp/js/myapp/Service.js:15
.callback@http://localhost:8080/myapp/js/ext/ext-all-dev.js:11190
.onComplete@http://localhost:8080/myapp/js/ext/ext-all-dev.js:36236
.onStateChange@http://localhost:8080/myapp/js/ext/ext-all-dev.js:36184
Ext.Function.bind/<@http://localhost:8080/myapp/js/ext/ext-all-dev.js:2970

This particular trace is a XHR success callback, which is going on to add items to a tree panel in ExtJS. If there are > 38 items in the list, then the call at that point as above. Otherwise, it succeeds in adding these tree panel items, but usually another similar call will fail shortly afterwards (presumably at the point IonMonkey decides to optimise something but screws up the optimisation, and 39 items is the tipping point for my specific use-case)






Expected results:

The callOverrideParent.caller property should have been set to the calling function, and not been null. This is indeed the behaviour of FF17, and also if you disable IonMonkey either by enabling the Firebug JS debugger, or by altering your about:config settings

Comment 1

[Matthias Versen [:Matti]]

Please do not move bugs out of the untriaged component. Firefox:General is in general wrong.

Comment 2

[Steve Storey]

I have a reproducible test case that I've got available here: http://adhoc.moo.com/ionmonkey-regression-test.html

If you load that page with the Firefox web console open (and Firebug DISABLED) then you should see the error message. If it doesn't break on load, use the "Click me" button at the top of the left hand panel - triple/quad click it to fire several refreshes quickly and it should fail.

Following the failure, the UI will usually then break in various ways (either collapsible panel doesn't close, or it leaves it's footprint, or it breaks "hilariously" on reopening)

Comment 3

[Jan de Mooij [:jandem]]

Probably bug 818023.

Comment 4

[Steve Storey]

Another ExtJS user has discovered a workaround: http://www.sencha.com/forum/showthread.php?253345-FF-18-problem&p=927706&viewfull=1#post927706

which is to add:

if (Ext.firefoxVersion >= 18) {
    var noArgs = [];
    Ext.override(Ext.Base, {
        callParent : function(args) {


            var method, superMethod = (method = this.callParent.caller) &&
                    (method.$previous || ((method = method.$owner ?
                            method :
                            method.caller) && method.$owner.superclass[method.$name]));


            // Workarround for Firefox 18. I don't know why this works, but it does. Perhaps functions wich have
            // a try-catch block are handled differently
            try {
            } catch (e) {
            }


            return superMethod.apply(this, args || noArgs);
        }
    });
}

if (Ext.firefoxVersion >= 18) {
    var noArgs = [];
    Ext.override(Ext.Base, {
        callParent : function(args) {


            var method, superMethod = (method = this.callParent.caller) &&
                    (method.$previous || ((method = method.$owner ?
                            method :
                            method.caller) && method.$owner.superclass[method.$name]));


            // Workarround for Firefox 18. I don't know why this works, but it does. Perhaps functions wich have
            // a try-catch block are handled differently
            try {
            } catch (e) {
            }


            return superMethod.apply(this, args || noArgs);
        }
    });
}

just after loading the ExtJS library (e.g. insert at the top of my test case script), which also appears to solve the issue for me as well.

Comment 5

[Jan de Mooij [:jandem]]

Yes, that works because IonMonkey does not compile functions containing try..catch.

fun.apply + fun.caller, 95% sure it's bug 818023, but let's keep this open until we have verified this.

Comment 6

[Matthias Versen [:Matti]]

(used the error in the error console ctrl+shift+j to determine the regression range)
Last good nightly: 2012-10-17
First bad nightly: 2012-10-18

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dac5700acf8b&tochan
ge=cb573b9307e5

regression from bug 801915 ?

Comment 7

[Nicolas B. Pierron [:nbp]]

(In reply to Steve Storey from comment #0)
> The issue is visible in one of our internal applications that uses the ExtJS
> library.

(In reply to Jan de Mooij [:jandem] from comment #5)
> fun.apply + fun.caller, 95% sure it's bug 818023, but let's keep this open
> until we have verified this.

Hi Steve, now that Bug 818023 has made it for Firefox 18.0.1, can you test if this bug still exists in beta such as we can guarantee that the fix made for Bug 818023 cover your use case?

You can find the latest beta (19.0b2) on the following ftp:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/19.0b2-candidates/build1/

Comment 8

[Steve Storey]

I've downloaded the beta and can confirm that I can't reproduce the issue on my basic test case. I'll take the workaround out of our actual application and test it there too and report back, but looking great so far! Thanks!

Steve

Comment 9

[Nicolas B. Pierron [:nbp]]

Ok, thanks for testing.

Just a note for QA: the function using fun.apply should be used at least ~10000 times before it can be compiled with IonMonkey.  Bug 818023 also fix a problem with JM which is that self-hosted (such as forEach) function might have appeared as a caller, so these change should be compared against fx17.