fix: edge-case where cancelled context causes process to hang (#60)

* fix: cancelled context now exits the process with error code

* fix: remove listening for <-ctx.Done in root.go

* docs: tweak wording of cancel context comments

Author: mwbrooks

cmd/root.go

@@ -327,20 +327,28 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob
 // It listens for process interrupts and sends to IOStreams' GetInterruptChannel() for use in
 // in communicating process interrupts elsewhere in the code.
 func ExecuteContext(ctx context.Context, rootCmd *cobra.Command, clients *shared.ClientFactory) {
+	// Derive a cancel context that is cancelled when the main execution is interrupted or cleaned up.
+	// Sub-commands can register for the cleanup wait group with clients.CleanupWaitGroup.Add(1)
+	// and listen for <-ctx.Done() to be notified when the main execution is interrupted, in order
+	// to have a chance to cleanup. This is useful for long running processes and background goroutines,
+	// such as the activity and upgrade commands.
 	ctx, cancel := context.WithCancel(ctx)
 
 	completedChan := make(chan bool, 1)      // completed is used for signalling an end to command
 	exitChan := make(chan bool, 1)           // exit blocks the command from exiting until completed
 	interruptChan := make(chan os.Signal, 1) // interrupt catches signals to avoid abrupt exits
+
 	signal.Notify(interruptChan, os.Interrupt, syscall.SIGTERM)
 	defer func() {
 		signal.Stop(interruptChan)
 		cancel()
 	}()
+
+	// Wait for either an interrupt signal (ctrl+c) or an explicit call to ctx.cancel()
 	go func() {
-		// Wait for either an interrupt signal (ctrl+c), or an explicit call to ctx.cancel()
 		select {
-		case <-interruptChan: // first interrupt signal, cancel context
+		// Received interrupt signal, so cancel the context and cleanup
+		case <-interruptChan:
 			clients.IO.PrintInfo(ctx, false, "\n") // flush the CTRL + C character
 			clients.IO.PrintDebug(ctx, "Got process interrupt signal, cancelling context")
 			cancel()
@@ -355,18 +363,17 @@ func ExecuteContext(ctx context.Context, rootCmd *cobra.Command, clients *shared
 				clients.IO.PrintDebug(ctx, "Exiting with cancel exit code.")
 				os.Exit(int(iostreams.ExitCancel))
 			}()
-		case <-ctx.Done():
-			// No interrupt signal sent, command executed to completion
-			// FIXME - `.Done()` channel is triggered by `cancel()` and not a successfully completion
+		// Received completed execution, so exit the process successfully
 		case <-completedChan:
-			// No canceled context, but command has completed execution
 			exitChan <- true
 		}
+
 		// If we get a second interrupt, no matter what exit the process
 		<-interruptChan
 		clients.IO.PrintDebug(ctx, "Got second process interrupt signal, exiting the process")
 		os.Exit(int(iostreams.ExitCancel))
 	}()
+
 	// The cleanup() method in the root command will invoke via `defer` from within Execute.
 	if err := rootCmd.ExecuteContext(ctx); err != nil {
 		errMsg := err.Error()
@@ -389,6 +396,7 @@ func ExecuteContext(ctx context.Context, rootCmd *cobra.Command, clients *shared
 	} else {
 		completedChan <- true
 	}
+
 	<-exitChan
 	_ = clients.EventTracker.FlushToLogstash(ctx, clients.Config, clients.IO, clients.IO.GetExitCode())
 }