From 661a8cf408e83e4901bf09e2a48e9306622442dd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 1 Sep 2015 16:22:43 -0400 Subject: [PATCH 1/2] run-command: provide in_async query function It's not easy for arbitrary code to find out whether it is running in an async process or not. A top-level function which is fed to start_async() can know (you just pass down an argument saying "you are async"). But that function may call other global functions, and we would not want to have to pass the information all the way through the call stack. Nor can we simply set a global variable, as those may be shared between async threads and the main thread (if the platform supports pthreads). We need pthread tricks _or_ a global variable, depending on how start_async is implemented. The callers don't have enough information to do this right, so let's provide a simple query function that does. Fortunately we can reuse the existing infrastructure to make the pthread case simple (and even simplify die_async() by using our new function). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 16 +++++++++++++++- run-command.h | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/run-command.c b/run-command.c index 4d73e90fa..6131a44bd 100644 --- a/run-command.c +++ b/run-command.c @@ -608,7 +608,7 @@ static NORETURN void die_async(const char *err, va_list params) { vreportf("fatal: ", err, params); - if (!pthread_equal(main_thread, pthread_self())) { + if (in_async()) { struct async *async = pthread_getspecific(async_key); if (async->proc_in >= 0) close(async->proc_in); @@ -627,6 +627,13 @@ static int async_die_is_recursing(void) return ret != NULL; } +int in_async(void) +{ + if (!main_thread_set) + return 0; /* no asyncs started yet */ + return !pthread_equal(main_thread, pthread_self()); +} + #else static struct { @@ -666,6 +673,12 @@ int git_atexit(void (*handler)(void)) } #define atexit git_atexit +static int process_is_async; +int in_async(void) +{ + return process_is_async; +} + #endif int start_async(struct async *async) @@ -725,6 +738,7 @@ int start_async(struct async *async) if (need_out) close(fdout[0]); git_atexit_clear(); + process_is_async = 1; exit(!!async->proc(proc_in, proc_out, async->data)); } diff --git a/run-command.h b/run-command.h index 1103805af..4aaac7ce0 100644 --- a/run-command.h +++ b/run-command.h @@ -113,5 +113,6 @@ struct async { int start_async(struct async *async); int finish_async(struct async *async); +int in_async(void); #endif From fd89433dd03babb6c8d760092c3e499f6a4145f4 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 1 Sep 2015 16:24:13 -0400 Subject: [PATCH 2/2] pkt-line: show packets in async processes as "sideband" If you run "GIT_TRACE_PACKET=1 git push", you may get confusing output like (line prefixes omitted for clarity): packet: push< \1000eunpack ok0019ok refs/heads/master0000 packet: push< unpack ok packet: push< ok refs/heads/master packet: push< 0000 packet: push< 0000 Why do we see the data twice, once apparently wrapped inside another pkt-line, and once unwrapped? Why do we get two flush packets? The answer is that we start an async process to demux the sideband data. The first entry comes from the sideband process reading the data, and the second from push itself. Likewise, the first flush is inside the demuxed packet, and the second is an actual sideband flush. We can make this a bit more clear by marking the sideband demuxer explicitly as "sideband" rather than "push". The most elegant way to do this would be to simply call packet_trace_identity() inside the sideband demuxer. But we can't do that reliably, because it relies on a global variable, which might be shared if pthreads are in use. What we really need is thread-local storage for packet_trace_identity. But the async code does not provide an interface for that, and it would be messy to add it here (we'd have to care about pthreads, initializing our pthread_key_t ahead of time, etc). So instead, let us just assume that any async process is handling sideband data. That's always true now, and is likely to remain so in the future. The output looks like: packet: sideband< \1000eunpack ok0019ok refs/heads/master0000 packet: push< unpack ok packet: push< ok refs/heads/master packet: push< 0000 packet: sideband< 0000 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pkt-line.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkt-line.c b/pkt-line.c index 08a1427c0..62fdb3707 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -1,5 +1,6 @@ #include "cache.h" #include "pkt-line.h" +#include "run-command.h" char packet_buffer[LARGE_PACKET_MAX]; static const char *packet_trace_prefix = "git"; @@ -11,6 +12,11 @@ void packet_trace_identity(const char *prog) packet_trace_prefix = xstrdup(prog); } +static const char *get_trace_prefix(void) +{ + return in_async() ? "sideband" : packet_trace_prefix; +} + static int packet_trace_pack(const char *buf, unsigned int len, int sideband) { if (!sideband) { @@ -57,7 +63,7 @@ static void packet_trace(const char *buf, unsigned int len, int write) strbuf_init(&out, len+32); strbuf_addf(&out, "packet: %12s%c ", - packet_trace_prefix, write ? '>' : '<'); + get_trace_prefix(), write ? '>' : '<'); /* XXX we should really handle printable utf8 */ for (i = 0; i < len; i++) {