go-shutdowner: Simple graceful shutdowns in Go

5

go-shutdowner: Simple graceful shutdowns in Go go jlelse.blog
authored by jlelse 1 year ago | archive
Archive.org Archive.today Ghostarchive
| 11 comments

11

9
arp242 1 year ago | link
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 :-)
1. 1
  
  animatronic 1 year ago | link
  
  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.
2. 1
  
  jlelse 1 year ago | link
  
  Thanks for the suggestion 😊
  1. 1
    
    jlelse 1 year ago | link
    
    @arp242 Done!
3

WilhelmVonWeiner 1 year ago | link

I thought this was the point of defer?
1. 6
  
  pondidum 1 year ago | link
  
  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.
2

msingle 1 year ago | link

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?

jrwren 1 year ago | link

	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.

jlelse 1 year ago | link

Should I add a time.sleep() with a few milliseconds maybe? 🤔

Feel free to send me a better solution 😊

jrwren 11 months ago | link

Here is a diff:

diff --git a/shutdown.go b/shutdown.go
index 2d02836..35d87c9 100644
--- a/shutdown.go
+++ b/shutdown.go
@@ -29,25 +29,20 @@ type ShutdownFunc func()
 
 // Internal method
 func (f ShutdownFunc) execute(c context.Context) {
-       done := false
+       done := make(chan struct{})
        // Execute ShutdownFunc in goroutine and set done = true
        go func() {
                f()
-               done = true
+               close(done)
        }()
-       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
-               }
+       // Check if context canceled or ShutdownFunc finished
+       select {
+       case <-c.Done():
+               // Context canceled, return
+               return
+       case <- done:
+               // ShutdownFunc finished, return
+               return
        }
 }

2

jlelse 11 months ago | link

Thank you! I’ll merge that 😊