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

> 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@google.com>
> ---
>   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.

>   		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);

> @@ -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);