Closed Bug 658441 Opened 10 years ago Closed 10 years ago

intermittent crashtests/552651.html | load failed: timed out waiting for reftest-wait to be removed


(Core :: General, defect)

Not set





(Reporter: mcmanus, Assigned: bjarne)



(Keywords: intermittent-failure)


(1 file)

seen on push

there are several other rare oranges of the reftest-wait to be removed variety.. its not clear to me that they are dups or not.

REFTEST TEST-START | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/554230-1.xhtml
REFTEST TEST-PASS | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/554230-1.xhtml | (LOAD ONLY)
REFTEST INFO | Loading a blank page
REFTEST TEST-START | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/552651.html
REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/552651.html | load failed: timed out waiting for reftest-wait to be removed
REFTEST INFO | Saved log: START file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/552651.html
REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering WaitForTestEnd
REFTEST INFO | Saved log: [CONTENT] WaitForTestEnd: Adding listeners
REFTEST INFO | Saved log: Initializing canvas snapshot
Blocks: 438871
So why is that 1000ms timeout there?

That said, reftest uses a 5 min timeout, so I'm a little surprised this test timed out...
Iirc, setTimeout() is used because I was unaware of executeSoon(). The 1000ms is random.
Whiteboard: [orange]
Let's change that for a start, then.
Change setTimeout() or 1000ms? Bigger or lower value? :)

The idea of the test is to make sure we don't leak if we abort an xhr in state 3. See bug #552651, comment #12 for why it ended up as a crashtest. My understanding is that clearing the classname is necessary to make the test finish..? (A quick search reveals that a number of crashtests in layout/base seems to use this technique.)

The only way I can interpret comment #0 is that clearing the classname does not happen (hence the test times out) indicating that setTimeout failed..?
> Change setTimeout() or 1000ms? Bigger or lower value? :)

Use executeSoon.

Chances are, the machine was under heavy load and the sync network load plus setTimeout call just took longer than the reftest timeout....
How would executeSoon() help in this situation? (Btw, the xhr loads async if this is what you mean by "sync network load".)
> How would executeSoon() help in this situation? 

By not going to the event loop for as long?

Wait.  So the setTimeout is just needed... why?  To make sure we stop the test after the async XHR gets aborted?
(In reply to comment #7)
> To make sure we stop the
> test after the async XHR gets aborted?

Yes. I'd be happy to learn about other, more robust ways.
Can you not just listen for onreadystatechange going into state 4?
Fair enough - this test works locally.
Assignee: nobody → bjarne
Attachment #534702 - Flags: review?(bzbarsky)
Comment on attachment 534702 [details] [diff] [review]
Dropping timeout, relying on getting to readystate==4

Attachment #534702 - Flags: review?(bzbarsky) → review+
Keywords: checkin-needed
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Can anyone please provide some guidelines that I can use to verify this fix? Do I need to create any specific environment to test it?

Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.