Android jsreftest-1 and jsreftest-2 mostly timing out mostly in e4x/Regress/regress-373082.js following June 6th TM merge

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
8 years ago
7 years ago

People

(Reporter: philor, Unassigned)

Tracking

({intermittent-failure})

Trunk
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Representative example in http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307400826.1307404228.31543.gz&fulltext=1#err0 - at some point in e4x/Regress/regress-373082.js (not the same point in all runs), it apparently drifts off to sleep, there's a section of httpd.js GC (which started filling up the Android logs with bug 508128), then a 2400 seconds without output timeout. So far since the merge, there have been 3 instances of this, 2 of an unrelated infra failure, and one passing run.
(Reporter)

Updated

8 years ago
Depends on: 662470
(Reporter)

Comment 7

8 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307668370.1307671518.4809.gz&fulltext=1
Blocks: 661896
Summary: Android jstreftest-1 mostly timing out in e4x/Regress/regress-373082.js following June 6th TM merge → Android jsreftest-1 mostly timing out in e4x/Regress/regress-373082.js following June 6th TM merge
(Reporter)

Comment 10

8 years ago
Starting to think that regress-373082.js is just the most common, or maybe it's very long so it offers more opportunity to be the one running when it hits. These two are in ecma/Date/15.9.5.9.js and ecma/Expressions/11.7.1.js.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307722194.1307727575.18058.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307698665.1307702713.28087.gz&fulltext=1
Summary: Android jsreftest-1 mostly timing out in e4x/Regress/regress-373082.js following June 6th TM merge → Android jsreftest-1 mostly timing out mostly in e4x/Regress/regress-373082.js following June 6th TM merge
(Reporter)

Comment 11

8 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307706428.1307710235.8033.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Inbound/1307726691.1307732158.3998.gz&fulltext=1
js1_8/regress/regress-384412.js
Summary: Android jsreftest-1 mostly timing out mostly in e4x/Regress/regress-373082.js following June 6th TM merge → Android jsreftest-1 and jsreftest-2 mostly timing out mostly in e4x/Regress/regress-373082.js following June 6th TM merge

Comment 13

8 years ago
What's up with all the before ... after ... messages? Aren't they from gc ? Is there a stuck preference somewhere ?
(Reporter)

Comment 14

8 years ago
They're from httpd.js doing gc, since bug 508128. I'm hoping that it's not a problem, other than being massively annoying in Android logs (particularly when it shows up in the middle of data: URIs in reftest logs).

Comment 15

8 years ago
I'm wondering if the successful deletion of Object.prototype.toString would cause reftest to stop responding? Would saving toString prior to the deletion and restoring it after the assigment to actual help?

expect = 'TypeError: String.prototype.toString called on incompatible XML';

try
{
    delete XML.prototype.function::toString;
> var saveToString = Object.prototype.toString;
    delete Object.prototype.toString;
    actual = <a>TEXT</a>.toString();
> Object.prototype.toString = saveToString
}
catch(ex)
{
    actual = ex + '';
}
TEST(7, expect, actual);
(Reporter)

Comment 23

8 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Inbound/1307764851.1307769546.24158.gz&fulltext=1

We've apparently pretty much thoroughly broken it, I'm hiding it on mozilla-inbound.

Comment 41

8 years ago
Igor, ping comment 15 ?

Comment 85

8 years ago
Posted patch patch (obsolete) — Splinter Review
lets try this.
Attachment #540104 - Flags: review?(igor)
Comment on attachment 540104 [details] [diff] [review]
patch

I think that code's supposed to throw, so putting that in a finally block would be better, if the patch actually does anything to address this.

Comment 89

8 years ago
Thanks Waldo. I'll try it on try.

Updated

8 years ago
Attachment #540104 - Flags: review?(igor)

Comment 93

8 years ago
try doesn't run jsreftests on android yet. philor is going to hide this until we get that resolved.

Comment 94

8 years ago
Comment on attachment 540104 [details] [diff] [review]
patch

