[CRIU] [PATCH 4/8] sig: Don't reset CHLD handler to old action, DFL it

[Andrew Vagin]

On Wed, Aug 06, 2014 at 04:24:52PM +0400, Pavel Emelyanov wrote:
> The whole idea behind this code was to stop receiving CHLD from
> restored tasks after resume. The comment about this is done for 
> scripts is wrong (we call more scripts before this).

This is wrong, because sigchld_handler() knows about scripts:

commit de71bc69170cfeceb24bddd431ad10b8ea607d42
@@ -931,6 +931,14 @@ static void sigchld_handler(int signal, siginfo_t *siginfo, void *data)
        exit = (siginfo->si_code == CLD_EXITED);
        status = siginfo->si_status;
+
+       /* skip scripts */
+       if (!current && root_item->pid.real != pid) {
+               pid = waitpid(root_item->pid.real, &status, WNOHANG);
+               if (pid <= 0)
+                       return;
+       }

Acked-by: Andrew Vagin <avagin at parallels.com>

> 
> And since CHLD handler makes little sence after exec, it's easier
> just to reset one to default action at the end.
> 
> Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
> ---
>  cr-restore.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 5aa5472..f27e1f8 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -1475,10 +1475,18 @@ detach:
>  	}
>  }
>  
> +static void ignore_kids(void)
> +{
> +	struct sigaction sa = { .sa_handler = SIG_DFL };
> +
> +	if (sigaction(SIGCHLD, &sa, NULL) < 0)
> +		pr_perror("Restoring CHLD sigaction failed");
> +}
> +
>  static int restore_root_task(struct pstree_item *init)
>  {
>  	int ret, fd;
> -	struct sigaction act, old_act;
> +	struct sigaction act;
>  
>  	fd = open("/proc", O_DIRECTORY | O_RDONLY);
>  	if (fd < 0) {
> @@ -1514,7 +1522,7 @@ static int restore_root_task(struct pstree_item *init)
>  	sigemptyset(&act.sa_mask);
>  	sigaddset(&act.sa_mask, SIGCHLD);
>  
> -	ret = sigaction(SIGCHLD, &act, &old_act);
> +	ret = sigaction(SIGCHLD, &act, NULL);
>  	if (ret < 0) {
>  		pr_perror("sigaction() failed");
>  		return -1;
> @@ -1588,13 +1596,6 @@ static int restore_root_task(struct pstree_item *init)
>  	if (ret < 0)
>  		goto out_kill;
>  
> -	/* Restore SIGCHLD here to skip SIGCHLD from a network sctip */
> -	ret = sigaction(SIGCHLD, &old_act, NULL);
> -	if (ret < 0) {
> -		pr_perror("sigaction() failed");
> -		goto out_kill;
> -	}
> -
>  	ret = run_scripts("post-restore");
>  	if (ret != 0) {
>  		pr_err("Aborting restore due to script ret code %d\n", ret);
> @@ -1607,6 +1608,12 @@ static int restore_root_task(struct pstree_item *init)
>  	network_unlock();
>  
>  	/*
> +	 * Stop getting sigchld, after we resume the tasks they
> +	 * may start to exit poking criu in vain.
> +	 */
> +	ignore_kids();
> +
> +	/*
>  	 * -------------------------------------------------------------
>  	 * Below this line nothing can fail, because network is unlocked
>  	 */
> -- 
> 1.8.4.2
> 
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu