Last Comment Bug 658441 - intermittent crashtests/552651.html | load failed: timed out waiting for reftest-wait to be removed
: intermittent crashtests/552651.html | load failed: timed out waiting for reft...
Status: RESOLVED FIXED
: intermittent-failure
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla7
Assigned To: Bjarne (:bjarne)
:
Mentors:
Depends on:
Blocks: 438871
  Show dependency treegraph
 
Reported: 2011-05-19 18:34 PDT by Patrick McManus [:mcmanus]
Modified: 2012-11-25 19:31 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Dropping timeout, relying on getting to readystate==4 (1018 bytes, patch)
2011-05-24 03:16 PDT, Bjarne (:bjarne)
bzbarsky: review+
Details | Diff | Review

Description Patrick McManus [:mcmanus] 2011-05-19 18:34:44 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1305852691.1305852965.4801.gz&fulltext=1#err0

seen on push
http://hg.mozilla.org/mozilla-central/rev/9197b91fd9f4

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
Comment 1 Boris Zbarsky [:bz] 2011-05-20 06:28:01 PDT
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...
Comment 2 Bjarne (:bjarne) 2011-05-20 06:52:39 PDT
Iirc, setTimeout() is used because I was unaware of executeSoon(). The 1000ms is random.
Comment 3 Boris Zbarsky [:bz] 2011-05-20 07:11:16 PDT
Let's change that for a start, then.
Comment 4 Bjarne (:bjarne) 2011-05-20 13:57:52 PDT
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..?
Comment 5 Boris Zbarsky [:bz] 2011-05-20 14:01:44 PDT
> 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....
Comment 6 Bjarne (:bjarne) 2011-05-20 14:10:16 PDT
How would executeSoon() help in this situation? (Btw, the xhr loads async if this is what you mean by "sync network load".)
Comment 7 Boris Zbarsky [:bz] 2011-05-20 14:22:46 PDT
> 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?
Comment 8 Bjarne (:bjarne) 2011-05-21 03:39:16 PDT
(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.
Comment 9 Boris Zbarsky [:bz] 2011-05-24 00:03:29 PDT
Can you not just listen for onreadystatechange going into state 4?
Comment 10 Bjarne (:bjarne) 2011-05-24 03:16:06 PDT
Created attachment 534702 [details] [diff] [review]
Dropping timeout, relying on getting to readystate==4

Fair enough - this test works locally.
Comment 11 Boris Zbarsky [:bz] 2011-05-24 13:08:19 PDT
Comment on attachment 534702 [details] [diff] [review]
Dropping timeout, relying on getting to readystate==4

r=me
Comment 12 Dão Gottwald [:dao] 2011-05-25 01:28:09 PDT
http://hg.mozilla.org/mozilla-central/rev/cbbec95be916
Comment 13 Mihaela Velimiroviciu (:mihaelav) 2011-08-23 06:34:07 PDT
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?

Thanks!

Note You need to log in before you can comment on or make changes to this bug.