Intermittent failure in content/base/test/test_bug482935.html | Received fresh value for second request - got "1", expected "0"

RESOLVED FIXED in Firefox 5

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: bjarne)

Tracking

({intermittent-failure})

Trunk
mozilla6
intermittent-failure
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox5 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1302227102.1302229749.31490.gz
Rev3 MacOSX Leopard 10.5.8 cedar debug test mochitests-1/5 on 2011/04/07 18:45:02

25764 INFO TEST-START | /tests/content/base/test/test_bug482935.html
++DOMWINDOW == 17 (0x1c916148) [serial = 540] [outer = 0x1c155da0]
25765 INFO TEST-PASS | /tests/content/base/test/test_bug482935.html | Expected empty response to cancelled request - "" should equal ""
25766 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug482935.html | Received fresh value for second request - got "1", expected "0"
25767 INFO TEST-PASS | /tests/content/base/test/test_bug482935.html | Expected empty response to cancelled request - "" should equal ""
25768 INFO TEST-PASS | /tests/content/base/test/test_bug482935.html | Received cached value for second request - "1" should equal "1"
25769 INFO TEST-END | /tests/content/base/test/test_bug482935.html | finished in 2092ms
OS: Mac OS X → All
Hardware: x86 → All
Created attachment 524696 [details] [diff] [review]
Changes setTimeout to executeSoon [checked-in]

That might fix it. At least, it's not broken locally.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #524696 - Flags: review?(ehsan)
Whiteboard: [orange] → [orange][has patch][needs review]
Attachment #524696 - Flags: review?(ehsan) → review+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/6f1a2405f71e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [orange][has patch][needs review] → [orange]
Target Milestone: --- → mozilla2.2
Attachment #524696 - Attachment description: Changes setTimeout to executeSoon → Changes setTimeout to executeSoon [checked-in]
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
The push right after that says "nope, not quite fixed."
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hmm, seems like this is now more flaky... :/
Created attachment 525310 [details] [diff] [review]
Second attempt [checked-in]

This patch should be better.
Attachment #524696 - Attachment is obsolete: true
Attachment #525310 - Flags: review?(ehsan)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment on attachment 525310 [details] [diff] [review]
Second attempt [checked-in]

Yeah, this does look better.  But it's best to pass the second parameter to setTimeout explicitly (0).  r=me with that fixed.
Attachment #525310 - Flags: review?(ehsan) → review+
Pushed the second attempt:
http://hg.mozilla.org/mozilla-central/rev/7a0810457ecd
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Attachment #525310 - Attachment description: Second attempt → Second attempt [checked-in]
Comment hidden (Treeherder Robot)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
I have no clue why this doesn't work. Bjarne and Jason, do you have an idea by any chance?
Comment hidden (Treeherder Robot)
Blocks: 482935
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 22

