Importing pterm causes program to not exit on SIGINT and SIGTERM

[adombeck]

The signal handlers for os.Interrupt and syscall.SIGTERM registered in the init function prevent Go's default behavior when receiving such a signal (exiting with an exit code which depends on the OS and/or signal):

pterm/pterm.go

Lines 31 to 43 in e376aa8

	func init() {
	color.ForceColor()
	// Make the cursor visible when the program stops
	c := make(chan os.Signal, 1)
	signal.Notify(c, os.Interrupt)
	signal.Notify(c, syscall.SIGTERM)
	go func() {
	for range c {
	cursor.Show()
	}
	}()
	}

In effect, any program which merely imports pterm does not exit when handling a SIGINT (e.g. by pressing Ctrl+C) or a SIGTERM (e.g. by calling kill $PID).

[adombeck]

This could be fixed by re-raising the signal in the init function after handling it:

diff --git a/pterm.go b/pterm.go
index 09557157..82397ed0 100644
--- a/pterm.go
+++ b/pterm.go
@@ -7,11 +7,13 @@
 package pterm
 
 import (
	"atomicgo.dev/cursor"
	"github.com/gookit/color"
+	"fmt"
 	"os"
 	"os/signal"
 	"syscall"
 )
 
 var (
@@ -36,8 +38,21 @@ func init() {
 	signal.Notify(c, os.Interrupt)
 	signal.Notify(c, syscall.SIGTERM)
 	go func() {
-               for range c {
+               for s := range c {
                        cursor.Show()
+
+                       // Re-raise the signal to trigger the default behavior
+                       signal.Stop(c)
+                       p, err := os.FindProcess(os.Getpid())
+                       if err != nil {
+                               _, _ = fmt.Fprintf(os.Stderr, "pterm: Failed to re-raise %s: %v\n", s, err)
+                               continue
+                       }
+                       err = p.Signal(s)
+                       if err != nil {
+                               _, _ = fmt.Fprintf(os.Stderr, "pterm: Failed to re-raise %s: %v\n", s, err)
+                               continue
+                       }
                }
        }()
 }

 }

However, this could lead to other unexpected behavior: Any other handlers registered by the program via signal.Notify would receive the same signal twice, because the Go runtime sends each signal to all handlers registered for that signal.

The only clean solution I see is to not register any signal handler in pterm and instead provide methods which the consumer can call themself to ensure that the cursor is restored. The ProgressbarPrinter already provides a Stop() method which can be used for that, for example like this:

func DownloadWithProgressbar() {
	p := pterm.DefaultProgressbar.WithTotal(len(fakeInstallList)).WithTitle("Downloading stuff")

	// Ensure that the progressbar printer is stopped when receiving a
	// terminating signal
	sigs := make(chan os.Signal, 1)
	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT)
	go func() {
		<-sigs
		p.Stop()
		os.Exit(1)
	}()
	defer signal.Stop(sigs)

	p.Start()

	for i := 0; i < p.Total; i++ {
		if i == 6 {
			time.Sleep(time.Second * 3) // Simulate a slow download.
		}
		p.UpdateTitle("Downloading " + fakeInstallList[i])         // Update the title of the progressbar.
		pterm.Success.Println("Downloading " + fakeInstallList[i]) // If a progressbar is running, each print will be printed above the progressbar.
		p.Increment()                                              // Increment the progressbar by one. Use Add(x int) to increment by a custom amount.
		time.Sleep(time.Millisecond * 350)                         // Sleep 350 milliseconds.
	}
}

The other cases where cursor.Hide is currently called in pterm are InteractiveSelectPrinter.Show and InteractiveMultiselectPrinter.Show. Those do not yet provide methods to clean up the terminal.

[MarvinJWendt]

Hi @adombeck, I think this was closed by accident (as a side-effect of closing another issue, that referenced this one). If the issue still persists, you can re-open this one :)

[adombeck]

Thanks, this was indeed closed by accident.

[MarvinJWendt]

Hi, I will definitely look into this.

The only clean solution I see is to not register any signal handler in pterm and instead provide methods which the consumer can call themself to ensure that the cursor is restored. The ProgressbarPrinter already provides a Stop() method which can be used for that, for example like this:

The problem with this is that if a user kills the process in the middle of a progressbar (or similar printer), then the Stop() method is not executed. This causes the cursor to remain hidden until the terminal is restarted.

Your example shows, how a user would have to implement the signal handling by himself, but I think this is not in favor of PTerms simplicity philosophy. Every PTerm printer should be usable without any extra setup.

[adombeck]

How about registering the signal handler in ProgressbarPrinter.Start (and InteractiveSelectPrinter.Show / InteractiveMultiselectPrinter.Show) instead of init and making that optional via ProgressbarPrinter.WithoutSignalHandling or something like that.

[MarvinJWendt]

I removed the whole signal handling in #570. I think PTerm should not mess with signal handling. While this could cause that the cursor disappears, when crtl+c is pressed while the cursor is hidden, I still think it's the better option. There would be no optimal solution, if we handle signals inside PTerm. There would always be an edge-case, where a user wants to handle the signal by himself.

Appending to that, many terminals unhide the cursor, as soon as the user starts typing.

Thanks for pointing it out, and sorry for the long wait.

[MarvinJWendt]

Released in v0.12.68