PDA

View Full Version : [OPEN-1349] onReady is unreliable due to lack of thread synchronization



Killing
13 Oct 2010, 3:15 PM
Just playing with ExtJS for the first time here to see if we want to use it going forward and have come across a fairly serious flaw in one of the key handlers, that being onReady.

The scenario is as follows:-
<content of ext-all.js>
Ext.onReady( function(){ // Large method here } );

Now what I'm seeing is that between the call to onReady and the function adding the handler to the queue isReady is being set to true. So we have the following flow:-

1. call onReady( func )
2. onReady checks isReady ( false )
3. isReady is set to true
4. onReady adds hander to queue

The result being the function passed to onReady is never called.

Looking at the code in ExtJS it seems that threading was never considered and hence this sort of timing issue could be all over the place :(

There's obviously a dirty hack to help alleviate the issue by rechecking isReady before leaving the onReady setup but this will just reduce the chance for it to happen not solve it totally. With browsers making radical changes to speed up pages this is likely going to become more and more an issue so needs to be solved properly.

For those interested the browser this is showing up in is chrome, in this case: 7.0.517.17

evant
13 Oct 2010, 3:41 PM
Javascript doesn't really have threading, so I'm not really sure what you're getting at here. Perhaps you could post a test case that demonstrates the issue.

Killing
13 Oct 2010, 3:57 PM
It seems thats no longer the case in chrome :(

Here's a trace from the following additional lines added to the handlers:-



onDocumentReady : function(fn, scope, options){
if(Ext.isReady){ // if it already fired or document.body is present
console.log( new Date().getTime() + ":ready" );
docReadyEvent || (docReadyEvent = new Ext.util.Event());
docReadyEvent.addListener(fn, scope, options);
docReadyEvent.fire();
docReadyEvent.listeners = [];
}else{
console.log( new Date().getTime() + ':notready' );
if(!docReadyEvent){
initDocReady();
}
options = options || {};
options.delay = options.delay || 1;
docReadyEvent.addListener(fn, scope, options);
console.log( new Date().getTime() +':wasnt ready:' + Ext.isReady )
}
},

console.log( new Date().getTime() +':before:' + Ext.isReady )
Ext.onReady( function() { // Large function } );


1287011968563:before:false
1287011968567:notready
1287011968567:wasnt ready:true
1287011968567:end:true

As you can see from that trace:
1. before onReady was called isReady was false
2. on entering onReady aka onDocumentReady isReady was false (notready)
3. after running the setup code in the else case of onReady isReady was now set to true

So isReady was altered "while" the thread that was running the onReady code was processing, hence my conclusion there is some threading going on.

jay@moduscreate.com
13 Oct 2010, 4:32 PM
JavaScript has no threading.

Killing
13 Oct 2010, 4:35 PM
I think its going to be hard to create a simple test case as even adding one line before the main onReady call changes the timing enough to make the problem go away, in addition its happening on my main work machine but not my laptop so was very hard to identify in the first place :(
// change chrome async timing
Ext.onReady( function(){});

Killing
13 Oct 2010, 4:38 PM
I think the above proves that's not the case does it not?

How else do you explain:


if ( Ext.isReady ) {
// true case
} else {
// false case
// ...
// Ext.isReady is true *here*
}

Killing
13 Oct 2010, 4:48 PM
To double, tripple check I added some more logging to ext-all.js:-


function fireDocReady(e){
...
if(docReadyEvent && !Ext.isReady){
console.log('SETREADY');
...
}
}
-----
onDocumentReady : function(fn, scope, options){
console.log('START');
...
console.log('DONE');
}


And the result in the console is:
START
SETREADY
DONE

That proves beyond doubt does it not that there are two executing js threads does it not?

fabio.parra
14 Oct 2010, 8:25 AM
Hey...see this video about threads and events in javascript.
http://www.yuiblog.com/blog/2010/08/30/yui-theater-douglas-crockford-crockford-on-javascript-scene-6-loopage-52-min/

jay@moduscreate.com
14 Oct 2010, 8:31 AM
To double, tripple check I added some more logging to ext-all.js:-


function fireDocReady(e){
...
if(docReadyEvent && !Ext.isReady){
console.log('SETREADY');
...
}
}
-----
onDocumentReady : function(fn, scope, options){
console.log('START');
...
console.log('DONE');
}


And the result in the console is:
START
SETREADY
DONE

That proves beyond doubt does it not that there are two executing js threads does it not?

Honestly, I never develop applications with more than one onDocumentReady.

Killing
14 Oct 2010, 10:22 AM
There's only one onDocumentReady, just the browser is firing off the event asynchronously hence the problem, is my current understanding.

jay@moduscreate.com
14 Oct 2010, 10:28 AM
yes -- but it has nothing to do with threading

Killing
14 Oct 2010, 10:36 AM
But if there isn't more than one executing javascript thread how can Ext.isReady change during the execution of the onDocumentReady function?

jay@moduscreate.com
14 Oct 2010, 10:50 AM
because javascript can execute methods asynchronously. setTimeout does just that.

Killing
14 Oct 2010, 11:13 AM
Call me silly though to execute asynchronously in a way which will cause the problem above you either need employ multiple threads or you need to suspend currently executing operation in the middle, run the new code, and then resume the old executing operation.

The result is inherently the same, none deterministic behaviour, exactly what we have here.

If this was a single event loop based system, which we would expect in javascript, the behaviour wouldn't occur; but it is occurring, so how and why is the question?

Are we thinking browser bug?

jay@moduscreate.com
14 Oct 2010, 11:33 AM
Ok, we can call them "threads", but they are not real "threads" as "threads" are defined, thus my responses :).

This issue came up in Ext JS as well. I think JQuery takes care of this in a way that Ext JS does not.

That said, I think it's poor architecture to have multiple onReady calls.

Killing
14 Oct 2010, 12:00 PM
But there doesn't have to be multiple onReady calls even if there was just one single onReady call this could still happen if executing code can be interrupted mid flow.

jay@moduscreate.com
14 Oct 2010, 12:02 PM
I've been developing Ext JS apps since 2006 and support many browsers --- never had this come up as an issue.

Killing
14 Oct 2010, 12:13 PM
Its very weird I'll give you that.

jay@moduscreate.com
14 Oct 2010, 12:15 PM
If you're launching your app from Ext.onReady, how are you getting to this issue?

Killing
14 Oct 2010, 12:33 PM
The onReady function is failing to call the passed in function due to this race.

jay@moduscreate.com
14 Oct 2010, 12:37 PM
I see. Some thoughts (I'm not a chrome user, i use safari/webkit native);

- Is the chrome version the most up to date?
- have you tried the chrome nightly builds?

Killing
14 Oct 2010, 12:50 PM
Using dev stream, one update outstanding not applied yet. Hopefully this is a browser bug, even if a very strange one :)

jnicora
20 Oct 2010, 7:32 AM
Ext.EventManager.onDocumentReady is firing before the body element is in the DOM in the latest Chrome build.



Ext.onReady(function(){
console.log(Ext.select("body"));
});


no elements are returned. This is a big deal for us, Chrome updates automatically and it's the browser our client base uses.

mschwartz
20 Oct 2010, 8:32 AM
There are no threads in any javascript implementation in a browser. It's possible to spawn java threads in Rhino though. The exception is worker threads, which I don't think ExtJS uses.

There are no interrupts in javascript either. Something like a timer "interrupt" is deferred until the current running script is finished.

For the most part, but not always the case, code loaded with <script> tags in the head or body stop the browser from doing ANYTHING until the file is loaded, parsed, and executed. This is because the script may do document.write(), if not for other reasons.

The exception to the synchronous loading and parsing of scripts is if you document.write() <script> tags into an iframe, or some browsers support an async option in the tag. But those are very browser specific.

The only possible conclusion from jnlcora's post above is that Ext.onReady() has a bug. Not sure what version of ExtJS he's using.

jnicora
20 Oct 2010, 9:37 AM
Still not sure exactly what is going on here, but I found this; it would seem that Ext.onReady is just fine and that possibly the latest version of Chrome is the only browser effectively reporting DOMContentLoaded. Anyway, my issue has been resolved and I'll look deeper; maybe find some more answers.

meroy
22 Oct 2010, 9:22 AM
Ext.EventManager.onDocumentReady is firing before the body element is in the DOM in the latest Chrome build.



Ext.onReady(function(){
console.log(Ext.select("body"));
});


no elements are returned. This is a big deal for us, Chrome updates automatically and it's the browser our client base uses.

Thanks for the report. Entering this into the issue tracking system.

meroy
22 Oct 2010, 9:41 AM
@jnicora

Can you try the latest SVN again. Evan reverted onReady change in Core revision r264.

http://www.sencha.com/forum/showthread.php?107208-FIXED-1214-Ext.onReady-code-changed-in-3.3x&p=527991#post527991