1451310 - [mozrunner] RuntimeError('Calling poll() on a non started process is not allowed.) in BaseRunner.cleanup()

Status: RESOLVED FIXED

(Testing :: Mozbase, defect, P1)

Description

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

Today I have seen that when mozrunner tries to create a new ProcessHandler instance but then fails to run it, the following assertion is shown in `cleanup()`:

https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/base/runner.py#128-129

> Exception RuntimeError: RuntimeError('Calling poll() on a non started process is not allowed.',) in <bound method GeckoRuntimeRunner.__del__ of <mozrunner.base.browser.GeckoRuntimeRunner object at 0x1064f6d90>> ignored

In this case the problem is that the call to 'self.Process()' in processhandler.py fails and `self.proc` is not getting set. Then in the above `start()` method the retrieval of the pid in line 132 fails.

Comment 1

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

I have to admit that the last part of my first comment is wrong. In case of `self.Process()` fails an exception is thrown which bubbles up to `start()` and higher, so the `pid` will not be retrieved. But some harnesses run a cleanup of the runner at the end of the test run, and as such fail.

It can be reproduced when modifying mozprofile to always have a Unicode char in the profile path, and then by running `mach crashtest`.

Comment 2

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

Attachment: Bug 1451310 - [mozrunner] Catch exceptions when starting the client process.

If a ProcessHandler instance has been created, but mozprocess fails
to start the child process, a dangling process_handler instance is
attached to the runner instance. This should be avoided, and a
RunnerNotStartedError has to be thrown.

Review commit: https://reviewboard.mozilla.org/r/233642/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/233642/

Comment 3

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

Comment on attachment 8964903 [details]
Bug 1451310 - [mozrunner] Catch exceptions when starting the client process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/233642/diff/1-2/

Comment 4

[Andrew Halberstadt [:ahal]]

Comment on attachment 8964903 [details]
Bug 1451310 - [mozrunner] Catch exceptions when starting the client process.

https://reviewboard.mozilla.org/r/233642/#review239418

Thanks, lgtm

Comment 5

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s c26f5209b48f725a4eac9debbe747fb062de77b7 -d dfc311c89e45: rebasing 456148:c26f5209b48f "Bug 1451310 - [mozrunner] Catch exceptions when starting the client process. r=ahal" (tip)
merging testing/mozbase/mozrunner/mozrunner/base/runner.py
warning: conflicts while merging testing/mozbase/mozrunner/mozrunner/base/runner.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 6

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

I have to wait until my patch from bug 1451246 got merged to mozilla-central to be able to rebase.

Comment 7

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

Comment on attachment 8964903 [details]
Bug 1451310 - [mozrunner] Catch exceptions when starting the client process.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/233642/diff/2-3/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c61adc663f6
[mozrunner] Catch exceptions when starting the client process. r=ahal

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/6c61adc663f6