util: block SIGCHLD to run a sub process

Submitted by Andrei Vagin on March 30, 2017, 5:48 a.m.

Details

Message ID 1490852901-19902-1-git-send-email-avagin@openvz.org
State New
Series "util: block SIGCHLD to run a sub process"
Headers show

Commit Message

Andrei Vagin March 30, 2017, 5:48 a.m.
From: Andrei Vagin <avagin@virtuozzo.com>

CRIU sets a sigchld handler and calls waitpid from it,
but when we call a sub-process, we want to wait it

Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
---
 criu/util.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/util.c b/criu/util.c
index 558d498..c014a5d 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -1369,6 +1369,7 @@  int open_fd_of_real_pid(pid_t pid, int fd, int flags)
 
 int call_in_child_process(int (*fn)(void *), void *arg)
 {
+	sigset_t blockmask, oldmask;
 	int size, status, ret = -1;
 	char *stack;
 	pid_t pid;
@@ -1379,14 +1380,27 @@  int call_in_child_process(int (*fn)(void *), void *arg)
 		pr_perror("Can't allocate stack");
 		return -1;
 	}
+
+	sigemptyset(&blockmask);
+	sigaddset(&blockmask, SIGCHLD);
+
+	if (sigprocmask(SIG_BLOCK, &blockmask, &oldmask) == -1) {
+		pr_perror("Can not set mask of blocked signals");
+		goto out_munmap;
+	}
+
 	pid = clone(fn, stack + size, CLONE_VM | CLONE_FILES | SIGCHLD, arg);
 	if (pid == -1) {
 		pr_perror("Can't clone");
-		goto out_munmap;
+		goto out_unblock;
 	}
 	errno = 0;
-	if (waitpid(pid, &status, 0) != pid || !WIFEXITED(status) || WEXITSTATUS(status)) {
-		pr_err("Can't wait or bad status: errno=%d, status=%d", errno, status);
+	if (waitpid(pid, &status, 0) != pid) {
+		pr_perror("Unable to wait %d", pid);
+		goto out_close;
+	}
+	if ( !WIFEXITED(status) || WEXITSTATUS(status)) {
+		pr_err("Can't wait or bad status: errno=%d (%s), status=%d", errno, strerror(errno), status);
 		goto out_close;
 	}
 	ret = 0;
@@ -1396,8 +1410,14 @@  out_close:
 	 * with the same pid later, it will try to reuse this /proc/self.
 	 */
 	close_pid_proc();
+out_unblock:
+	if (sigprocmask(SIG_SETMASK, &oldmask, NULL) == -1) {
+		pr_perror("Can not unset mask of blocked signals");
+		ret = -1;
+	}
 out_munmap:
 	munmap(stack, size);
+
 	return ret;
 }
 

Comments

Kirill Tkhai March 30, 2017, 9:05 a.m.
On 30.03.2017 08:48, Andrei Vagin wrote:
> From: Andrei Vagin <avagin@virtuozzo.com>
> 
> CRIU sets a sigchld handler and calls waitpid from it,
> but when we call a sub-process, we want to wait it
> 
> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>

Ok for idea, but, please, see comments below.

> ---
>  criu/util.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/util.c b/criu/util.c
> index 558d498..c014a5d 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -1369,6 +1369,7 @@ int open_fd_of_real_pid(pid_t pid, int fd, int flags)
>  
>  int call_in_child_process(int (*fn)(void *), void *arg)
>  {
> +	sigset_t blockmask, oldmask;
>  	int size, status, ret = -1;
>  	char *stack;
>  	pid_t pid;
> @@ -1379,14 +1380,27 @@ int call_in_child_process(int (*fn)(void *), void *arg)
>  		pr_perror("Can't allocate stack");
>  		return -1;
>  	}
> +
> +	sigemptyset(&blockmask);
> +	sigaddset(&blockmask, SIGCHLD);
> +
> +	if (sigprocmask(SIG_BLOCK, &blockmask, &oldmask) == -1) {
> +		pr_perror("Can not set mask of blocked signals");
> +		goto out_munmap;
> +	}
> +
>  	pid = clone(fn, stack + size, CLONE_VM | CLONE_FILES | SIGCHLD, arg);
>  	if (pid == -1) {
>  		pr_perror("Can't clone");
> -		goto out_munmap;
> +		goto out_unblock;
>  	}
>  	errno = 0;

