The general stopping framework has been improved as a side-effect of the restructuring have to do with manifests: https://github.com/mozautomation/mozmill/commit/5a58949f9628d7ebbcbe64ede2674c2100a8ea0a This should be backported to 1.5.2 to improve its robustness as well.
Created attachment 497969 [details] [diff] [review] patch to improve things I'm not convinced this is 100%, even ignoring the SIGINT vs SIGKILL in killableprocess. Its a good port from master, but the control flow has changed a lot
Attachment #497969 - Flags: review?(fayearthur+bugs)
The patch doesn't help for bug 615662, but fixes the problem reported on bug 619110.
I'm not sure where our logic fails in 1.5.2 for stopping. As a hacky fix, I would suggest putting in a SIGKILL if the process hasn't shut down (which *may*? fix bug 615662). The control flow is much simpler on master: https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/__init__.py#L347 I'm not sure if this is an issue there. If it is, we should make mozrunner.cleanup more robust.
Comment on attachment 497969 [details] [diff] [review] patch to improve things Definitely looks like an improvement, thanks.
Attachment #497969 - Flags: review?(fayearthur+bugs) → review+
:ctalbert, :whimboo: okay to mozmill-1.5.2+ so I can land?
(In reply to comment #5) > :ctalbert, :whimboo: okay to mozmill-1.5.2+ so I can land? yes, of course.
Whiteboard: [mozmill-1.5.2?] → [mozmill-1.5.2+]
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.