829650 - Talos runs mozcrash more than once on the same minidump

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Ed Morley [:emorley]]

(Note spotted this on runs before the recent deploy, so unrelated)

On:
https://tbpl.mozilla.org/php/getParsedLog.php?id=18716694&tree=Mozilla-Inbound

Summary:
{
mozcrash ERROR | PROCESS-CRASH | ttest.py | application crashed [@ nssCertificate_Destroy]
Thread 4 (crashed)
mozcrash ERROR | PROCESS-CRASH | ttest.py | application crashed [@ nssCertificate_Destroy]
Thread 4 (crashed)
talosError: 'stack found after process termination (org.mozilla.fennec: terminated by testAgent.killProcess, plugin-container: terminated by testAgent.killProcess, crashreporter: terminated by testAgent.killProcess)'
}

If you look at both of the crash entries, they are using the same minidump (37a2d562-34e3-3f6a-2613a290-6a79a19b.dmp).

I'm presuming we didn't clean up successfully after the first run, so low and behold find a minidump after the next run too.

In ttest.py::cleanupAndCheckForCrashes() we have:
{
        if browser_config['remote'] == True:
            # cleanup dumps on remote
            self._hostproc.removeDirectory(minidumpdir)
}

So we should be cleaning up surely?

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

here is what is happening:
we have a function cleanupAndCheckForCrashes() where we raise an error:
http://hg.mozilla.org/build/talos/file/tip/talos/ttest.py#l168

The problem is we call this from:
http://hg.mozilla.org/build/talos/file/tip/talos/ttest.py#l380

and when the error is raised, we fall into the 'except' clause where we call cleanupAndCheckForCrashes() again!


to fix this should we have a finally clause for cleanup, etc...?

Comment 2

[Ed Morley [:emorley]]

Could we just get rid of the raises at http://hg.mozilla.org/build/talos/file/tip/talos/ttest.py#l168 ?

This would not only solve this problem, but also mean we don't get the redundant "stack found after process termination" lines now that we have PROCESS-CRASH working correctly.

I don't know how this would affect the result of the run (I haven't looked yet), but it would be good if crashes turned the run orange instead of red - which presumably would be done by stopping the exception and handling in buildbot like we do for the other suites?

Comment 3

[Ed Morley [:emorley]]

(I think making the crashes appear orange would go some way to making devs realise it's an in-product problem not an automation problem, for these many many crashes)

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

if we have a crash it should be orange, since it is not infra, machine, network related.

to get rid of the double crash, a finally wouldn't work, we could set a state variable such that it would indicate if we have processedCrashes and to ignore future processing.  More likely we should remove all crash files from the device and local host once we are done processing them.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

I wonder how to make talos turn orange?  TEST-UNEXPECTED-FAIL ?

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: remove the crash from the device if we process it (1.0)

Comment 7

[Jeff Hammel]

Comment on attachment 701856 [details] [diff] [review]
remove the crash from the device if we process it (1.0)

lgtm!

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/build/talos/rev/5f5e467ab89b