Not need to clear errno, if there are two separate "if" and "pr_perror".

> -	if (waitpid(pid, &status, 0) != pid || !WIFEXITED(status) || WEXITSTATUS(status)) {
> -		pr_err("Can't wait or bad status: errno=%d, status=%d", errno, status);
> +	if (waitpid(pid, &status, 0) != pid) {
> +		pr_perror("Unable to wait %d", pid);
> +		goto out_close;
> +	}
> +	if ( !WIFEXITED(status) || WEXITSTATUS(status)) {
> +		pr_err("Can't wait or bad status: errno=%d (%s), status=%d", errno, strerror(errno), status);

Not need to print errno as it's already in the first "if".

>  		goto out_close;
>  	}
>  	ret = 0;
> @@ -1396,8 +1410,14 @@ out_close:
>  	 * with the same pid later, it will try to reuse this /proc/self.
>  	 */
>  	close_pid_proc();
> +out_unblock:
> +	if (sigprocmask(SIG_SETMASK, &oldmask, NULL) == -1) {
> +		pr_perror("Can not unset mask of blocked signals");
> +		ret = -1;
> +	}
>  out_munmap:
>  	munmap(stack, size);
> +
>  	return ret;
>  }
>  
>
Andrey Vagin March 31, 2017, 9:20 p.m.
Applied
On Thu, Mar 30, 2017 at 08:48:21AM +0300, Andrei Vagin wrote:
> From: Andrei Vagin <avagin@virtuozzo.com>
> 
> CRIU sets a sigchld handler and calls waitpid from it,
> but when we call a sub-process, we want to wait it
> 
> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> ---
>  criu/util.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/util.c b/criu/util.c
> index 558d498..c014a5d 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -1369,6 +1369,7 @@ int open_fd_of_real_pid(pid_t pid, int fd, int flags)
>  
>  int call_in_child_process(int (*fn)(void *), void *arg)
>  {
> +	sigset_t blockmask, oldmask;
>  	int size, status, ret = -1;
>  	char *stack;
>  	pid_t pid;
> @@ -1379,14 +1380,27 @@ int call_in_child_process(int (*fn)(void *), void *arg)
>  		pr_perror("Can't allocate stack");
>  		return -1;
>  	}
> +
> +	sigemptyset(&blockmask);
> +	sigaddset(&blockmask, SIGCHLD);
> +
> +	if (sigprocmask(SIG_BLOCK, &blockmask, &oldmask) == -1) {
> +		pr_perror("Can not set mask of blocked signals");
> +		goto out_munmap;
> +	}
> +
>  	pid = clone(fn, stack + size, CLONE_VM | CLONE_FILES | SIGCHLD, arg);
>  	if (pid == -1) {
>  		pr_perror("Can't clone");
> -		goto out_munmap;
> +		goto out_unblock;
>  	}
>  	errno = 0;
> -	if (waitpid(pid, &status, 0) != pid || !WIFEXITED(status) || WEXITSTATUS(status)) {
> -		pr_err("Can't wait or bad status: errno=%d, status=%d", errno, status);
> +	if (waitpid(pid, &status, 0) != pid) {
> +		pr_perror("Unable to wait %d", pid);
> +		goto out_close;
> +	}
> +	if ( !WIFEXITED(status) || WEXITSTATUS(status)) {
> +		pr_err("Can't wait or bad status: errno=%d (%s), status=%d", errno, strerror(errno), status);
>  		goto out_close;
>  	}
>  	ret = 0;
> @@ -1396,8 +1410,14 @@ out_close:
>  	 * with the same pid later, it will try to reuse this /proc/self.
>  	 */
>  	close_pid_proc();
> +out_unblock:
> +	if (sigprocmask(SIG_SETMASK, &oldmask, NULL) == -1) {
> +		pr_perror("Can not unset mask of blocked signals");
> +		ret = -1;
> +	}
>  out_munmap:
>  	munmap(stack, size);
> +
>  	return ret;
>  }
>  
> -- 
> 2.7.4
>