One of the things I like to add to these sort of things is the ability to force kill it by repeatedly sending signals; otherwise, a hanging shutdown process is a real pain.
For example:
^C 23:36:43 INFO: Waiting for background tasks to finish; send HUP, TERM, or INT twice to force kill (may lose data!)
1 tasks: test-task ^COne more to kill…
1 tasks: test-task ^CForce killing
It also shows which tasks are running, which is kinda nice.
I’ve been meaning to clean this up and extract it to a package for ages. Either way, just some ideas for future improvements to your package :-)
One of the nice things about triggering shutdown on a certain rate of signal delivery as that you also get a small amount of protection from accidental kills.
Sigint/sigterm won’t run your deferred functions, the program just terminates - I had this issue with missing spans from an opentelemetry client. I just had to add the signal handler and call .Close() to solve that.
I understand this library, but in general I won’t bother with a dependency for something as trivial as handling a os signal.
Cool. It’s been a couple years since I dealt with the problem, but I’m not sure that this works 100% on windows. Things might have changed, but I recall the signal handling logic didn’t map correctly (and I needed the golang/x/sys/windows). Does that sound right or have things improved to the point this package doesn’t need to worry about those details?
for {
// Check if context canceled or ShutdownFunc finished
select {
case <-c.Done():
// Context canceled, return
return
default:
if done {
// ShutdownFunc finished, return
return
}
// Otherwise continue
}
looks like a spin wait and will max out a core until the function is done or the context is canceled. Or am I reading it wrongly? I try to avoid these, even if they are unlikely to last very long. e.g. a web server being attacked slowloris style might take a long time to shutdown and this spin wait would waste a lot of CPU resources on a server that might be starved anyway because of the attack. Maybe better to not have a default in the select and to use a channel instead of done bool.
One of the things I like to add to these sort of things is the ability to force kill it by repeatedly sending signals; otherwise, a hanging shutdown process is a real pain.
For example:
It also shows which tasks are running, which is kinda nice.
I’ve been meaning to clean this up and extract it to a package for ages. Either way, just some ideas for future improvements to your package :-)
One of the nice things about triggering shutdown on a certain rate of signal delivery as that you also get a small amount of protection from accidental kills.
Thanks for the suggestion 😊
@arp242 Done!
I thought this was the point of
defer
?Sigint/sigterm won’t run your deferred functions, the program just terminates - I had this issue with missing spans from an opentelemetry client. I just had to add the signal handler and call
.Close()
to solve that.I understand this library, but in general I won’t bother with a dependency for something as trivial as handling a os signal.
Cool. It’s been a couple years since I dealt with the problem, but I’m not sure that this works 100% on windows. Things might have changed, but I recall the signal handling logic didn’t map correctly (and I needed the golang/x/sys/windows). Does that sound right or have things improved to the point this package doesn’t need to worry about those details?
looks like a spin wait and will max out a core until the function is done or the context is canceled. Or am I reading it wrongly? I try to avoid these, even if they are unlikely to last very long. e.g. a web server being attacked slowloris style might take a long time to shutdown and this spin wait would waste a lot of CPU resources on a server that might be starved anyway because of the attack. Maybe better to not have a default in the select and to use a channel instead of done bool.
Should I add a time.sleep() with a few milliseconds maybe? 🤔
Feel free to send me a better solution 😊
Here is a diff:
Thank you! I’ll merge that 😊