Permalink
Browse files

Dying in an async procedure should only exit the thread, not the proc…

…ess.

Async procedures are intended as helpers that perform a very restricted
task, and the caller usually has to manage them in a larger context.
Conceptually, the async procedure is not concerned with the "bigger
picture" in whose context it is run. When it dies, it is not supposed
to destroy this "bigger picture", but rather only its own limit view
of the world. On POSIX, the async procedure is run in its own process,
and exiting this process naturally had only these limited effects.

On Windows (or when ASYNC_AS_THREAD is set), calling die() exited the
whole process, destroying the caller (the "big picture") as well.
This fixes it to exit only the thread.

Without ASYNC_AS_THREAD, one particular effect of exiting the async
procedure process is that it automatically closes file descriptors, most
notably the writable end of the pipe that the async procedure writes to.

The async API already requires that the async procedure closes the pipe
ends when it exits normally. But for calls to die() no requirements are
imposed. In the non-threaded case the pipe ends are closed implicitly
by the exiting process, but in the threaded case, the die routine must
take care of closing them.

Now t5530-upload-pack-error.sh passes on Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information...
1 parent 200a76b commit 0ea1c89ba616397ef7e5f6f601ef7a24d2c27b8e @j6t j6t committed with Mar 6, 2010
Showing with 34 additions and 0 deletions.
  1. +34 −0 run-command.c
View
@@ -448,12 +448,35 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
}
#ifdef ASYNC_AS_THREAD
+static pthread_t main_thread;
+static int main_thread_set;
+static pthread_key_t async_key;
+
static void *run_thread(void *data)
{
struct async *async = data;
+
+ pthread_setspecific(async_key, async);
+
intptr_t ret = async->proc(async->proc_in, async->proc_out, async->data);
return (void *)ret;
}
+
+static NORETURN void die_async(const char *err, va_list params)
+{
+ vreportf("fatal: ", err, params);
+
+ if (!pthread_equal(main_thread, pthread_self())) {
+ struct async *async = pthread_getspecific(async_key);
+ if (async->proc_in >= 0)
+ close(async->proc_in);
+ if (async->proc_out >= 0)
+ close(async->proc_out);
+ pthread_exit((void *)128);
+ }
+
+ exit(128);
+}
#endif
int start_async(struct async *async)
@@ -525,6 +548,17 @@ int start_async(struct async *async)
else if (async->out)
close(async->out);
#else
+ if (!main_thread_set) {
+ /*
+ * We assume that the first time that start_async is called
+ * it is from the main thread.
+ */
+ main_thread_set = 1;
+ main_thread = pthread_self();
+ pthread_key_create(&async_key, NULL);
+ set_die_routine(die_async);
+ }
+
if (proc_in >= 0)
set_cloexec(proc_in);
if (proc_out >= 0)

0 comments on commit 0ea1c89

Please sign in to comment.