> try
> {
>     delete XML.prototype.function::toString;
>+    var saveToString = Object.prototype.toString;
>     delete Object.prototype.toString;
>     actual = <a>TEXT</a>.toString();
>+    Object.prototype.toString = saveToString;
> }
> catch(ex)
> {
>     actual = ex + '';
> }


Waldo is right as we need to restore prototype in any case. So lets write this as:

delete XML.prototype.function::toString;
var xml = <a>TEXT</a>;
var saveToString = Object.prototype.toString;
delete Object.prototype.toString;
try {
    actual = xml.toString();
} catch(ex) {
    actual = ex + '';
} finally {
    Object.prototype.toString = saveToString;
}

Note that the try covers only the part of the code that is expected to throw.
(Reporter)

Updated

8 years ago
Depends on: 665331

Comment 95

8 years ago
Posted patch patch v2Splinter Review
Attachment #540104 - Attachment is obsolete: true
Attachment #540416 - Flags: review?(igor)

Comment 96

8 years ago
Comment on attachment 540416 [details] [diff] [review]
patch v2

Review of attachment 540416 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #540416 - Flags: review?(igor) → review+

Comment 97

8 years ago
http://hg.mozilla.org/tracemonkey/rev/2ff6092157a5

leaving open as we don't know this was the issue.
(Reporter)

Comment 98

8 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1308679224.1308682382.5597.gz&fulltext=1

REFTEST TEST-START | http://10.250.48.211:30091/jsreftest/tests/jsreftest.html?test=e4x/Regress/regress-373082.js
REFTEST TEST-PASS | http://10.250.48.211:30091/jsreftest/tests/jsreftest.html?test=e4x/Regress/regress-373082.js | Section 1 of test - Simpler sharing of XML and XMLList functions  item 1
REFTEST TEST-PASS | http://10.250.48.211:30091/jsreftest/tests/jsreftest.html?test=e4x/Regress/regress-373082.js | Section 2 of test - Simpler sharing of XML and XMLList functions  item 2
REFTEST TEST-PASS | http://10.250.48.211:30091/jsreftest/tests/jsreftest.html?test=e4x/Regress/regressbefore 393216, after 389120, break 057ff000
(etc. etc.)
command timed out: 2400 seconds without output, killing pid 12122
(In reply to comment #93)
> try doesn't run jsreftests on android yet. philor is going to hide this
> until we get that resolved.

Filed bug#666647 to get jsreftests on android enabled in TryServer.
(In reply to comment #94)
> Comment on attachment 540104 [details] [diff] [review] [review]
> patch
> 
> > try
> > {
> >     delete XML.prototype.function::toString;
> >+    var saveToString = Object.prototype.toString;
> >     delete Object.prototype.toString;
> >     actual = <a>TEXT</a>.toString();
> >+    Object.prototype.toString = saveToString;
> > }
> > catch(ex)
> > {
> >     actual = ex + '';
> > }
> 
> 
> Waldo is right as we need to restore prototype in any case. So lets write
> this as:
> 
> delete XML.prototype.function::toString;
> var xml = <a>TEXT</a>;
> var saveToString = Object.prototype.toString;
> delete Object.prototype.toString;
> try {
>     actual = xml.toString();
> } catch(ex) {
>     actual = ex + '';
> } finally {
>     Object.prototype.toString = saveToString;
> }
> 
> Note that the try covers only the part of the code that is expected to throw.

Has this patch landed in m-c yet? I ask because I see later reports of continues problems, but cant tell if this fix landed yet or not.
(In reply to comment #100)

> Has this patch landed in m-c yet? I ask because I see later reports of
> continues problems, but cant tell if this fix landed yet or not.

No, but the problem still persists on tracemonkey where it did land.
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/2ff6092157a5
Note: not marking as fixed because fixed-in-tracemonkey is not present on the whiteboard.
Depends on: 668594
Is this still happening, or did bug 668594 fix it?
that bug landed on 07-09 (4 days ago) on mozilla-central.  I would say this is fixed, and if we see issues we can reopen.  If y'all agree, change the status.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WORKSFORME
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.