Re: [PATCH] run-command: detect finished children by closed pipe rather than waitpid

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

 



Am 07.11.2015 um 00:48 schrieb Stefan Beller:
Detect if a child stopped working by checking if their stderr pipe
was closed instead of checking their state with waitpid.
As waitpid is not fully working in Windows, this is an approach which
allows for better cross platform operation. (It's less code, too)

Previously we did not close the read pipe of finished children, which we
do now.

The old way missed some messages on an early abort. We just killed the
children and did not bother to look what was left over. With this approach
we'd send a signal to the children and wait for them to close the pipe to
have all the messages (including possible "killed by signal 15" messages).

To have the test suite passing as before, we allow for real graceful
abortion now. In case the user wishes to abort parallel execution
the user needs to provide either the signal used to kill all children
or the children are let run until they finish normally.

Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
---
  Hi,

  this applis on top of origin/sb/submodule-parallel-fetch,
  making Windows folks possibly even more happy. It makes the code easier
  to read and has less races on cleaning up a terminated child.

  It follows the idea of Johannes patch, instead of encoding information in .err
  I removed the in_use flag and added a state, currently having 3 states.

  Thanks,
  Stefan

  Johannes schrieb:
  > First let me say that I find it very questionable that the callbacks
  > receive a struct child_process.

  I tried to get rid of the child_process struct in the callbacks, but that's
  not as easy as one may think.

Fair enough. I see you removed .err, .no_stdin and .stdout_to_stderr from the callback. Good.

  		pp->nr_processes--;
-		pp->children[i].in_use = 0;
+		pp->children[i].state = FREE;
  		pp->pfd[i].fd = -1;
  		child_process_deinit(&pp->children[i].process);

This cleanup is implied by finish_command and can be removed.

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

@@ -1195,12 +1175,12 @@ int run_processes_parallel(int n,
  		    i < spawn_cap && !pp->shutdown &&
  		    pp->nr_processes < pp->max_processes;
  		    i++) {
-			int code = pp_start_one(pp);
+			code = pp_start_one(pp);
  			if (!code)
  				continue;
  			if (code < 0) {
  				pp->shutdown = 1;
-				kill_children(pp, SIGTERM);
+				kill_children(pp, -code);

I'll see what this means for our kill emulation on Windows. Currently, we handle only SIGTERM.

  			}
  			break;
  		}

Thanks you very much!

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