Closed Bug 662468 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: philor, Unassigned)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

patch v2
1.18 KB, patch
igor
: review+
Details | Diff | Splinter Review 
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.
Depends on: 662470
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
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
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
What's up with all the before ... after ... messages? Aren't they from gc ? Is there a stuck preference somewhere ?
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).
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);
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.
Igor, ping comment 15 ?
Attached 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.
Thanks Waldo. I'll try it on try.
Attachment #540104 - Flags: review?(igor)
try doesn't run jsreftests on android yet. philor is going to hide this until we get that resolved.
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.
Depends on: 665331
Attached patch patch v2 — — Splinter Review 

        Attachment #540104 -
        Attachment is obsolete: true

        Attachment #540416 -
        Flags: review?(igor)

    
    

    
  
Comment on attachment 540416 [details] [diff] [review]
patch v2

Review of attachment 540416 [details] [diff] [review]:
-----------------------------------------------------------------

        Attachment #540416 -
        Flags: review?(igor) → review+

    
    

    
  
http://hg.mozilla.org/tracemonkey/rev/2ff6092157a5

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

    
    

    
  
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
Closed: 13 years ago
Resolution: --- → WORKSFORME
Whiteboard: [orange]

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: