1521191 - Failing to produce a leak log does not cause a test failure

Status: NEW

(Testing :: Mochitest, defect, P2)

Description

[Andrew McCreight [:mccr8]]

I did a try push with a patch that makes it so that it never prints out a leak log, and this does not cause an orange on TreeHerder:

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=222757782&repo=try&lineNumber=5844

Note that you do get a FAIL if you run the tests locally through mach.

Bug 1518115 is about how we print out the "missing output line" message even if the failure is expected, while this is how the test job does not turn orange if we actually hit that failure.

This is probably not hiding any leaks because the most common cause of this error is a crash, but we should make sure of that.

Comment 1

[Andrew McCreight [:mccr8]]

I'm not exactly sure when this code is run, but it only handles the leaking failure case from mozleak_total, and not the missing log case, so maybe that's the reason for not failing? https://hg.mozilla.org/mozilla-central/rev/d40ebdfc91a0

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Never print out a leak log. DO NOT LAND

Here's the patch I used to disable leak logging.

Comment 3

[Andrew McCreight [:mccr8]]

James, can you take a look at this? Thanks.

Comment 4

[Andrew McCreight [:mccr8]]

I think I have a handle on how to fix this. I think the logic for mozleak_total in statushandler.py needs to be extended to handle this case.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: Bug 1521191, part 1 - Add method to disable dump statistics.

In order to test the test harness's handling of a process failing to
produce a leak log, add a special function that disables the bloat log
output.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Bug 1521191, part 2 - Ensure missing leak logs cause mozharness to fail.

Depends on D17534

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: Bug 1521191, part 3 - Add a function to create a negative leak.

This is needed to test the test harness's handling of a negative leak
being reported by the XPCOM leak checker.

Depends on D17535

Comment 8

[Andrew McCreight [:mccr8]]

Attachment: Bug 1521191, part 4 - Add a test for producing an error if there's a negative leak.

Depends on D17536

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6ce4b187195
part 1 - Add method to disable dump statistics. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/38d3da4804d7
part 2 - Ensure missing leak logs cause mozharness to fail. r=ahal
https://hg.mozilla.org/integration/autoland/rev/3782d011cc9f
part 3 - Add a function to create a negative leak. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/138e162d2778
part 4 - Add a test for producing an error if there's a negative leak. r=ahal

Comment 10

[Dorel Luca [:dluca]]

Backed out 4 changesets (bug 1521191) for causing leaks on multiple tests

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=224150195&repo=autoland&lineNumber=53008

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=224150195&searchStr=linux%2Cdebug%2Cweb%2Cplatform%2Ctests%2Ctest-linux32%2Fdebug-web-platform-tests-16%2Cw%28wpt16%29&revision=138e162d2778d37a1b3cc474aade64647bbc1bd6

Backout:
https://hg.mozilla.org/integration/autoland/rev/044a86a4032cf1ab76e4cd43f5d7929fcff0c9a0

Comment 11

[Andrew McCreight [:mccr8]]

There's a leak in the webrtc/ directory, which is hitting the "NSS is still alive" fatal assertion, so we crash and don't produce a leak log. That assertion should probably be changed because "crash on leak" is not helpful. Anyways, I'm not sure if this leak is new or I just failed to do a try push with these patches.

Comment 12

[Andrew McCreight [:mccr8]]

There's a number of issues here:

• I think this WebRTC leak is a regression, so that should get tracked down.
• The "NSS_Shutdown failed" crash should be turned into a leak, not a crash.
• There needs to be a way to specify in a dir.ini file that there is no leak for a process type, and that needs to be updated on import. I don't see anything like that right now, but maybe I'm just missing it. I meant to look at this before I landed this, but I forgot about it.

Comment 13

[Andrew McCreight [:mccr8]]

I found my try push for these patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42f5f6be042d6a915f37bc3943cf3d11ee9411ba

It does contain the NSS_Shutdown crash, but it is still somehow green. I think this implies that:

1. The WebRTC leak isn't a regression.
2. My patch somehow doesn't actually detect a missing leak log for the WebRTC leak.
3. There's some other crash that has started happening?

The logs don't seem to contain the pids of things that crash which makes it hard to figure out what is going on.

In fact, with my try push, there are a bunch of other crashes in WebRTC that show up in the "failure summary" but didn't cause the test to go orange, which is peculiar:
PROCESS-CRASH | /webrtc/RTCPeerConnection-removeTrack.https.html | application crashed [@ libxul.so + 0xd1a42f]

Comment 14

[Andrew McCreight [:mccr8]]

Oh, I see, bug 1519862 recently landed and made the leak checker ignore missing logs for content processes. The orange in my attempted landed was from a main process crash. That at least explains that.

Comment 16

[Andrew McCreight [:mccr8]]

I'm not going to be able to get to this any time soon.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Marking fix-optional across all versions to remove this from weekly regression triage.