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

[Pavel Emelyanov]

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?

> 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
> .
>