diff mbox series

[v2] util: Flush cursor state to stdout before removing signal handler

Message ID Yk0TnKL3qShB/xyH@chrisdown.name
State Accepted, archived
Headers show
Series [v2] util: Flush cursor state to stdout before removing signal handler | expand

Commit Message

Chris Down April 6, 2022, 4:14 a.m. UTC 
It's possible that the cursor does not reappear after pressing ^C during
shutdown. In my case, I noticed this when pressing ^C after getting
results from `pacman -F` -- this can reasonably reliably be triggered by
issuing a file query and pressing ^C shortly after results are shown.

There are two reasons for this issue:

1. The graceful SIGINT handler is removed at the start of cleanup(), but
   the window from entering cleanup() to reaching exit() is non trivial.
   The main offender is FREELIST(pm_targets), which on my T14s takes
   >0.1s to execute. This means that if you are unlucky enough to press
   ^C while there, the cursor isn't coming back, because we haven't
   issued any command to show the cursor again yet, and the userspace
   signal handler is already blown away.
2. Moving console_cursor_show() to earlier in cleanup() only half solves
   the issue. While it's fine not to flush after _hiding_ the cursor,
   since it will at least make itself apparent before any other text
   reaches the screen, _showing_ the cursor must be followed by flushing
   stdout, because once the graceful SIGINT handler is gone, if you
   press ^C, no flush will be triggered (and thus there will be no
   cursor).

This fixes the issue by always starting out by showing the cursor again
at cleanup() time. This means that no matter where we get caught at ^C,
we will not end up leaving the terminal without its beloved ensign.

Signed-off-by: Chris Down <chris@chrisdown.name>
---
 src/pacman/pacman.c |  2 +-
 src/pacman/util.c   | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

Comments

Chris Down May 23, 2022, 6:28 a.m. UTC | #1 
Hey folks,

Just following up on this patch -- any thoughts about this one?

Thanks,

Chris
Chris Down July 21, 2022, 1:20 p.m. UTC | #2 
Since I see some activity just checking in again -- let me know if you want me 
to rebase this one or if there are any problems with it. Thanks!
Allan McRae July 22, 2022, 12:21 a.m. UTC | #3 
On 6/4/22 14:14, Chris Down wrote:
> It's possible that the cursor does not reappear after pressing ^C during
> shutdown. In my case, I noticed this when pressing ^C after getting
> results from `pacman -F` -- this can reasonably reliably be triggered by
> issuing a file query and pressing ^C shortly after results are shown.
> 
> There are two reasons for this issue:
> 
> 1. The graceful SIGINT handler is removed at the start of cleanup(), but
>     the window from entering cleanup() to reaching exit() is non trivial.
>     The main offender is FREELIST(pm_targets), which on my T14s takes
>     >0.1s to execute. This means that if you are unlucky enough to press
>     ^C while there, the cursor isn't coming back, because we haven't
>     issued any command to show the cursor again yet, and the userspace
>     signal handler is already blown away.
> 2. Moving console_cursor_show() to earlier in cleanup() only half solves
>     the issue. While it's fine not to flush after _hiding_ the cursor,
>     since it will at least make itself apparent before any other text
>     reaches the screen, _showing_ the cursor must be followed by flushing
>     stdout, because once the graceful SIGINT handler is gone, if you
>     press ^C, no flush will be triggered (and thus there will be no
>     cursor).
> 
> This fixes the issue by always starting out by showing the cursor again
> at cleanup() time. This means that no matter where we get caught at ^C,
> we will not end up leaving the terminal without its beloved ensign.
> 
> Signed-off-by: Chris Down <chris@chrisdown.name>
> ---

Applied.

Thanks,
Allan
Chris Down July 22, 2022, 1:07 p.m. UTC | #4 
Allan McRae writes:
>Applied.

Thanks! Appreciate you looking at this :-)
diff mbox series

Patch

diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index ea536a47..e5c6e420 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -289,6 +289,7 @@  static void setuseragent(void)
  */
 static void cleanup(int ret)
 {
+	console_cursor_show();
 	remove_soft_interrupt_handler();
 	if(config) {
 		/* free alpm library resources */
@@ -302,7 +303,6 @@  static void cleanup(int ret)
 
 	/* free memory */
 	FREELIST(pm_targets);
-	console_cursor_show();
 	exit(ret);
 }
 
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 53833d6f..e92b2012 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -1426,6 +1426,16 @@  void console_cursor_hide(void) {
 void console_cursor_show(void) {
 	if(isatty(fileno(stdout))) {
 		printf(CURSOR_SHOW_ANSICODE);
+
+		/* We typically explicitly show the cursor either when we are
+		 * getting input from stdin, or when we are in the process of
+		 * exiting. In the former case, it's not guaranteed that the
+		 * terminal will see the command before reading from stdin. In
+		 * the latter case, we need to make sure that if we get a
+		 * further TERM/INT after we return signal disposition to
+		 * SIG_DFL, it doesn't leave the cursor invisible.
+		 */
+		fflush(stdout);
 	}
 }