handle signals #4740
handle signals #4740
Conversation
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
There was a problem hiding this comment.
TL;DR: Confirming it works as expected, even on Windows. I feel comfortable merging as-is in the spirit of getting v1 in time.
Can confirm this works as expected, even on Windows. I think we could test it more thoroughly, especially on Windows, and I agree with the TODO comment that we should probably have a timeout forcing the process group to exit in case it takes too long or never does (considering OOMs and 100% CPU tests are still a thing, it might be a potential issue with signals never arriving).
I'll open an issue to track those topics, and make sure it's on the map for the next release.
internal/cmd/launcher.go
Outdated
| case <-sigC: | ||
| // TODO: maybe we should set a timeout while waiting for the subprocess | ||
| gs.Logger.Debug("Signal received. Waiting for subprocess to end.") |
There was a problem hiding this comment.
| case <-sigC: | |
| // TODO: maybe we should set a timeout while waiting for the subprocess | |
| gs.Logger.Debug("Signal received. Waiting for subprocess to end.") | |
| case sig := <-sigC: | |
| gs.Logger.WithField("signal", sig.String()).Debug("Signal received, waiting for the subprocess to handle it and return.") |
internal/cmd/launcher.go
Outdated
| } | ||
| gs.OSExit(rc) | ||
| case <-sigC: | ||
| // TODO: maybe we should set a timeout while waiting for the subprocess |
There was a problem hiding this comment.
| // TODO: maybe we should set a timeout while waiting for the subprocess |
I want to centralize this logic to k6 as much as possible, so the signal handling should be fully delegated to k6.
Except if we see a good reason to force the exit here.
Signed-off-by: Pablo Chacin <pablochacin@gmail.com>
What?
Handle signals in the parent process when calling a subprocess.
Why?
Prevent the parent from ending prematurely before the subprocess ends.
Also fixes #4735. By separating the
Startof the process from waiting for the child to end, we can handle separately errors like binary not found (and report them) from the non-zero return codes from the child.Checklist
make check) and all pass.Checklist: Documentation (only for k6 maintainers and if relevant)
Please do not merge this PR until the following items are filled out.
Related PR(s)/Issue(s)