6 years ago
(In reply to comment #16)
> I have no clue why this doesn't work. Bjarne and Jason, do you have an idea by
> any chance?

Please see bug #482935, comment #69 and please also observe the time between landing bug #482935 (March 25th) and the first report of this failure (April 7th).
(In reply to comment #22)
> please also observe the time between
> landing bug #482935 (March 25th) and the first report of this failure (April
> 7th).

Do you have another changeset in mind which might have caused this failure?
Group: mozilla-confidential
Group: mozilla-confidential
Duplicate of this bug: 650132
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 55

6 years ago
(In reply to comment #23)
> Do you have another changeset in mind which might have caused this failure?

Not really - I'm just pointing out the timespan and the fact that there was another unrelated issue (bug #552651 I believe) which caused this *test* to always leak independent of the code-change from bug #482935.

Btw, is it just me or has the frequency of this increased? (8 reports on April 19th vs 2 reports on April 12th)
(In reply to comment #55)
> Btw, is it just me or has the frequency of this increased? (8 reports on April
> 19th vs 2 reports on April 12th)

My fault. While trying to fix it, I actually make it more orange. Though, I don't think we should backout unless my patches went in the wrong direction.
Bjarne, do you think you can find the time to investigate? To know if I should take some to do it.
(In reply to comment #56)
> Bjarne, do you think you can find the time to investigate? To know if I should
> take some to do it.

And you seem to be the best person to look at that ;)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 60

6 years ago
(In reply to comment #57)
> And you seem to be the best person to look at that ;)

Who said that flattery will get you nowhere..?  ;)  Try changing the two xhr2.open-calls to do async requests (i.e. remove last param). Since you've changed them to terminate on listeners anyway there's no reason to do those calls synchronously. I tried it locally and it works (but I'm not able to reproduce the error). I could submit a patch but this change is utterly trivial and you seem to have a shorter review-approval-checkin path than I do...

Why would this improve the situation? Well, I'm actually not 100% sure it will but it's worth a shot and it won't hurt. XHR spins the main-thread event-loop on sync-requests and that has never seemed to me like a very safe thing to do.
(In reply to comment #60)
> (In reply to comment #57)
> > And you seem to be the best person to look at that ;)
> 
> Who said that flattery will get you nowhere..?  ;)  Try changing the two
> xhr2.open-calls to do async requests (i.e. remove last param). Since you've
> changed them to terminate on listeners anyway there's no reason to do those
> calls synchronously. I tried it locally and it works (but I'm not able to
> reproduce the error).

That was no flattery, you obviously know much more than I do about what the test is actually testing.

> I could submit a patch but this change is utterly trivial
> and you seem to have a shorter review-approval-checkin path than I do...

You just have to know the person to ask the review to ;). If you write a patch and ask a review to Ehsan, I can bet he will reply *very* quickly: he is very responsive for random oranges patches.

> Why would this improve the situation? Well, I'm actually not 100% sure it will
> but it's worth a shot and it won't hurt. XHR spins the main-thread event-loop
> on sync-requests and that has never seemed to me like a very safe thing to do.

The only thing I would not like to happen is to hide a real issue by writing a test that is going around it.
(Assignee)

Comment 62

6 years ago
(In reply to comment #61)
> The only thing I would not like to happen is to hide a real issue by writing a
> test that is going around it.

It won't hide anything - it will indicate that spinning the event-loop causes issues.
(Assignee)

Comment 63

6 years ago
Created attachment 527243 [details] [diff] [review]
Using asynchronous xhrs instead of synchronous
Attachment #525310 - Attachment is obsolete: true
Attachment #527243 - Flags: review?(ehsan)
Comment on attachment 527243 [details] [diff] [review]
Using asynchronous xhrs instead of synchronous

Cool, let's try this!
Attachment #527243 - Flags: review?(ehsan) → review+
(Assignee)

Comment 65

6 years ago
Ok - requesting checkin...
Keywords: checkin-needed
Fixed in cedar. Should be synced with mozilla-central in the next 24 hours.
Assignee: mounir.lamouri → bjarne
Keywords: checkin-needed
Whiteboard: [orange] → [orange][has patch][fixed in cedar]
Target Milestone: mozilla5 → mozilla6
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Pushed:
http://hg.mozilla.org/mozilla-central/rev/4230d1472963

And thanks for the patch Bjarne :)
Flags: in-testsuite+
Whiteboard: [orange][has patch][fixed in cedar] → [orange]
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Thank you all!  :-)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 85

6 years ago
Test only change for a random orange.  Can we land it on aurora?
tracking-firefox5: --- → ?
Comment hidden (Treeherder Robot)

Updated

6 years ago
Attachment #527243 - Flags: approval-mozilla-aurora+

Comment 87

6 years ago
Please land for Aurora by Monday May 16 or the approval will potentially be lost.  Please mark as status-firefox5 fixed when you do.
(Assignee)

Comment 88

6 years ago
I'm really not sure about the procedure to get this into aurora so I'll try first with good, old "checkin-needed" (assuming I'm supposed to drive this)...
Keywords: checkin-needed
(In reply to comment #85)
> Test only change for a random orange.  Can we land it on aurora?

You can generally do this without approval.
Whiteboard: [orange] → [orange] [needs aurora landing]
http://hg.mozilla.org/releases/mozilla-aurora/rev/d350446b6794
http://hg.mozilla.org/releases/mozilla-aurora/rev/b0096e6a2bd4
status-firefox5: --- → fixed
tracking-firefox5: ? → ---
Keywords: checkin-needed
Whiteboard: [orange] [needs aurora landing] → [orange]
Keywords: intermittent-failure
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.