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

[Andrew Vagin]

On Wed, Aug 06, 2014 at 06:35:58PM +0400, Pavel Emelyanov wrote:
> On 08/06/2014 04:55 PM, Andrew Vagin wrote:
> > 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:
> 
> What is wrong?

Sorry for my french.

The comment is wrong because ... I just prove yours comment;)
> 
> > 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
> > .
> > 
>