[FIXED-512] Ext.layout.ContainerLayout: race condition in resizeTask

[Jaroslav Snajdr]

Ext version tested:

Ext 3.0.3
Ext 3.1.0


Browser versions tested against:

FF3 (firebug 1.3.0.10 installed)


Operating System:

Snow Leopard


Description:

When ContainerLayout is being resized, it creates a DelayedTask to call method runLayout 50ms later.

However, when the component with this layout is destroyed before the task is run, the runLayout method is called on already destroyed object. This leads to a JavaScript exception about undefined DOM elements somewhere down the call stack. (Or some other error - quite random things can happen.)

We discovered the bug in our application when solving bug report like "when I click around fast enough in the app navigation, sooner or later I always get a JS error"

Possible fix:

Cancel the resizeTask in the ContainerLayout.destroy method:



destroy: function() {
if (this.resizeTask) {
this.resizeTask.cancel();
}
/* ... rest of the method ... */
}

[Animal]

I would recommend setting Ext.Container.prototype.bufferResize = false

[Jaroslav Snajdr]

That would considerably slow down my application when resizing, wouldn't it?

Or is bufferResize some kind of obsolete feature that doesn't really improve performance and is not worth fixing?

[Condor]

There are more instances in the Ext code that create an Ext.util.DelayedTask, but do not destroy it when the component is destroyed:


Ext.layout.ContainerLayout.onResize - resizeTask
Ext.layout.BorderLayout.Region.initAutoHide - st
Ext.direct.RemotingProvider.queueTransaction - callTask
Ext.chart.Chart.delayRefresh - refreshTask

[Animal]

That would considerably slow down my application when resizing, wouldn't it?

Or is bufferResize some kind of obsolete feature that doesn't really improve performance and is not worth fixing?


I don't think it improves performance.

[bsaks]

I've also noticed a couple of similar cases involving the defer() method instead of Ext.util.DelayedTask:



Ext.grid.GridPanel.afterRender - v.afterRender.defer(10, this.view);
Ext.grid.GridView.onLoad - this.scrollToTop.defer(Ext.isGecko ? 1 : 0, this);


In both cases, storing the return value and passing it to clearTimeout() when the "destroy" event fired seemed to fix the problem.

[Jamie Avins]

We will look at this around a 3.2 timeframe to address all cases. Any test cases that can be provided where we can reproduce the issue consistently would be of great help. We may need an approach which tackles this from both a removal side and a sanity check in case the delayed function is called after for any reason.

[Jamie Avins]

Fixed in SVN 6112. Updated destroy methods for ContainerLayout, BorderLayout, Charts, GridView, and GridPanel. Direct Providers do not have a lifecycle.