Closed
Bug 562901
Opened 15 years ago
Closed 15 years ago
Synchronous XMLHttpRequests cause setInterval to stop functioning
Categories
(Core :: DOM: Core & HTML, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla2.0
People
(Reporter: michaelblizzard, Assigned: smaug)
References
Details
(Keywords: regression)
Attachments
(3 files, 1 obsolete file)
3.95 KB,
application/x-zip-compressed
|
Details | |
1.01 KB,
patch
|
Details | Diff | Splinter Review | |
2.07 KB,
patch
|
jst
:
review+
christian
:
approval1.9.2.9-
christian
:
approval1.9.1.12-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
setInterval will stop functioning in Firefox 3.5+ when making multiple synchronous XMLHttpRequests. This issue is intermittent but I am able to reproduce it.
NOTE: I am NOT able to reproduce this issue when Firebug is installed.
Reproducible: Sometimes
Steps to Reproduce:
1.Create a page with two frames each containing an input box.
2.On the load event of each page, use setInterval to populate the input boxes with a counter running every 50 milliseconds.
3.For each setInterval, make a simple synchronous XMLHttpRequest:
var xhReq = new XMLHttpRequest();
xhReq.open("POST", "form1.aspx",false);
xhReq.send(null);
var serverResponse = xhReq.responseText;
Most of the time the counters will run as expected but if you refresh the page a few times you'll eventually see that one (if not both) of the counters will not work.
NOTE: When the issue is reproduced, setInterval never initially fires (it doesn't stop working half way through the process).
Actual Results:
Intermittently, you'll see that the input boxes don't get populated, the counter never actually starts.
Expected Results:
For every iteration of of setInterval, the counter in the input boxes should increment.
Reporter | ||
Updated•15 years ago
|
Version: unspecified → 3.5 Branch
Reporter | ||
Updated•15 years ago
|
Version: 3.5 Branch → 3.6 Branch
Reporter | ||
Updated•15 years ago
|
Priority: -- → P1
Reporter | ||
Updated•15 years ago
|
OS: Windows 7 → All
Reporter | ||
Updated•15 years ago
|
Hardware: x86 → All
Reporter | ||
Comment 1•15 years ago
|
||
This is not an issue before version 3.5.
Version: 3.6 Branch → 3.5 Branch
Component: General → DOM
Product: Firefox → Core
QA Contact: general → general
Version: 3.5 Branch → 1.9.1 Branch
Sounds like this is a request that bug 340345 be undone.
Blocks: 340345
Reporter | ||
Comment 3•15 years ago
|
||
Just to simplify my repro steps, you don’t need multiple setIntervals running. You make a synchronous XMLHttpRequest within a set interval and refresh the page a few times; you’ll see that the setInterval will not fire. I believe it’s more apparent with multiple setIntervals because they interfere with each other; plus, this is more of a realistic real world scenario. In our application we have multiple setIntervals running and allot of the time when the page loads one of those setIntervals never initially executes.
Yeah, I've definitely seen this in the past too. It affects my apps as well. Please fix this.
James
Reporter | ||
Comment 6•15 years ago
|
||
Thanks Olli, let me know if you need any additional details, I really appreciate the help.
Assignee | ||
Comment 7•15 years ago
|
||
Well, could you provide a *minimal* testcase attached using "Add an attachment".
And if I don't remember to debug this before next Tuesday, please add a comment
here :)
Reporter | ||
Comment 8•15 years ago
|
||
Run the attached example from a virtual directory. The ability to reproduce the issue seems to vary from machine to machine. It only seems possible to reproduce in Firefox 3.5 plus. I’ve been able to reproduce this with and without firebug enabled but if you can’t get it to happen one way then try the other. When you open default.htm you should see 4 counters running. Hit refresh a couple of times and on occasion you’ll see that some of the counters stop running.
Assignee | ||
Comment 9•15 years ago
|
||
I have no idea what is "virtual directory".
Does the example work if I unzip it to a local directory.
The example seem to use some Windows thing "Web.config".
is that needed? (I use mainly just Linux and OSX)
Assignee | ||
Comment 10•15 years ago
|
||
But seems like the testcase is simple enough to work on my webserver.
I'll try later this week.
Reporter | ||
Comment 11•15 years ago
|
||
Sorry Olli, webserver will do. Thanks.
Reporter | ||
Comment 12•15 years ago
|
||
Also, the web.config is not needed, I was using IIS and it got packaged with my zip file. Thanks again.
Reporter | ||
Comment 14•15 years ago
|
||
Bug 504569 (mentioned above) seems to be the same issue. I've tested this with setTimeout and setInterval with simular results.
Assignee | ||
Comment 15•15 years ago
|
||
Looking at this today.
Assignee | ||
Comment 16•15 years ago
|
||
Ok, I can reproduce this on 3.6, but not on trunk.
Michael, do you think you could try to search when this was fixed on trunk.
You can find nightly builds here
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/
mozilla-central is "trunk".
3.6 branched sometime last august I think, so after that mozilla-central is different than 3.6 (i.e. 1.9.2).
Binary-search like testing works fine when trying to find (un)regression ranges
in builds.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 17•15 years ago
|
||
Ben, do you remember any relevant fixes on trunk?
Hm, not really...
Reporter | ||
Comment 19•15 years ago
|
||
Olli,
I'm glad you've been able to reproduce our issue, that's great news. I'll do some searching first thing tomorrow.
Reporter | ||
Comment 20•15 years ago
|
||
Hi Olli, I'm new Mozilla bug tracking, what's the best way to go about tracking down when this was fixed? Is there documentation I can look over that lists bug fixes in various versions?
Reporter | ||
Comment 21•15 years ago
|
||
I was able to reproduce the error with the following build: firefox-3.7a5pre.en-US.win32.installer.exe ... could you point me in the direction of the "truck" build you tried?
Assignee | ||
Comment 22•15 years ago
|
||
I was using my own builds.
I guess I need to try harder with trunk builds, if you managed to reproduce on
trunk.
Reporter | ||
Comment 23•15 years ago
|
||
It’s so intermittent that is seems the slightest change can throw it off. For example, for the longest time I couldn’t reproduce it when I was using Firebug but then suddenly it would happen every time. If you are still running my sample project with the four timers try changing their intervals. I had them all running at 50 milliseconds. Try making them run at different intervals (e.g. 50, 75, 100, and 150).
FYI, the issue doesn't happen as often with "Minefield 3.7a5pre (rv: 1.9.3a5pre)" as it does with "Firefox 3.6.3"
Reporter | ||
Comment 24•15 years ago
|
||
Hi Ollie, any luck reproducing this issue on trunk?
Comment 25•15 years ago
|
||
I am also experiencing this issue and I was wondering how the progress is going towards fixing it?
Comment 26•15 years ago
|
||
I am also experiencing this issue and I was wondering how the progress is going towards fixing it?
Assignee | ||
Comment 27•15 years ago
|
||
Well, at least I need to find some way to reproduce it on trunk.
Though, I guess I could try to fix it first on branch and port the fix to trunk.
(I've been just a bit busy lately.)
Reporter | ||
Comment 28•15 years ago
|
||
Hi Olli, I made a few suggestions in comment 23 that might help you reproduce this on trunk; it's sort of hit and miss. I’m not 100% familiar with how Mozilla handles its defects; since this is open source is it up to the community to resolve these issues or do you work actually work for Mozilla? The reason I ask is because I want to take the quickest path possible to resolve this issue as it is seriously effecting one of our commercial products. If there is anything I can do to help please let me know, we appreciate your efforts so far.
Assignee | ||
Comment 29•15 years ago
|
||
I do work for Mozilla. I have plenty of other things in my todo list, so the
problem is to find the time for this. But I'll try, this week.
On Thursday, I hope.
Assignee | ||
Comment 30•15 years ago
|
||
Actually, I think I know where the problem is. Patch and tryserver builds coming
today or tomorrow, I hope.
Assignee | ||
Comment 31•15 years ago
|
||
This seems to help on 1.9.2 branch.
Look for 7d545e6f5cca in https://build.mozilla.org/tryserver-builds/?C=M;O=D
in a few hours. That should be a trunk build with the patch.
I need to write a mochitest for this.
Assignee | ||
Comment 32•15 years ago
|
||
Bah, the patch is probably wrong after all.
Assignee | ||
Comment 33•15 years ago
|
||
I've tried to find an automated way to test this, but not luck yet.
Attachment #447424 -
Attachment is obsolete: true
Attachment #447513 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 34•15 years ago
|
||
Comment on attachment 447513 [details] [diff] [review]
patch
Another possibility would be perhaps to change the code
to iterate over subdocuments, not docshells.
End result should be the same.
Attachment #447513 -
Flags: review?(bent.mozilla) → review?(jst)
Reporter | ||
Comment 35•15 years ago
|
||
If you could post a Win32 installation I can test this thoroughly on my side Olli.
Assignee | ||
Comment 36•15 years ago
|
||
Reporter | ||
Comment 37•15 years ago
|
||
This seems to have resolved the issue in our sandbox environment, and by looking at your code I think I can see why, but unfortunately the issue is still present in our production application. Our application uses multiple setInterval/setTimeout calls across multiple frame sets. Is it possible the root of the problem lies deeper in the architecture of how Firefox handles setInterval/setTimeout?
Assignee | ||
Comment 38•15 years ago
|
||
(In reply to comment #37)
> Is it possible the
> root of the problem lies deeper in the architecture of how Firefox handles
> setInterval/setTimeout?
I'm pretty sure the bug is in Suspend/Resume, not in timeout/interval code.
But without a testcase it is hard fix the problem you're still seeing.
Could you perhaps try to find a minimal testcase for that?
Assignee | ||
Comment 39•15 years ago
|
||
(Btw, have to say, synchronous XHR is generally bad for user experience, since it locks up UI. But nevertheless, this bug must be fixed.)
Reporter | ||
Comment 40•15 years ago
|
||
Is it possible to use my original test case to track down the root of the problem? That is, unless that patch you've already implemented is still required to solve the over all problem.
Assignee | ||
Comment 41•15 years ago
|
||
Well, the patch does fix the problem your testcase shows.
I assume there is some other variant of the problem you're seeing in your
web app.
Assignee | ||
Comment 42•15 years ago
|
||
Would it be possible to get access to your web app?
Reporter | ||
Comment 43•15 years ago
|
||
Definitly, I'll email you the login information and repro steps shortly, I just need to create you an account. Thanks.
Comment 44•15 years ago
|
||
This is awesome news that the issue is being addressed.
I was told of this issue with some who were using Windows XP. Even when I was on XP I didn't see the issue but I always had Firebug installed and near as I can tell, the cases when the issue was seen, they didn't have the add-on.
Will this fix be part of the 3.6.4 release or would it wait until a later release (4.0 perhaps)?
Awesome stuff! Very excited to see this one ironed out.
Assignee | ||
Updated•15 years ago
|
Attachment #447513 -
Flags: review?(jst)
Assignee | ||
Comment 45•15 years ago
|
||
I pushed this to tryserver.
This seems to fix the testcase and the web app I have been testing.
The patch is very close to the previous one. The main change is the inner window check, which prevents
rv = win->ResumeTimeouts(aThawChildren);
NS_ENSURE_SUCCESS(rv, rv);
to run, and thus allows looping over all the child windows.
Jst, this is still simpler approach than what I mentioned to you yesterday.
So should be safer for branches.
But let's see what tryserver says about this and would be great if you Michael could
test the tryserver builds once they are ready.
Reporter | ||
Comment 46•15 years ago
|
||
That's great news! I'll test it thoroughly once the builds are up. Thanks Olli.
Assignee | ||
Comment 47•15 years ago
|
||
The builds will be here https://build.mozilla.org/tryserver-builds/opettay@mozilla.com-try-ce133559fc99/
Reporter | ||
Comment 48•15 years ago
|
||
Great, I'll keep a look out for them.
Assignee | ||
Comment 49•15 years ago
|
||
Tryserver doesn't complain any new failures because of the patch.
Not sure why only maemo build is upload to the build.mozilla.org atm.
Assignee | ||
Comment 50•15 years ago
|
||
Aha, other builds are uploaded nowadays to http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/opettay@mozilla.com-ce133559fc99/
Assignee | ||
Comment 51•15 years ago
|
||
Comment on attachment 449016 [details] [diff] [review]
patch
Asking review for this (assuming Michael can confirm that the web app works).
I need to still find some way to test this.
Attachment #449016 -
Flags: review?(jst)
Reporter | ||
Comment 52•15 years ago
|
||
Olli, your patch works perfectly, I wasn't able to repro the issue in any of my test enviroments. Thank you so much. What is the process for getting this patch included in a Firefox release, do you need anything else from me?
Assignee | ||
Comment 53•15 years ago
|
||
The process is that the patch needs to get reviewed, and there should be automated
tests for it. Then for branches (FF 3.5.x and FF 3.6.x) the patch needs
approval.
I agree this is something which should be fixed on branches.
If you want to help, would be awesome if you could make tests which don't
need user interaction. I could change such test to mochitest.
Assignee | ||
Comment 54•15 years ago
|
||
Jst, could you review the patch? I'd like to land to patch to trunk even
without tests so that we could see whether this causes regressions.
I somehow don't seem to find the way to test this all
in mochitest, but still trying.
Comment 55•15 years ago
|
||
Comment on attachment 449016 [details] [diff] [review]
patch
r=jst, but I'd like to know how this makes us work compared to other browsers with respect to the interaction between timeouts and sync XHRs.
Attachment #449016 -
Flags: review?(jst) → review+
Reporter | ||
Comment 56•15 years ago
|
||
We are anxious to know if we will be seeing this patch in an upcoming version of Firefox. Is it possible to get a status update? Thanks, Mike
Assignee | ||
Comment 57•15 years ago
|
||
Uh, I forgot to post the link to the hg changeset.
http://hg.mozilla.org/mozilla-central/rev/1181eb2743a9
So this should be fixed on trunk, but before asking approval for
branch, I need to figure out some test which doesn't involve
user interaction.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 58•15 years ago
|
||
Ok, so "trunk" is the Minefield build and "branch" would be Firefox?
Assignee | ||
Comment 59•15 years ago
|
||
Trunk is Minefield, or the becoming Firefox 4.
Branches are Firefox 3.6.x and 3.5.x
Reporter | ||
Comment 60•15 years ago
|
||
Ok great, thanks for the info. I also found this link for anyone following that wants to learn more:
http://forums.mozillazine.org/viewtopic.php?t=305281
I guess branch is what we are pushing for then, the sooner we can get this to our clients the better. We definitly appreciate the efforts so far.
Comment 61•15 years ago
|
||
What release can we expect the patch to be in?
Assignee | ||
Updated•15 years ago
|
Attachment #449016 -
Flags: approval1.9.2.8?
Attachment #449016 -
Flags: approval1.9.1.12?
Assignee | ||
Comment 62•15 years ago
|
||
Comment on attachment 449016 [details] [diff] [review]
patch
Asking approval, since this is pretty nasty bug. But I haven't found yet a way to test this automatically - so no mochitests yet.
Assignee | ||
Comment 63•15 years ago
|
||
Martijn, could you perhaps help with the mochitest?
Reporter | ||
Comment 64•15 years ago
|
||
Any luck getting that patch pushed through without the mochitest?
Reporter | ||
Comment 65•15 years ago
|
||
To anyone waiting for this fix, it appears as if it is no longer an issue in Firefox Beta 4. Could anyone confirm?
Comment 66•15 years ago
|
||
Comment on attachment 449016 [details] [diff] [review]
patch
Denying for branches. This is an old bug, behavior we specifically took, and it is not worth the risk to fix it on branches.
Attachment #449016 -
Flags: approval1.9.2.9?
Attachment #449016 -
Flags: approval1.9.2.9-
Attachment #449016 -
Flags: approval1.9.1.12?
Attachment #449016 -
Flags: approval1.9.1.12-
Assignee | ||
Comment 67•15 years ago
|
||
(In reply to comment #66)
>This is an old bug,
Yes
> behavior we specifically took,
No. This is pretty serious regression from Sync XHR handling changes.
> and it
> is not worth the risk to fix it on branches.
This might be true.
Assignee | ||
Updated•15 years ago
|
Keywords: regression
Comment 68•13 years ago
|
||
I am struggling with this same problem on FF 3.6 and am not in a position to move up.
Is there any chance this will be reconsidered for inclusion in 3.6?
(In reply to sgrube from comment #68)
> I am struggling with this same problem on FF 3.6 and am not in a position to
> move up.
> Is there any chance this will be reconsidered for inclusion in 3.6?
No, sorry.
Target Milestone: --- → mozilla2.0
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•