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

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Mark Banner (:standard8)]

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

[(no longer active)]

Is there a stack for this assertion?

Comment 2

[Mark Banner (:standard8)]

Attachment: Stack of assertion

Yes, but it is all js basically.

Comment 3

[(no longer active)]

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)]

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)]

Attachment: nsFileStream debug

Just for historical purposes, here's the patch I used to debug nsFileStream.

Comment 6

[Mark Banner (:standard8)]

Attachment: 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)]

Attachment: 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

[(no longer active)]

Comment on attachment 637674 [details] [diff] [review]
The fix

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

Looks good, thanks for figuring it out!

Comment 9

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7da6936a2c8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c7da6936a2c8

Comment 11

[Mark Banner (:standard8)]

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]

Comment on attachment 637674 [details] [diff] [review]
The fix

low risk, fixes orange test, totally approved.

Comment 13

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5bd681b5cbcb