Bug 1864413: don't report success for tasks killed by a signal#626
Bug 1864413: don't report success for tasks killed by a signal#626jcristau merged 1 commit intomozilla-releng:mainfrom
Conversation
f20343d to
56993b0
Compare
f8f9144 to
40ef14c
Compare
| import os | ||
| import signal | ||
|
|
||
| os.kill(0, signal.SIGKILL) |
There was a problem hiding this comment.
I'm curious, wouldn't this kill the process on import? How does this work?
There was a problem hiding this comment.
It's not imported, it's executed by the test (we point task_script at it).
| if exitcode < 0: | ||
| status_line = "Automation Error: python exited with signal {}".format(exitcode) | ||
| # we must return a value > 0 to signal an error | ||
| exitcode = 1 |
There was a problem hiding this comment.
As far as I can tell this bug was introduced by #480, where we started returning exitcode directly instead of 0/1. Looking at that change it seems the intent might be to return 256 + exitcode instead, maybe, so that the signal number can be looked up in reversed_statuses (with 241 and 245 for SIGTERM and SIGSEGV)?
There was a problem hiding this comment.
After some more digging, the context in bug 1654250 is that we wanted to propagate "some tool (i.e. subprocess) called by iscript segfaults" into "task status is intermittent-failure". I'm not convinced that ever worked, but in any case it should be fine, when the task script itself (not a command it ran) is killed by a signal, to return 1 (failure).
| import os | ||
| import signal | ||
|
|
||
| os.kill(0, signal.SIGKILL) |
There was a problem hiding this comment.
It's not imported, it's executed by the test (we point task_script at it).
`run_task`, and thus `do_run_task`, were using the (negative) exit code from the task process as status. That status was then combined with the 0 from `do_upload` in `RunTasks.invoke` via `worst_level`, and ended up getting swallowed. Replace that with a failure code.
40ef14c to
86e43cb
Compare
run_task, and thusdo_run_task, were using the (negative) exit code from the task process as status. That status was then combined with the 0 fromdo_uploadinRunTasks.invokeviaworst_level, and ended up getting swallowed. Replace that with a failure code.