Closed Bug 906100 Opened 11 years ago Closed 11 years ago

Intermittent failures in tests that list sources, but don't call gc() after adding test globals

Categories

(DevTools :: Debugger, defect)

x86_64
Linux
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: decoder, Assigned: fitzgen)

References

(Blocks 1 open bug)

Details

(Whiteboard: [asan][asan-test-failure])

Attachments

(1 file, 1 obsolete file)

The two mentioned xpcshell tests fail on AddressSanitizer builds:

https://tbpl.mozilla.org/?tree=Try&rev=b7a423bc4b04

A manual bisection pointed to bug 890555 as the regressor. This is a blocker for getting our tests to run, as this is the last failure turning them orange.
Component: XPCShell Harness → Developer Tools
Product: Testing → Firefox
Summary: ASan: xpcshell tests toolkit/devtools/server/tests/unit/test_sourcemaps-06.js and toolkit/devtools/server/tests/unit/head_dbg.js fail → ASan: xpcshell test toolkit/devtools/server/tests/unit/test_sourcemaps-06.js fails
Looked into the full log a bit.

We should get three sources: http://example.com/www/js/a.js, http://example.com/www/js/b.js, and http://example.com/www/js/c.js, but in fact we are getting a fourth source "/builds/slave/test/build/tests/xpcshell/tests/toolkit/devtools/server/tests/unit/head_dbg.js". Because it isn't one of our mocked sources, it doesn't have the expected source content ("content for " + url; see line 81).

What I *think* might be happening is that the Debugger.Script created by the evalInSandbox call inside addTestGlobal hasn't been gc'd yet, and so we create a source for head_dbg.js. I think we have hit this before, and have added manual calls to force gc before we start testing stuff. I think perhaps a better solution than calling that all over the place, would be to do it in addTestGlobal.
Attached patch gc-addTestGlobal.patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=f64ad2c01c60
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Attachment #791549 - Flags: review?(jimb)
This actually has nothing to do with ASan, and you guys just happened to pick up this latent intermittent.
Component: Developer Tools → Developer Tools: Debugger
Summary: ASan: xpcshell test toolkit/devtools/server/tests/unit/test_sourcemaps-06.js fails → Intermittent failures in tests that list sources, but don't call gc() after adding test globals
Comment on attachment 791549 [details] [diff] [review]
gc-addTestGlobal.patch

Confirmed to fix my problem on try, thanks :)
Attachment #791549 - Flags: feedback+
Just a note, but ae91affad44f (included in that push) was backed out for test failures. I am 99% sure my changes aren't causing that try push to be so sad, but I will push to try again in a minute.
Comment on attachment 791549 [details] [diff] [review]
gc-addTestGlobal.patch

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

Couldn't we just avoid GC-sensitivity altogether? Is that eval really needed?
Attachment #791549 - Flags: review?(jimb)
Good catch, we don't really need to eval, so we can avoid the hack altogether.

https://tbpl.mozilla.org/?tree=Try&rev=8fd6274c08d0
Attachment #791549 - Attachment is obsolete: true
Attachment #792373 - Flags: review?(jimb)
Comment on attachment 792373 [details] [diff] [review]
bug-906100-gc.patch

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

Love it.
Attachment #792373 - Flags: review?(jimb) → review+
Whiteboard: [asan][asan-test-failure] → [asan][asan-test-failure][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/45b023d6ed73
Whiteboard: [asan][asan-test-failure][land-in-fx-team] → [asan][asan-test-failure][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/45b023d6ed73
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [asan][asan-test-failure][fixed-in-fx-team] → [asan][asan-test-failure]
Target Milestone: --- → Firefox 26
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: