1. 5
  1.  

  2. 9

    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

      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.

      1. 1

        Thanks for the suggestion 😊

        1. 1

          @arp242 Done!

      2. 3

        I thought this was the point of defer?

        1. 6

          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. 2

          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?

          1. 1
            	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.

            1. 1

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

              Feel free to send me a better solution 😊

              1. 2

                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
                        }
                 }
                
                1. 2

                  Thank you! I’ll merge that 😊