Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Listen to SIGINT and close scripts #62

Merged
merged 1 commit into from
Mar 23, 2021
Merged

Conversation

Crevil
Copy link
Member

@Crevil Crevil commented Mar 22, 2021

Currently if a shuttle script is cancelled by sending a SIGINT signal the
process running in a script is not signalled. This leads to orphaned processes
and is especially an issue when launching web servers listening on a port that
can only by used once. Here the second run of the script will fail as the port
is already used.

This change introduces a context.Context down through the call chain and the
shell executor will listen for cancellations on this and signal the processes
accordingly. The context is cancelled when receiving SIGINT.

Module github.com/go-cmd/cmd is updated to the latest version which contains a
more stable shutdown API.

Closes #59

@Crevil Crevil requested a review from a team March 22, 2021 16:32
cmd/error.go Show resolved Hide resolved
pkg/executors/executor_test.go Show resolved Hide resolved
Currently if a shuttle script is cancelled by sending a SIGINT signal the
process running in a script is not signalled. This leads to orphaned processes
and is especially an issue when launching web servers listening on a port that
can only by used once. Here the second run of the script will fail as the port
is already used.

This change introduces a context.Context down through the call chain and the
shell executor will listen for cancellations on this and signal the processes
accordingly. The context is cancelled when receiving SIGINT.

Module github.com/go-cmd/cmd is updated to the latest version which contains a
more stable shutdown API.
@Crevil Crevil enabled auto-merge (squash) March 23, 2021 07:35
@Crevil Crevil merged commit e26da03 into master Mar 23, 2021
@Crevil Crevil deleted the fix/script-exit-signal branch March 23, 2021 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signals are not forwarded to running scripts
2 participants