Refactored nxt_process_create() for more explicit pipe closing.

1 files changed, 29 insertions, 40 deletions

diff --git a/src/nxt_process.c b/src/nxt_process.c
index d83eed28..0cc9ccc4 100644
--- a/src/nxt_process.c
+++ b/src/nxt_process.c
@@ -42,7 +42,8 @@ nxt_bool_t  nxt_proc_remove_notify_matrix[NXT_PROCESS_MAX][NXT_PROCESS_MAX] = {
 
 
 static nxt_int_t
-nxt_process_worker_setup(nxt_task_t *task, nxt_process_t *process, int parentfd) {
+nxt_process_worker_setup(nxt_task_t *task, nxt_process_t *process, int parentfd)
+{
     pid_t               rpid, pid;
     ssize_t             n;
     nxt_int_t           parent_status;
@@ -87,11 +88,6 @@ nxt_process_worker_setup(nxt_task_t *task, nxt_process_t *process, int parentfd)
         return NXT_ERROR;
     }
 
-    if (nxt_slow_path(close(parentfd) == -1)) {
-        nxt_alert(task, "failed to close reader pipe fd");
-        return NXT_ERROR;
-    }
-
     if (nxt_slow_path(parent_status != NXT_OK)) {
         return parent_status;
     }
@@ -152,37 +148,26 @@ nxt_process_create(nxt_task_t *task, nxt_process_t *process)
     init = process->init;
 
 #if (NXT_HAVE_CLONE)
-    pid = nxt_clone(SIGCHLD|init->isolation.clone.flags);
-#else
-    pid = fork();
-#endif
-
+    pid = nxt_clone(SIGCHLD | init->isolation.clone.flags);
     if (nxt_slow_path(pid < 0)) {
-#if (NXT_HAVE_CLONE)
         nxt_alert(task, "clone() failed while creating \"%s\" %E",
                   init->name, nxt_errno);
+        goto cleanup;
+    }
 #else
+    pid = fork();
+    if (nxt_slow_path(pid < 0)) {
         nxt_alert(task, "fork() failed while creating \"%s\" %E",
                   init->name, nxt_errno);
-#endif
-
-        if (nxt_slow_path(close(pipefd[0]) != 0)) {
-            nxt_alert(task, "failed to close pipe: %E", nxt_errno);
-        }
-
-        if (nxt_slow_path(close(pipefd[1]) != 0)) {
-            nxt_alert(task, "failed to close pipe: %E", nxt_errno);
-        }
-
-        return pid;
+        goto cleanup;
     }
+#endif
 
     if (pid == 0) {
         /* Child. */
 
         if (nxt_slow_path(close(pipefd[1]) == -1)) {
             nxt_alert(task, "failed to close writer pipe fd");
-            return NXT_ERROR;
         }
 
         ret = nxt_process_worker_setup(task, process, pipefd[0]);
@@ -190,6 +175,10 @@ nxt_process_create(nxt_task_t *task, nxt_process_t *process)
             exit(1);
         }
 
+        if (nxt_slow_path(close(pipefd[0]) == -1)) {
+            nxt_alert(task, "failed to close writer pipe fd");
+        }
+
         /*
          * Explicitly return 0 to notice the caller function this is the child.
          * The caller must return to the event engine work queue loop.
@@ -199,10 +188,6 @@ nxt_process_create(nxt_task_t *task, nxt_process_t *process)
 
     /* Parent. */
 
-    if (nxt_slow_path(close(pipefd[0]) != 0)) {
-        nxt_alert(task, "failed to close pipe: %E", nxt_errno);
-    }
-
     /*
      * At this point, the child process is blocked reading the
      * pipe fd to get its real pid (rpid).
@@ -219,14 +204,14 @@ nxt_process_create(nxt_task_t *task, nxt_process_t *process)
 
     if (nxt_slow_path(write(pipefd[1], &pid, sizeof(pid)) == -1)) {
         nxt_alert(task, "failed to write real pid");
-        goto fail_cleanup;
+        goto fail;
     }
 
 #if (NXT_HAVE_CLONE_NEWUSER)
     if ((init->isolation.clone.flags & CLONE_NEWUSER) == CLONE_NEWUSER) {
         ret = nxt_clone_proc_map(task, pid, &init->isolation.clone);
         if (nxt_slow_path(ret != NXT_OK)) {
-            goto fail_cleanup;
+            goto fail;
         }
     }
 #endif
@@ -235,20 +220,16 @@ nxt_process_create(nxt_task_t *task, nxt_process_t *process)
 
     if (nxt_slow_path(write(pipefd[1], &ret, sizeof(ret)) == -1)) {
         nxt_alert(task, "failed to write status");
-        goto fail_cleanup;
-    }
-
-    if (nxt_slow_path(close(pipefd[1]) != 0)) {
-        nxt_alert(task, "failed to close pipe: %E", nxt_errno);
+        goto fail;
     }
 
     process->pid = pid;
 
     nxt_runtime_process_add(task, process);
 
-    return pid;
+    goto cleanup;
 
-fail_cleanup:
+fail:
 
     ret = NXT_ERROR;
 
@@ -256,13 +237,21 @@ fail_cleanup:
         nxt_alert(task, "failed to write status");
     }
 
-    if (nxt_slow_path(close(pipefd[1]) != 0)) {
+    waitpid(pid, NULL, 0);
+
+    pid = -1;
+
+cleanup:
+
+    if (nxt_slow_path(close(pipefd[0]) != 0)) {
         nxt_alert(task, "failed to close pipe: %E", nxt_errno);
     }
 
-    waitpid(pid, NULL, 0);
+    if (nxt_slow_path(close(pipefd[1]) != 0)) {
+        nxt_alert(task, "failed to close pipe: %E", nxt_errno);
+    }
 
-    return -1;
+    return pid;
 }