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

Kill processes instead of interrupting them #97

Closed
wants to merge 2 commits into from
Closed

Kill processes instead of interrupting them #97

wants to merge 2 commits into from

Conversation

Fruchtzwerg94
Copy link
Contributor

@Fruchtzwerg94 Fruchtzwerg94 commented Oct 5, 2020

Kill custom commands instead of just interrupting them.
Why?:

  • Interrupting not supported on Windows
  • Libraries like VLC are not interrupting gracefully always. If a connection gets closed, the interrupt takes too much time to be executed until a potential new connection gets established. This leads to two processes of the same type are running, which can cause resource problems (like audio outout, ports, ...).

Just killing the processes is not the fine way, but more stable / reliable in this case.

@aler9
Copy link
Member

aler9 commented Oct 8, 2020

Hello, thanks for reporting the issue. I finally got the time to turn on a Windows partition, which i never use, wait for the automatic updates and check the issue.

I didn't noticed that Go doesn't support sending SIGINT to Windows processes, probably SIGINT doesn't even exist in Windows, i don't remember since the last time i wrote something native for Windows was at eleven years old with Visual Studio 2003 (then it was only Cygwin and Python). Therefore, Kill() must be used on Windows.

At the same time, SIGINT is very handy in Linux, since it allows to write scripts that can handle both the start and the end of an event; for instance, this script

#!/bin/sh

trap 'echo stopped' SIGINT

echo started

while true; do sleep 86400; done

Will print started when it is started, and ended when it is closing.

If you need to kill a process on Linux too, it's possible to use a wrapper like this:

#!/bin/sh

otherprogram &
P=$?

trap 'kill -9 $P' SIGINT

wait

So:

  • i'll switch to Kill() on Windows
  • i'll keep the current behavior on Unix

aler9 added a commit that referenced this pull request Oct 8, 2020
aler9 added a commit that referenced this pull request Oct 8, 2020
@Fruchtzwerg94
Copy link
Contributor Author

Fruchtzwerg94 commented Oct 9, 2020

Sounds good for me, thanks for your response. So I will close the pull-request - okay?

Update:
I've ported your script to a command, which can be directly defined in the configuration file. After fixing some small problems with the script (Use $! instead of $?, INT instead of SIGINT and wait $pid), the following command does the job:

runOnPublish: otherprogramm & pid=$! ; trap 'echo KILL ; kill -9 $pid' INT ; echo WAIT $pid ; wait $pid ; echo DONE

Thanks for your hint with the trap.

@aler9 aler9 force-pushed the master branch 3 times, most recently from b389dd7 to 618f891 Compare October 13, 2020 22:54
@aler9
Copy link
Member

aler9 commented Oct 17, 2020

feature added in v0.10.1

@aler9 aler9 closed this Oct 17, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Jan 1, 2023

This issue is being locked automatically because it has been closed for more than 6 months.
Please open a new issue in case you encounter a similar problem.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants