Filed by: philringnalda@gmail.com https://treeherder.mozilla.org/logviewer.html#?job_id=385359&repo=autoland http://archive.mozilla.org/pub/firefox/tinderbox-builds/autoland-linux-pgo/1467941401/autoland_ubuntu32_vm_test_pgo-jsreftest-bm06-tests1-linux32-build2.txt.gz https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=http://archive.mozilla.org/pub/firefox/tinderbox-builds/autoland-linux-pgo/1467941401/autoland_ubuntu32_vm_test_pgo-jsreftest-bm06-tests1-linux32-build2.txt.gz&only_show_unexpected=1

Started on a push of yours AFAICT. Can you please investigate this annoyingly-frequent orange, arai?

Apparently the error itself started from bug 1280362. https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=cbf85303aeb2cea6326574a39db03dad860b578f it touches the test harness, so it's possible that it causes the issue. not sure why it becomes intermittent orange tho...

from IRC talk, there should be another issue than the TypeError. I think, it would be better fixing the TypeError first, as it's hard to figure out the intermittent hidden behind many permanent errors.

Neat. This is actually what I call permagreenorange, every single jsreftest run (except some chunks on the platforms where it runs in more than one chunk) has so much of this spew that it is the only thing visible on treeherder, masking any actual failures unless they happen to come before the spew has maxed out what treeherder will parse. Clock's ticking, being unable to see failures on treeherder is a visiblity violation so the correct thing for me to have done upon seeing that would have been to hide it on every trunk tree and aurora, and only *then* comment.

https://hg.mozilla.org/mozilla-central/diff/bbde749db2bd/js/src/tests/shell.js#l200 > + /** Peeks at the top of the call stack. */ > + function currentFunc() { > + assertEq(callStack.length > 0, true, > + "must be a current function to examine"); > + > + return callStack[callStack.length - 1]; > + } > + global.currentFunc = currentFunc; > ... > -function currentFunc() > -{ > - return callStack[callStack.length - 1]; > -} callStack was not used properly. Most testcase doesn't contain calls to enterFunc/exitFunc. That means callStack.length is 0 in most cases :/ So, the assert there shouldn't be added.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e240c90e6a96 removed the assertion, and changed to just return "top level script" string if callStack.length is 0. I think we should remove all those manual callStack handling, and use "new Error().stack" instead. but that needs many change, that won't fit aurora uplift. So, I'd like to go with this as a first step.

(In reply to Tooru Fujisawa [:arai] from comment #7) > callStack was not used properly. > Most testcase doesn't contain calls to enterFunc/exitFunc. > That means callStack.length is 0 in most cases :/ > So, the assert there shouldn't be added. That doesn't explain why the failure is *intermittent* and not permanent, does it? I mean, the call-stack thingy here is a bit stupid IMO, and if we get rid of it, great. (Although the number of ancient tests to change to do that is somewhat daunting, and possibly not worth the trouble.) But the assertion can't be wrong, otherwise it would be making something always fail. Something is bad in JS execution if we're hitting this, but only sometimes.

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #9) > (In reply to Tooru Fujisawa [:arai] from comment #7) > > callStack was not used properly. > > Most testcase doesn't contain calls to enterFunc/exitFunc. > > That means callStack.length is 0 in most cases :/ > > So, the assert there shouldn't be added. > > That doesn't explain why the failure is *intermittent* and not permanent, > does it? I mean, the call-stack thingy here is a bit stupid IMO, and if we > get rid of it, great. (Although the number of ancient tests to change to do > that is somewhat daunting, and possibly not worth the trouble.) But the > assertion can't be wrong, otherwise it would be making something always > fail. Something is bad in JS execution if we're hitting this, but only > sometimes. The TypeError is permanent, but doesn't caught by test harness. it hides the actual cause of intermittent, because the TypeError prints so much error-like log. So I'm about to fix the permanent failure first.

I know this is not right solution for whole wrong callStack things, but this should remove the extra TypeError that was introduced by the bug 1280362 patch, and helps investigating the intermittent, that is more important here.

forgot to link the log https://treeherder.mozilla.org/logviewer.html#?job_id=25076329&repo=try#L159686-L159687 there is no more troublesome TypeError.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1988d3ecc0349a70bc1b18443a7caa4a5dfd1bb6 Bug 1285465 - Do not throw when callStack is empty. r=jwalden

Comment on attachment 8777667 [details] [diff] [review] Do not throw when callStack is empty. Approval Request Comment > [Feature/regressing bug #] bug 1280362 > [User impact if declined] Hard to investigate other intermittent failure, because of permanent failure with many error message that is not caught by test harness (doesn't become orange) but shown in logviewer. > [Describe test coverage new/current, TreeHerder] tested in treeherder in m-c. > [Risks and why] Low, this changes assertion that is known to fail into fallback code. > [String/UUID change made/needed] None

Hello Tomcat, Wes: Has this fix helped get rid of the intermittent on Nightly? I would like to get confirmation before we uplift to Aurora. Thanks!

Judging by the downward slope on https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1285465 this seems to have solved the problem.

(In reply to Wes Kocher (:KWierso) from comment #18) > Judging by the downward slope on > https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1285465 this > seems to have solved the problem. Thanks! I'll take it then.

Comment on attachment 8777667 [details] [diff] [review] Do not throw when callStack is empty. Fix for an intermittent failure that seems promising based on Nightly data, Aurora50+

Apparently, the actual intermittent oranges are filed separately (bugs with "application terminated with exit code 5") than this bug. removing leave-open.