Last Comment Bug 766269 - Permanent orange: TEST_UNEXPECTED_FAIL | test_0030_general.js | test failed (with xpcshell return code: -2147483645) - application crashed ###!!! ASSERTION: mTempFile not equal to mTargetFile
: Permanent orange: TEST_UNEXPECTED_FAIL | test_0030_general.js | test failed (...
Status: RESOLVED FIXED
: intermittent-failure
Product: Toolkit
Classification: Components
Component: Application Update (show other bugs)
: Trunk
: All Windows XP
: -- critical (vote)
: mozilla16
Assigned To: Mark Banner (:standard8)
:
Mentors:
Depends on:
Blocks: bgupdates
  Show dependency treegraph
 
Reported: 2012-06-19 12:53 PDT by Mark Banner (:standard8)
Modified: 2012-11-25 19:31 PST (History)
2 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Stack of assertion (6.88 KB, text/plain)
2012-06-19 13:24 PDT, Mark Banner (:standard8)
no flags Details
nsFileStream debug (1.31 KB, patch)
2012-06-28 08:06 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
Potential fix (2.17 KB, patch)
2012-06-28 08:43 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
The fix (2.26 KB, patch)
2012-06-28 14:26 PDT, Mark Banner (:standard8)
ehsan: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Review

Description Mark Banner (:standard8) 2012-06-19 12:53:14 PDT
Since the background updates (bug 307181) landed, we've been seeing this assertion on our debug Windows XP tests. It has been covered up a bit until now. Possibly related to bug 766264.

https://tbpl.mozilla.org/php/getParsedLog.php?id=12798852&tree=Thunderbird-Trunk#error0

TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\xpcshell\tests\toolkit\mozapps\update\test\unit\test_0030_general.js | test failed (with xpcshell return code: -2147483645), see following log:
>>>>>>>
### XPCOM_MEM_LEAK_LOG defined -- logging leaks to c:\docume~1\cltbld\locals~1\temp\tmpd9mc_v\runxpcshelltests_leaks.log

TEST-INFO | (xpcshell/head.js) | test 1 pending

TEST-INFO | (xpcshell/head.js) | test 2 pending
TEST-INFO | c:/talos-slave/test/build/xpcshell/tests/toolkit/mozapps/update/test/unit/head_update.js | [setUpdateURLOverride : 773] setting app.update.url.override to http://localhost:4444/update.xml

TEST-INFO | (xpcshell/head.js) | test 3 pending

TEST-INFO | (xpcshell/head.js) | test 3 finished

TEST-INFO | (xpcshell/head.js) | running event loop
TEST-INFO | c:/talos-slave/test/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0030_general.js | [run_test_pt1 : 90] mar download with a valid MD5 hash
*** AUS:SVC Checker:getUpdateURL - update URL: http://localhost:4444/update.xml?force=1
*** AUS:SVC gCanCheckForUpdates - able to check for updates
*** AUS:SVC Checker:checkForUpdates - sending request to: http://localhost:4444/update.xml?force=1

TEST-INFO | (xpcshell/head.js) | test 3 pending

TEST-INFO | (xpcshell/head.js) | test 3 finished
*** AUS:SVC Checker:onLoad - request completed downloading document
*** AUS:SVC Checker:getUpdateURL - update URL: http://localhost:4444/update.xml?force=1
*** AUS:SVC Checker:onLoad - number of updates available: 1
TEST-INFO | c:/talos-slave/test/build/xpcshell/tests/toolkit/mozapps/update/test/unit/head_update.js | [UCL_onCheckComplete : 2422] url = http://localhost:4444/update.xml?force=1, request.status = 400, update.statusText = undefined, updateCount = 1

TEST-INFO | (xpcshell/head.js) | test 3 pending

TEST-INFO | (xpcshell/head.js) | test 3 finished

TEST-PASS | c:/talos-slave/test/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0030_general.js | [check_test_helper_pt1_1 : 65] 1 == 1
*** AUS:SVC readStringFromFile - file doesn't exist: C:\Documents and Settings\cltbld\Local Settings\Application Data\\\thunderbird\updates\0\update.status
*** AUS:SVC readStatusFile - status: null, path: C:\Documents and Settings\cltbld\Local Settings\Application Data\\\thunderbird\updates\0\update.status
*** AUS:SVC UpdateManager:_loadXMLFileIntoArray: XML file does not exist
*** AUS:SVC UpdateManager:_loadXMLFileIntoArray: XML file does not exist
*** AUS:SVC UpdateManager:_loadXMLFileIntoArray: XML file does not exist
*** AUS:SVC Downloader:downloadUpdate - downloading from http://localhost:4444/data/simple_no_pib.mar to C:\Documents and Settings\cltbld\Local Settings\Application Data\\\thunderbird\updates\0\update.mar
###!!! ASSERTION: mTempFile not equal to mTargetFile: 'Error', file e:/builds/moz2_slave/tb-comm-cen-w32-dbg/build/mozilla/netwerk/base/src/nsFileStreams.cpp, line 759
Comment 1 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-19 13:03:34 PDT
Is there a stack for this assertion?
Comment 2 Mark Banner (:standard8) 2012-06-19 13:24:27 PDT
Created attachment 634563 [details]
Stack of assertion

Yes, but it is all js basically.
Comment 3 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-19 13:27:27 PDT
If you have some time to debug this, it would be great if you can instrument a build to call DumpJSStack() when that NS_ERROR is reached, and get to the JS stack there, so that we can start to figure out what's going on here.
Comment 4 Mark Banner (:standard8) 2012-06-28 06:58:02 PDT
Ok, I did a try server run today, with a patched nsFileStream, here's the interesting stuff...

The NS_ERROR is:

###!!! ASSERTION: mTempFile not equal to mTargetFile: 'Error', file e:/builds/moz2_slave/tb-try-c-cen-w32-dbg/build/mozilla/netwerk/base/src/nsFileStreams.cpp, line 769

The two files it is talking about have paths:

Target File Path: C:\Users\cltbld\AppData\Local\\\thunderbird\updates\0\update.status
Temp File Path: C:\Users\cltbld\AppData\Local\thunderbird\updates\0\update.status

Hence Target file isn't normalised :-(

JS Stack:

0 FileUtils_closeSafeFileOutputStream() ["resource://gre/modules/FileUtils.jsm":120]
1 <TOP LEVEL> ["<unknown>":0]
2 writeStringToFile() ["jar:file:///c:/talos-slave/test/build/thunderbird/omni.ja!/components/nsUpdateService.js":841]
3 writeStatusFile() ["jar:file:///c:/talos-slave/test/build/thunderbird/omni.ja!/components/nsUpdateService.js":610]
4 Downloader_downloadUpdate() ["jar:file:///c:/talos-slave/test/build/thunderbird/omni.ja!/components/nsUpdateService.js":2960]
5 AUS_downloadUpdate() ["jar:file:///c:/talos-slave/test/build/thunderbird/omni.ja!/components/nsUpdateService.js":2094]
6 <TOP LEVEL> ["<unknown>":0]
7 check_test_helper_pt1_1() ["c:/talos-slave/test/build/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0030_general.js":68]
8 anonymous() ["c:\talos-slave\test\build\xpcshell\head.js":407]
Comment 5 Mark Banner (:standard8) 2012-06-28 08:06:21 PDT
Created attachment 637514 [details] [diff] [review]
nsFileStream debug

Just for historical purposes, here's the patch I used to debug nsFileStream.
Comment 6 Mark Banner (:standard8) 2012-06-28 08:43:20 PDT
Created attachment 637527 [details] [diff] [review]
Potential fix

So what I think is going on here is that xpcshell isn't taking account of the fact that Thunderbird doesn't have a vendor id or basename when setting the update directory. This gives us the strange \\\ path, which the test obviously doesn't like.

The patch will hopefully fix that.

I'm not sure if it'll fix bug 766264 as well, but at least it would be a step forward.
Comment 7 Mark Banner (:standard8) 2012-06-28 14:26:01 PDT
Created attachment 637674 [details] [diff] [review]
The fix

This fixes this issue (it doesn't fix bug 766264) - by only adding the vendor and base name when they are set, then we get the correct path. I did consider using normalize, but I wasn't sure if that would deal with the \\\ correctly - especially as that was happening in the sprintf part, and appending it and letting nsIFile do the job seemed much more appropriate.

I've pushed this to Firefox try here:

https://tbpl.mozilla.org/?noignore=1&tree=Try&rev=bc0ad78976da

The Thunderbird try server run is here:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&noignore=1&rev=d4c39f9435b0
Comment 8 :Ehsan Akhgari (busy, don't ask for review please) 2012-06-28 16:27:01 PDT
Comment on attachment 637674 [details] [diff] [review]
The fix

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

Looks good, thanks for figuring it out!
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-06-30 12:46:30 PDT
https://hg.mozilla.org/mozilla-central/rev/c7da6936a2c8
Comment 11 Mark Banner (:standard8) 2012-07-02 01:01:09 PDT
Comment on attachment 637674 [details] [diff] [review]
The fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 307181
User impact if declined: None, test-only fix
Testing completed (on m-c, etc.): Landed on inbound, central
Risk to taking this patch (and alternatives if risky): Test-only, so none
String or UUID changes made by this patch: None

This is a test-only fix that takes account of Thunderbird not having the vendor/basename set (and possibly other apps using gecko) and avoids permanent orange due to assertions in debug mode.
Comment 12 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-02 16:34:56 PDT
Comment on attachment 637674 [details] [diff] [review]
The fix

low risk, fixes orange test, totally approved.
Comment 13 Mark Banner (:standard8) 2012-07-03 01:02:38 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/5bd681b5cbcb

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