Re: [PATCH 1/2] run-command: Remove set_nonblocking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 05.11.2015 um 23:20 schrieb Stefan Beller:
On Thu, Nov 5, 2015 at 12:27 PM, Johannes Sixt <j6t@xxxxxxxx> wrote:

diff --git a/run-command.c b/run-command.c
index 51d078c..3e42299 100644
--- a/run-command.c
+++ b/run-command.c
@@ -977,7 +977,7 @@ static struct parallel_processes *pp_init(int n,
         for (i = 0; i < n; i++) {
                 strbuf_init(&pp->children[i].err, 0);
                 child_process_init(&pp->children[i].process);
-               pp->pfd[i].events = POLLIN;
+               pp->pfd[i].events = POLLIN|POLLHUP;
                 pp->pfd[i].fd = -1;
         }
         sigchain_push_common(handle_children_on_signal);
@@ -1061,11 +1061,17 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout)
         /* Buffer output from all pipes. */
         for (i = 0; i < pp->max_processes; i++) {
                 if (pp->children[i].in_use &&
-                   pp->pfd[i].revents & POLLIN)
-                       if (strbuf_read_once(&pp->children[i].err,
-                                            pp->children[i].process.err, 0) < 0)
+                   pp->pfd[i].revents & (POLLIN|POLLHUP)) {
+                       int n = strbuf_read_once(&pp->children[i].err,
+                                            pp->children[i].process.err, 0);
+                       if (n == 0) {
+                               close(pp->children[i].process.err);
+                               pp->children[i].process.err = -1;

So you set .err to -1 to signal the process has ended here...

-
                 for (i = 0; i < pp->max_processes; i++)
                         if (pp->children[i].in_use &&
-                           pid == pp->children[i].process.pid)
+                           pp->children[i].process.err == -1)
                                 break;

to make a decision here if we want to finish_command on it.

Correct.

+               code = finish_command(&pp->children[i].process);

-               child_process_clear(&pp->children[i].process);

but .err stays stays -1 here for the next iteration?
We would need to reset it to 0 again.

No. In the next round, we need -1 to request a pipe. get_next_task callback sets it to -1 as well (and I think it is wrong that the callback does it; pp_start_one should do that).

So .err is
   0 when the slot is not in use
  -1 when the child has finished awaiting termination
  >0 when the child is living a happy life.

But, as I said, .err is not the right place to mark dying processes (it was just the simplest way to demonstrate the concept in this patch). Better extend .in_use to a tri-state indicator.

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]