git/run-command.h, branch v2.9.2

Merge branch 'jk/push-client-deadlock-fix'

2016-04-29T19:59:08Z

"git push" from a corrupt repository that attempts to push a large number of refs deadlocked; the thread to relay rejection notices for these ref updates blocked on writing them to the main thread, after the main thread at the receiving end notices that the push failed and decides not to read these notices and return a failure. * jk/push-client-deadlock-fix: t5504: drop sigpipe=ok from push tests fetch-pack: isolate sigpipe in demuxer thread send-pack: isolate sigpipe in demuxer thread run-command: teach async threads to ignore SIGPIPE send-pack: close demux pipe before finishing async process

run-command: teach async threads to ignore SIGPIPE

2016-04-20T20:33:53Z

Async processes can be implemented as separate forked processes, or as threads (depending on the NO_PTHREADS setting). In the latter case, if an async thread gets SIGPIPE, it takes down the whole process. This is obviously bad if the main process was not otherwise going to die, but even if we were going to die, it means the main process does not have a chance to report a useful error message. There's also the small matter that forked async processes will not take the main process down on a signal, meaning git will behave differently depending on the NO_PTHREADS setting. This patch fixes it by adding a new flag to "struct async" to block SIGPIPE just in the async thread. In theory, this should always be on (which makes async threads behave more like async processes), but we would first want to make sure that each async process we spawn is careful about checking return codes from write() and would not spew endlessly into a dead pipe. So let's start with it as optional, and we can enable it for specific sites in future patches. The natural name for this option would be "ignore_sigpipe", since that's what it does for the threaded case. But since that name might imply that we are ignoring it in all cases (including the separate-process one), let's call it "isolate_sigpipe". What we are really asking for is isolation. I.e., not to have our main process taken down by signals spawned by the async process. How that is implemented is up to the run-command code. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'sb/submodule-parallel-update'

2016-04-06T18:39:01Z

A major part of "git submodule update" has been ported to C to take advantage of the recently added framework to run download tasks in parallel. * sb/submodule-parallel-update: clone: allow an explicit argument for parallel submodule clones submodule update: expose parallelism to the user submodule helper: remove double 'fatal: ' prefix git submodule update: have a dedicated helper for cloning run_processes_parallel: rename parameters for the callbacks run_processes_parallel: treat output of children as byte array submodule update: direct error message to stderr fetching submodules: respect `submodule.fetchJobs` config option submodule-config: drop check against NULL submodule-config: keep update strategy around

Merge branch 'sb/submodule-parallel-fetch'

2016-03-04T21:46:30Z

Simplify the two callback functions that are triggered when the child process terminates to avoid misuse of the child-process structure that has already been cleaned up. * sb/submodule-parallel-fetch: run-command: do not pass child process data into callbacks

run_processes_parallel: rename parameters for the callbacks

2016-03-01T19:57:19Z

The refs code has a similar pattern of passing around 'struct strbuf *err', which is strictly used for error reporting. This is not the case here, as the strbuf is used to accumulate all the output (whether it is error or not) for the user. Rename it to 'out'. Suggested-by: Jonathan Nieder Reviewed-by: Jonathan Nieder Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano

run-command: do not pass child process data into callbacks

2016-03-01T17:42:01Z

The expected way to pass data into the callback is to pass them via the customizable callback pointer. The error reporting in default_{start_failure, task_finished} is not user friendly enough, that we want to encourage using the child data for such purposes. Furthermore the struct child data is cleaned by the run-command API, before we access them in the callbacks, leading to use-after-free situations. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano

Merge branch 'jk/epipe-in-async'

2016-02-26T21:37:26Z

Handling of errors while writing into our internal asynchronous process has been made more robust, which reduces flakiness in our tests. * jk/epipe-in-async: t5504: handle expected output from SIGPIPE death test_must_fail: report number of unexpected signal fetch-pack: ignore SIGPIPE in sideband demuxer write_or_die: handle EPIPE in async threads

write_or_die: handle EPIPE in async threads

2016-02-25T21:51:45Z

When write_or_die() sees EPIPE, it treats it specially by converting it into a SIGPIPE death. We obviously cannot ignore it, as the write has failed and the caller expects us to die. But likewise, we cannot just call die(), because printing any message at all would be a nuisance during normal operations. However, this is a problem if write_or_die() is called from a thread. Our raised signal ends up killing the whole process, when logically we just need to kill the thread (after all, if we are ignoring SIGPIPE, there is good reason to think that the main thread is expecting to handle it). Inside an async thread, the die() code already does the right thing, because we use our custom die_async() routine, which calls pthread_join(). So ideally we would piggy-back on that, and simply call: die_quietly_with_code(141); or similar. But refactoring the die code to do this is surprisingly non-trivial. The die_routines themselves handle both printing and the decision of the exit code. Every one of them would have to be modified to take new parameters for the code, and to tell us to be quiet. Instead, we can just teach write_or_die() to check for the async case and handle it specially. We do have to build an interface to abstract the async exit, but it's simple and self-contained. If we had many call-sites that wanted to do this die_quietly_with_code(), this approach wouldn't scale as well, but we don't. This is the only place where do this weird exit trick. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

run-command: add an asynchronous parallel child processor

2015-12-16T20:06:08Z

This allows to run external commands in parallel with ordered output on stderr. If we run external commands in parallel we cannot pipe the output directly to the our stdout/err as it would mix up. So each process's output will flow through a pipe, which we buffer. One subprocess can be directly piped to out stdout/err for a low latency feedback to the user. Example: Let's assume we have 5 submodules A,B,C,D,E and each fetch takes a different amount of time as the different submodules vary in size, then the output of fetches in sequential order might look like this: time --> output: |---A---| |-B-| |-------C-------| |-D-| |-E-| When we schedule these submodules into maximal two parallel processes, a schedule and sample output over time may look like this: process 1: |---A---| |-D-| |-E-| process 2: |-B-| |-------C-------| output: |---A---|B|---C-------|DE So A will be perceived as it would run normally in the single child version. As B has finished by the time A is done, we can dump its whole progress buffer on stderr, such that it looks like it finished in no time. Once that is done, C is determined to be the visible child and its progress will be reported in real time. So this way of output is really good for human consumption, as it only changes the timing, not the actual output. For machine consumption the output needs to be prepared in the tasks, by either having a prefix per line or per block to indicate whose tasks output is displayed, because the output order may not follow the original sequential ordering: |----A----| |--B--| |-C-| will be scheduled to be all parallel: process 1: |----A----| process 2: |--B--| process 3: |-C-| output: |----A----|CB This happens because C finished before B did, so it will be queued for output before B. To detect when a child has finished executing, we check interleaved with other actions (such as checking the liveliness of children or starting new processes) whether the stderr pipe still exists. Once a child closed its stderr stream, we assume it is terminating very soon, and use `finish_command()` from the single external process execution interface to collect the exit status. By maintaining the strong assumption of stderr being open until the very end of a child process, we can avoid other hassle such as an implementation using `waitpid(-1)`, which is not implemented in Windows. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano

Merge branch 'rs/daemon-plug-child-leak'

2015-11-03T23:13:12Z

"git daemon" uses "run_command()" without "finish_command()", so it needs to release resources itself, which it forgot to do. * rs/daemon-plug-child-leak: daemon: plug memory leak run-command: factor out child_process_clear()