[File] [PATCH 4/4] fix waiting bugs: deadlock and waiting for a potentially wrong child

Denys Vlasenko dvlasenk at redhat.com
Fri May 3 10:57:01 UTC 2019


Signed-off-by: Denys Vlasenko <dvlasenk at redhat.com>
---
 src/compress.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/compress.c b/src/compress.c
index 417ccc86..061452dc 100644
--- a/src/compress.c
+++ b/src/compress.c
@@ -607,11 +607,10 @@ copydesc(int i, int fd)
 	return 1;
 }
 
-static void
+static pid_t
 writechild(int fd, const void *old, size_t n)
 {
 	pid_t pid;
-	int status;
 
 	/*
 	 * fork again, to avoid blocking because both
@@ -631,11 +630,7 @@ writechild(int fd, const void *old, size_t n)
 		exit(0);
 	}
 	/* parent */
-	if (wait(&status) == -1) {
-		DPRINTF("Wait failed (%s)\n", strerror(errno));
-		exit(1);
-	}
-	DPRINTF("Grandchild wait return %#x\n", status);
+	return pid;
 }
 
 static ssize_t
@@ -682,8 +677,9 @@ uncompressbuf(int fd, size_t bytes_max, size_t method, const unsigned char *old,
     unsigned char **newch, size_t* n)
 {
 	int fdp[3][2];
-	int status, rv;
+	int status, rv, w;
 	pid_t pid;
+	pid_t writepid = -1;
 	size_t i;
 	ssize_t r;
 
@@ -756,8 +752,7 @@ uncompressbuf(int fd, size_t bytes_max, size_t method, const unsigned char *old,
 	/* Write the buffer data to child stdin, if we don't have fd */
 	if (fd == -1) {
 		closefd(fdp[STDIN_FILENO], 0);
-///BUG: can block in wait()
-		writechild(fdp[STDIN_FILENO][1], old, *n);
+		writepid = writechild(fdp[STDIN_FILENO][1], old, *n);
 		closefd(fdp[STDIN_FILENO], 1);
 	}
 
@@ -796,8 +791,10 @@ err:
 	closefd(fdp[STDIN_FILENO], 1);
 	closefd(fdp[STDOUT_FILENO], 0);
 	closefd(fdp[STDERR_FILENO], 0);
-///BUG: can wait for a wrong child
-	if (wait(&status) == -1) {
+
+	w = waitpid(pid, &status, 0);
+wait_err:
+	if (w == -1) {
 		free(*newch);
 		rv = makeerror(newch, n, "Wait failed, %s", strerror(errno));
 		DPRINTF("Child wait return %#x\n", status);
@@ -806,8 +803,17 @@ err:
 	} else if (WEXITSTATUS(status) != 0) {
 		DPRINTF("Child exited (%#x)\n", WEXITSTATUS(status));
 	}
+	if (writepid > 0) {
+		/* _After_ we know decompressor has exited, our input writer
+		 * definitely will exit now (at worst, writing fails in it,
+		 * since output fd is closed now on the reading size).
+		 */
+		w = waitpid(writepid, &status, 0);
+		writepid = -1;
+		goto wait_err;
+	}
 
-	closefd(fdp[STDIN_FILENO], 0);
+	closefd(fdp[STDIN_FILENO], 0); //why? it is already closed here!
 	DPRINTF("Returning %p n=%" SIZE_T_FORMAT "u rv=%d\n", *newch, *n, rv);
 
 	return rv;
-- 
2.21.0



More information about the File mailing list