run_signscript should never return success for a killed worker

RESOLVED FIXED

Status

RESOLVED FIXED
7 years ago
6 months ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [signing])

Attachments

(1 attachment)

We currently rely on proc.poll()'s return value to decide what we return. However, workers that don't handle SIGINT well or otherwise return 0 when killed can result in unwanted behaviour.
Created attachment 626296 [details] [diff] [review]
Set error rc when child process is killed

Set a flag when the timer runs out. If the flag is set, we should ignore the child's return code and use -1 instead.

There may be a more elegant way to refactor the block and avoid using the flag, but it's not coming to me right now.
Attachment #626296 - Flags: feedback?(bhearsum)
Comment on attachment 626296 [details] [diff] [review]
Set error rc when child process is killed

Will try to look at this soon, Catlee should probably look, too.
Attachment #626296 - Flags: feedback?(catlee)

Updated

7 years ago
Attachment #626296 - Flags: feedback?(catlee) → feedback+

Updated

7 years ago
Assignee: nobody → bhearsum
Whiteboard: [signing]
(Assignee)

Updated

7 years ago
Assignee: bhearsum → edransch.contact
(Assignee)

Updated

6 years ago
Attachment #626296 - Flags: feedback?(bhearsum) → review+
Looks like this just needs to be checked in. I'll see if I can do that later this week.
Assignee: edransch.contact → nobody
(Assignee)

Updated

6 years ago
Assignee: nobody → bhearsum
Comment on attachment 626296 [details] [diff] [review]
Set error rc when child process is killed

This patch applied cleanly to lib/python/signing/server.py and passes all tests. It looks sane and safe still, so I went ahead and landed it. Planning to restart the signing servers today.
Attachment #626296 - Flags: checked-in+
All signing servers have been updated for this change.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Component: General Automation → General
Product: Release Engineering → Release Engineering
You need to log in before you can comment on or make changes to this bug.