1943038 - geckodriver doesn't find minidump files for content process crashes

Status: RESOLVED FIXED

(Testing :: geckodriver, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

While working on tests for bug 1880257 I noticed that for the wdspec test for content crashes the minidump files are not copied to the folder as specified with the MINIDUMP_SAVE_PATH environment. When having a look at our implementation I noticed that we are actually trying to copy the minidump files before waiting for the browser process to have shutdown:

https://searchfox.org/mozilla-central/rev/ef34dd879ef8d4a0f3d68d371d23caebcd8fef65/testing/geckodriver/src/marionette.rs#1355-1369

We need to switch the order to actually check for minidump files when the Firefox process is gone.

Comment 1

[Alexandra Borovova [:Sasha]]

I've tried to test with about:crashcontent and for me, it looks like that we don't see on the geckodriver side that connection is closed, so the close method where we copy the files doesn't get executed. If I try to send another command after, then the whole chain is executed and the minidump files are copied.

Also, just coping the files after the browser shutdown doesn't seem to work, because the profile is gone by then. If we want to do it, we have to delay the removal of profile or do the copying of minidump files at a different codepoint.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So there is a race condition between sending the next command and the shutdown duration of Firefox in case of a content crash. Sending the next command immediately will cause a invalid session id error and also by then the minidump files are not copied. Only waiting at least until the browser process is gone will result in a unknown error with the minidump files copied.

Maybe we should check for the minidump files before removing the profile, but it could still be that we need something else. The exit code of Firefox in case of a content crash is 245.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1943038 - [geckodriver] Use drop implementation to clean-up browser state.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1943038 - [geckodriver] Only print MINIDUMP_SAVE_PATH info line if minidump files are present.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c119a1524b71
https://hg.mozilla.org/integration/autoland/rev/90d3e98eb435
[geckodriver] Use drop implementation to clean-up browser state. r=geckodriver-reviewers,jgraham
https://github.com/mozilla-firefox/firefox/commit/4c3bdc43b33a
https://hg.mozilla.org/integration/autoland/rev/c39a0c557412
[geckodriver] Only print MINIDUMP_SAVE_PATH info line if minidump files are present. r=geckodriver-reviewers,Sasha

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/90d3e98eb435
https://hg.mozilla.org/mozilla-central/rev/c39a0c557412