Bug Tracker

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#7344 closed bug (duplicate)

New "readyWait" logic and webkit don't get along

Reported by: Pointy Owned by: Pointy
Priority: high Milestone: 1.4.4
Component: event Version: 1.4.3
Keywords: readyWait ready Cc:
Blocked by: Blocking:

Description

The 1.4.3 "jQuery.ready" code differs from the 1.4.2 code. In particular, in 1.4.2 the fact that "jQuery.isReady" is set to "true" before the readyList is acted upon assured that reentering "jQuery.ready" would not cause a problem. In 1.4.3, however, things are different. There's a counter called "readyWait", which provides another way for control flow to get to the point at which the registered "ready" handlers are invoked. For reasons I don't understand, it seems that webkit (Chrome 7.something on Linux, in this case) is triggering a call to "jQuery.ready" while the code is in the middle of a pending call. (The stack trace consistently shows that a call to "appendChild" on a document fragment, from inside the "clean" routine, is the parent of the call to jQuery.ready, which confuses me.) Maybe that happened in 1.4.2 also, but in that code the call would have had no effect. Now, however, that nested call to "ready" manages to make it to the ready list, resulting in the ready handlers being called again, and (worse) the readyList being set to null. When that call to "ready" returns, eventually the initial call will make it back to the loop, and get an exception when it tries to look at the ready list.

I have no idea how to create a test case for this; it's happening to me on a moderately complicated page on a site I can't easily expose. However, a visual inspection of the new 1.4.3 version of the code should make it pretty clear that it's possible to get to the loop if the code is invoked somehow from inside a handler call.

Change History (13)

comment:1 Changed 9 years ago by Pointy

I've just seen in the Chrome debugger that when "ready" is entered the second time (reentered I should say), the "wait" parameter is an Event object. The code seems to expect "wait" to be either boolean "true" or boolean "false", or at least those are the only values it's interested in.

comment:2 Changed 9 years ago by Pointy

I see the same behavior in Safari/Windows as in Chrome.

comment:3 Changed 9 years ago by Pointy

If I change line 419 from:

  if ( !jQuery.readyWait || (wait !== true && !jQuery.isReady) ) {

to

  if ( (!jQuery.readyWait || wait !== true) && !jQuery.isReady ) {

then the code works fine (for me). Whether that's semantically what's wanted, I don't know. All I know is that some weird WebKit behavior is causing the "ready" handler to be invoked while a call is pending, so that "if" statement is no longer correctly terminating the call.

comment:4 Changed 9 years ago by Pointy

It's not clear in my original writeup, but this effect is not noticed all the time. In fact, it's pretty uncommon; when it starts happening, however, it tends to "stick".

comment:5 Changed 9 years ago by john

Component: unfiledevent
Milestone: 1.51.4.4
Owner: set to Pointy
Status: newpending

Can you test this in 1.4.4rc1? Improvements were made to this code after 1.4.3: http://code.jquery.com/jquery-1.4.4rc1.js

comment:6 Changed 9 years ago by Pointy

Status: pendingnew

I'll give it a try, but I don't think that's going to work. The root problem is really the fact that a call to that "ready" function is somehow being triggered while the function is already running, and it's invoked with an Event object as its single parameter. At that point, readyWait has already been decremented to zero (during the execution of the call higher up the stack). Thus, the "wait" parameter is neither "true" nor "false", so that "if" statement that checks to "!jQuery.readyWait" is going to allow control to flow into the loop, and then eventually to set readyList to null.

I wish I knew how it can be the case that the event handler is invoked like that; my understanding of reality is such that I would expect that to be impossible. It occurs to me however that it might be interesting to examine the Event object ...

comment:7 Changed 9 years ago by Pointy

It still happens with 1.4.4rc1. (Really, I don't see any obvious difference in that particular code.)

The Event object being passed in to the reentrant call to "jQuery.ready" is a "load" event with the document as the target. That is apparently being triggered by that call to "appendChild" inside "clean". At least, that's what the stack trace shows. It's pretty weird.

I can make an attempt to come up with test case, but as I said the site is a secured web app and I don't have an easy way of peeling out just this one page. (It may happen on more of my pages of course, since they all use the same tools, but they'd be no easier to make available.)

comment:8 Changed 9 years ago by Pointy

Oh also, more on the weirdness: it does not appear to be any sort of race condition. Whatever it is about the DOM that causes the browser to do what it does, it happens whether I let the script just run normally or if I pepper it with breakpoints. It always happens when it's doing exactly the same thing; it's not just any call to the domManip code, it's a specific thing in this page (that's not unusual with my application, but not all pages seem to suffer the same way).

comment:9 Changed 9 years ago by Pointy

More possibly useless information: I'm pretty sure that whatever WebKit is doing, it did it on my page under 1.4.2 but because that code just relied on "isReady" to avoid reentrance no harm was done. If I fiddle with the 1.4.2 version to detect the situation, I can see it happening. Thus the weird behavior does not seem to be triggered by anything else new going on in 1.4.3.

comment:10 Changed 9 years ago by SlexAxton

Keywords: readyWait ready added
Priority: undecidedhigh
Status: newopen

I can verify this. I'd say this is a high priority to get done before 1.4.4, but any second opinions?

comment:11 Changed 9 years ago by john

Resolution: duplicate
Status: openclosed

Sounds like #7352 is a duplicate of this - also has a test case. Will be closing this one and moving the discussion over there.

comment:12 Changed 9 years ago by john

Duplicate of #7352.

comment:13 Changed 9 years ago by anonymous

I can confirm that my page is appending a frame in a "ready" handler!

Note: See TracTickets for help on using tickets.