Closed nathanwoctopusdeploy closed 1 year ago
[sc-57128]
This pull request has been linked to Shortcut Story #57128: Make the compat binary terminate after a certain amount of time..
I think it might make sense to re-write this change.
Re-write may be a little strong here!! Is the concept completely wrong, or just a few things need tweaking!!
Right now if the CT is cancelled which is passed to the
StayAliveUntilHelper
then that helper will stop detecting if the parent dies. We probably never want that.
Were cancelling to stop all the things and then exit. What use would the StayAliveUntilHelper
be here, were on the exit path anyway.
The existing
StayAliveUntilHelper
also just callsEnvironment.Exit
which kills the process immediately. It might make sense to do the same here rather than return an exit code just terminate the process.
Is there any difference given the implementation? This PR tries to cancel all the things and then exit. It seems to work when I tested it locally - hard coding a very short timeout for the binary and ensuring it had shut down unexpectedly during the test run.
Yep returning in main will wait for some things to finish up before terminating. So Environment.Exit
would make sense.
I think the change would be to not pass a CT to the stayalivehelper
and in the top main method if the When.Any
returns and it is the timeout then just call Environment.Exit
since we want it to stop rather than wait around for things to finish. Re-write is the wrong word, slightly modify :p
Making those changes means this change is an additional way of exiting which doesn't degrade the existing termination logic.
Background
Ensure the CompatBinary does not stay running for longer than the test timeout
How to review this PR
Quality :heavy_check_mark:
Pre-requisites