1199184 - Hitting Ctrl-C for stopping firefox_ui_updates script does not sanely shutdown Firefox

Status: RESOLVED WORKSFORME

Description

[Henrik Skupin (:whimboo)]

I have seen this today during testing the script. Whenever you hit Ctrl-C while the tests are running, the mozharness script exists immediately and keeps the Firefox instance running. As a result follow-up tests cannot be run because the port for Marionette is still in use.

We have to make sure that we always shutdown the sanely.

Comment 1

[Armen [:armenzg]]

We can use the PostScriptRun decorator to deal with this:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/androidx86_emulator_unittest.py#374

There is also _post_fatal() and it will do what you want before finishing the script if self.fatal() is called:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/android_emulator_build.py?offset=0#932
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/log.py?offset=0#582

Comment 2

[Henrik Skupin (:whimboo)]

I think the problem here lays in mozprocess (ProcessHandler) which uses POpen to start and control processes. Sending the control character Ctrl+C will kill the started process without giving it the time to clean-up internally. If we could make sure that ProcessHandler can delay that a bit and tell the started process to shutdown and clean-up all would be fine. Lemme see if I can find something.

Comment 3

[Henrik Skupin (:whimboo)]

We are blocked on Marionette here. A ctrl-c in mozharness will cause a SIGTERM/SIGINT for the firefox-ui-update subprocess. That's not handled correctly.

Comment 4

[Henrik Skupin (:whimboo)]

It's actually not a hard-blocker but we should try to get this fixed. I pinged Ted via email again and hope to get a reply from him on the other bug soonish.

Comment 5

[Henrik Skupin (:whimboo)]

Maybe something has been changed in mozharness base scripts but the problem does no longer occur for me on my Linux and OS X machine. I would close this bug as WFM for now, and keep the option to reopen in case I see it again.