[pacman-dev,1/1] signhandler: add SIGQUIT soft_interrupt_handler

Message ID 20210407104711.510374-2-adlternative@gmail.com
State Rejected, archived
Headers show
Series signhandler add SIGQUIT soft_interrupt_handler | expand

Commit Message

ZheNing Hu April 7, 2021, 10:47 a.m. UTC
When we downloading with pacman. If we want to
stop downloading, use `Ctrl +C` can exit normally,
and "Interrupt signal received" will be print
on the terminal. But if we accidentally use
`Ctrl + \` to exit the download, it's not quite
so perfect. The program exits with a segment
fault, the database lock file was not deleted
in time. The next time we use pacman downloading,
the error will appear.

So we can add signal handler with SIGQUIT, just
like we did with SIGINT and SIGHUP. This way the
user doesn't have to delete lock file manually.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 src/pacman/sighandler.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Allan McRae May 9, 2021, 12:08 p.m. UTC | #1
On 7/4/21 8:47 pm, ZheNing Hu wrote:
> When we downloading with pacman. If we want to
> stop downloading, use `Ctrl +C` can exit normally,
> and "Interrupt signal received" will be print
> on the terminal. But if we accidentally use
> `Ctrl + \` to exit the download, it's not quite
> so perfect. The program exits with a segment
> fault, the database lock file was not deleted
> in time. The next time we use pacman downloading,
> the error will appear.
> 
> So we can add signal handler with SIGQUIT, just
> like we did with SIGINT and SIGHUP. This way the
> user doesn't have to delete lock file manually.
> 
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---

From the glibc manual:

"Certain kinds of cleanups are best omitted in handling SIGQUIT. For
example, if the program creates temporary files, it should handle the
other termination requests by deleting the temporary files. But it is
better for SIGQUIT not to delete them, so that the user can examine them
in conjunction with the core dump. "

https://www.gnu.org/software/libc/manual/html_node/Termination-Signals.html


>  src/pacman/sighandler.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c
> index ff54dce8..9fdbf917 100644
> --- a/src/pacman/sighandler.c
> +++ b/src/pacman/sighandler.c
> @@ -81,15 +81,18 @@ void install_soft_interrupt_handler(void)
>  	new_action.sa_flags = SA_RESTART;
>  	sigemptyset(&new_action.sa_mask);
>  	sigaddset(&new_action.sa_mask, SIGINT);
> +	sigaddset(&new_action.sa_mask, SIGQUIT);
>  	sigaddset(&new_action.sa_mask, SIGHUP);
>  
>  	sigaction(SIGINT, &new_action, NULL);
> +	sigaction(SIGQUIT, &new_action, NULL);
>  	sigaction(SIGHUP, &new_action, NULL);
>  }
>  
>  void remove_soft_interrupt_handler(void)
>  {
>  	_reset_handler(SIGINT);
> +	_reset_handler(SIGQUIT);
>  	_reset_handler(SIGHUP);
>  }
>  
>
Eli Schwartz May 9, 2021, 12:10 p.m. UTC | #2
On 4/7/21 6:47 AM, ZheNing Hu wrote:
> When we downloading with pacman. If we want to
> stop downloading, use `Ctrl +C` can exit normally,
> and "Interrupt signal received" will be print
> on the terminal. But if we accidentally use
> `Ctrl + \` to exit the download, it's not quite
> so perfect. The program exits with a segment
> fault, the database lock file was not deleted
> in time. The next time we use pacman downloading,
> the error will appear.
> 
> So we can add signal handler with SIGQUIT, just
> like we did with SIGINT and SIGHUP. This way the
> user doesn't have to delete lock file manually.

The entire point of SIGQUIT is to let users kill a program in such a way
that it produces a coredump. "You can think of this as a program error
condition 'detected' by the user."

I'm curious how one might accidentally do this.

> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>  src/pacman/sighandler.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c
> index ff54dce8..9fdbf917 100644
> --- a/src/pacman/sighandler.c
> +++ b/src/pacman/sighandler.c
> @@ -81,15 +81,18 @@ void install_soft_interrupt_handler(void)
>  	new_action.sa_flags = SA_RESTART;
>  	sigemptyset(&new_action.sa_mask);
>  	sigaddset(&new_action.sa_mask, SIGINT);
> +	sigaddset(&new_action.sa_mask, SIGQUIT);
>  	sigaddset(&new_action.sa_mask, SIGHUP);
>  
>  	sigaction(SIGINT, &new_action, NULL);
> +	sigaction(SIGQUIT, &new_action, NULL);
>  	sigaction(SIGHUP, &new_action, NULL);
>  }
>  
>  void remove_soft_interrupt_handler(void)
>  {
>  	_reset_handler(SIGINT);
> +	_reset_handler(SIGQUIT);
>  	_reset_handler(SIGHUP);
>  }
>  
>

Patch

diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c
index ff54dce8..9fdbf917 100644
--- a/src/pacman/sighandler.c
+++ b/src/pacman/sighandler.c
@@ -81,15 +81,18 @@  void install_soft_interrupt_handler(void)
 	new_action.sa_flags = SA_RESTART;
 	sigemptyset(&new_action.sa_mask);
 	sigaddset(&new_action.sa_mask, SIGINT);
+	sigaddset(&new_action.sa_mask, SIGQUIT);
 	sigaddset(&new_action.sa_mask, SIGHUP);
 
 	sigaction(SIGINT, &new_action, NULL);
+	sigaction(SIGQUIT, &new_action, NULL);
 	sigaction(SIGHUP, &new_action, NULL);
 }
 
 void remove_soft_interrupt_handler(void)
 {
 	_reset_handler(SIGINT);
+	_reset_handler(SIGQUIT);
 	_reset_handler(SIGHUP);
 }