threads / patch / 37713

patchHandle atexit list internaly fo unthreaded builds

Subject: [PATCH] Handle atexit list internaly fo unthreaded builds

## tl;dr

10 messages between Oct 11, 2014 and Oct 18, 2014. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Etienne Buira· Oct 11, 2014, 14:53 UTC · lore

Replace atexit()s calls with cmd_atexit that is atexit() on threaded builds, but handles the callbacks list internally for unthreaded builds.

This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of the parent process). That led to remove temporary and lock files too early.

Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it)

Signed-off-by: Etienne Buira <etienne.buira@gmail.com>
---
 builtin/clone.c |  7 +------
 builtin/fetch.c |  2 +-
 builtin/gc.c    |  2 +-
 diff.c          |  2 +-
 lockfile.c      |  2 +-
 pager.c         |  2 +-
 read-cache.c    |  2 +-
 run-command.c   | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 shallow.c       |  7 ++-----
 trace.c         |  2 +-
 10 files changed, 54 insertions(+), 19 deletions(-)
Show changes to 10 files +54 −19

builtin/clone.c, builtin/fetch.c, builtin/gc.c, diff.c, lockfile.c, pager.c, read-cache.c, run-command.c, shallow.c, trace.c

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..2992ac0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 
 static const char *junk_work_tree;
 static const char *junk_git_dir;
-static pid_t junk_pid;
 static enum {
 	JUNK_LEAVE_NONE,
 	JUNK_LEAVE_REPO,
@@ -417,8 +416,6 @@ static void remove_junk(void)
 		break;
 	}
 
-	if (getpid() != junk_pid)
-		return;
 	if (junk_git_dir) {
 		strbuf_addstr(&sb, junk_git_dir);
 		remove_dir_recursively(&sb, 0);
@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct refspec *refspec;
 	const char *fetch_pattern;
 
-	junk_pid = getpid();
-
 	packet_trace_identity("clone");
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
@@ -843,7 +838,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		set_git_work_tree(work_tree);
 	}
 	junk_git_dir = git_dir;
-	atexit(remove_junk);
+	cmd_atexit(remove_junk);
 	sigchain_push_common(remove_junk_on_signal);
 
 	if (safe_create_leading_directories_const(git_dir) < 0)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 159fb7e..1e44d58 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1095,7 +1095,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	}
 
 	sigchain_push_common(unlock_pack_on_signal);
-	atexit(unlock_pack);
+	cmd_atexit(unlock_pack);
 	refspec = parse_fetch_refspec(ref_nr, refs);
 	exit_code = do_fetch(gtransport, refspec, ref_nr);
 	free_refspec(ref_nr, refspec);
diff --git a/builtin/gc.c b/builtin/gc.c
index 8d219d8..cf2defa 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -254,7 +254,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 
 	pidfile = git_pathdup("gc.pid");
 	sigchain_push_common(remove_pidfile_on_signal);
-	atexit(remove_pidfile);
+	cmd_atexit(remove_pidfile);
 
 	return NULL;
 }
diff --git a/diff.c b/diff.c
index 867f034..9c6ef9a 100644
--- a/diff.c
+++ b/diff.c
@@ -2833,7 +2833,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
 	}
 
 	if (!remove_tempfile_installed) {
-		atexit(remove_tempfile);
+		cmd_atexit(remove_tempfile);
 		sigchain_push_common(remove_tempfile_on_signal);
 		remove_tempfile_installed = 1;
 	}
diff --git a/lockfile.c b/lockfile.c
index 2564a7f..ad0d1e2 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -141,7 +141,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 	if (0 <= lk->fd) {
 		if (!lock_file_list) {
 			sigchain_push_common(remove_lock_file_on_signal);
-			atexit(remove_lock_file);
+			cmd_atexit(remove_lock_file);
 		}
 		lk->owner = getpid();
 		if (!lk->on_list) {
diff --git a/pager.c b/pager.c
index 8b5cbc5..09ab2fa 100644
--- a/pager.c
+++ b/pager.c
@@ -102,7 +102,7 @@ void setup_pager(void)
 
 	/* this makes sure that the parent terminates after the pager */
 	sigchain_push_common(wait_for_pager_signal);
-	atexit(wait_for_pager);
+	cmd_atexit(wait_for_pager);
 }
 
 int pager_in_use(void)
diff --git a/read-cache.c b/read-cache.c
index 6f0057f..8b10c92 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2104,7 +2104,7 @@ static int write_shared_index(struct index_state *istate,
 		return do_write_locked_index(istate, lock, flags);
 	}
 	if (!installed_handler) {
-		atexit(remove_temporary_sharedindex);
+		cmd_atexit(remove_temporary_sharedindex);
 		sigchain_push_common(remove_temporary_sharedindex_on_signal);
 	}
 	move_cache_to_base_index(istate);
diff --git a/run-command.c b/run-command.c
index 35a3ebf..5df57b5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -45,7 +45,7 @@ static void mark_child_for_cleanup(pid_t pid)
 	children_to_clean = p;
 
 	if (!installed_child_cleanup_handler) {
-		atexit(cleanup_children_on_exit);
+		cmd_atexit(cleanup_children_on_exit);
 		sigchain_push_common(cleanup_children_on_signal);
 		installed_child_cleanup_handler = 1;
 	}
@@ -624,6 +624,48 @@ static int async_die_is_recursing(void)
 	return ret != NULL;
 }
 
+int cmd_atexit(void (*handler)(void))
+{
+	return atexit(handler);
+}
+
+#else
+
+static struct {
+	void (**handlers)(void);
+	size_t nr;
+	size_t alloc;
+} cmd_atexit_hdlrs;
+
+static int cmd_atexit_installed;
+
+static void cmd_atexit_dispatch()
+{
+	size_t i;
+
+	for (i=cmd_atexit_hdlrs.nr ; i ; i--)
+		cmd_atexit_hdlrs.handlers[i-1]();
+}
+
+static void cmd_atexit_clear()
+{
+	free(cmd_atexit_hdlrs.handlers);
+	memset(&cmd_atexit_hdlrs, 0, sizeof(cmd_atexit_hdlrs));
+	cmd_atexit_installed = 0;
+}
+
+int cmd_atexit(void (*handler)(void))
+{
+	ALLOC_GROW(cmd_atexit_hdlrs.handlers, cmd_atexit_hdlrs.nr + 1, cmd_atexit_hdlrs.alloc);
+	cmd_atexit_hdlrs.handlers[cmd_atexit_hdlrs.nr++] = handler;
+	if (!cmd_atexit_installed) {
+		if (!atexit(&cmd_atexit_dispatch))
+			return -1;
+		cmd_atexit_installed = 1;
+	}
+	return 0;
+}
+
 #endif
 
 int start_async(struct async *async)
@@ -682,6 +724,7 @@ int start_async(struct async *async)
 			close(fdin[1]);
 		if (need_out)
 			close(fdout[0]);
+		cmd_atexit_clear();
 		exit(!!async->proc(proc_in, proc_out, async->data));
 	}
 
diff --git a/shallow.c b/shallow.c
index de07709..881580b 100644
--- a/shallow.c
+++ b/shallow.c
@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)
 
 const char *setup_temporary_shallow(const struct sha1_array *extra)
 {
-	static int installed_handler;
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
 		strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX"));
 		fd = xmkstemp(temporary_shallow.buf);
 
-		if (!installed_handler) {
-			atexit(remove_temporary_shallow);
-			sigchain_push_common(remove_temporary_shallow_on_signal);
-		}
+		cmd_atexit(remove_temporary_shallow);
+		sigchain_push_common(remove_temporary_shallow_on_signal);
 
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
 			die_errno("failed to write to %s",
diff --git a/trace.c b/trace.c
index e583dc6..36c5076 100644
--- a/trace.c
+++ b/trace.c
@@ -420,7 +420,7 @@ void trace_command_performance(const char **argv)
 		return;
 
 	if (!command_start_time)
-		atexit(print_command_performance_atexit);
+		cmd_atexit(print_command_performance_atexit);
 
 	strbuf_reset(&command_line);
 	sq_quote_argv(&command_line, argv, 0);
-- 
1.8.5.5
Etienne Buira· Oct 12, 2014, 09:09 UTC · re: Etienne Buira · lore

[PATCH v2] Handle atexit list internaly fo unthreaded builds

Replace atexit()s calls with cmd_atexit that is atexit() on threaded builds, but handles the callbacks list internally for unthreaded builds.

This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of the parent process). That led to remove temporary and lock files too early.

Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it)

V2: remove an obvious mistake
Signed-off-by: Etienne Buira <etienne.buira@gmail.com>
---
 builtin/clone.c |  7 +------
 builtin/fetch.c |  2 +-
 builtin/gc.c    |  2 +-
 diff.c          |  2 +-
 lockfile.c      |  2 +-
 pager.c         |  2 +-
 read-cache.c    |  2 +-
 run-command.c   | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 shallow.c       |  7 ++-----
 trace.c         |  2 +-
 10 files changed, 54 insertions(+), 19 deletions(-)
Show changes to 10 files +54 −19

builtin/clone.c, builtin/fetch.c, builtin/gc.c, diff.c, lockfile.c, pager.c, read-cache.c, run-command.c, shallow.c, trace.c

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..2992ac0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 
 static const char *junk_work_tree;
 static const char *junk_git_dir;
-static pid_t junk_pid;
 static enum {
 	JUNK_LEAVE_NONE,
 	JUNK_LEAVE_REPO,
@@ -417,8 +416,6 @@ static void remove_junk(void)
 		break;
 	}
 
-	if (getpid() != junk_pid)
-		return;
 	if (junk_git_dir) {
 		strbuf_addstr(&sb, junk_git_dir);
 		remove_dir_recursively(&sb, 0);
@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct refspec *refspec;
 	const char *fetch_pattern;
 
-	junk_pid = getpid();
-
 	packet_trace_identity("clone");
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
@@ -843,7 +838,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		set_git_work_tree(work_tree);
 	}
 	junk_git_dir = git_dir;
-	atexit(remove_junk);
+	cmd_atexit(remove_junk);
 	sigchain_push_common(remove_junk_on_signal);
 
 	if (safe_create_leading_directories_const(git_dir) < 0)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 159fb7e..1e44d58 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1095,7 +1095,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	}
 
 	sigchain_push_common(unlock_pack_on_signal);
-	atexit(unlock_pack);
+	cmd_atexit(unlock_pack);
 	refspec = parse_fetch_refspec(ref_nr, refs);
 	exit_code = do_fetch(gtransport, refspec, ref_nr);
 	free_refspec(ref_nr, refspec);
diff --git a/builtin/gc.c b/builtin/gc.c
index 8d219d8..cf2defa 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -254,7 +254,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 
 	pidfile = git_pathdup("gc.pid");
 	sigchain_push_common(remove_pidfile_on_signal);
-	atexit(remove_pidfile);
+	cmd_atexit(remove_pidfile);
 
 	return NULL;
 }
diff --git a/diff.c b/diff.c
index 867f034..9c6ef9a 100644
--- a/diff.c
+++ b/diff.c
@@ -2833,7 +2833,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
 	}
 
 	if (!remove_tempfile_installed) {
-		atexit(remove_tempfile);
+		cmd_atexit(remove_tempfile);
 		sigchain_push_common(remove_tempfile_on_signal);
 		remove_tempfile_installed = 1;
 	}
diff --git a/lockfile.c b/lockfile.c
index 2564a7f..ad0d1e2 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -141,7 +141,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 	if (0 <= lk->fd) {
 		if (!lock_file_list) {
 			sigchain_push_common(remove_lock_file_on_signal);
-			atexit(remove_lock_file);
+			cmd_atexit(remove_lock_file);
 		}
 		lk->owner = getpid();
 		if (!lk->on_list) {
diff --git a/pager.c b/pager.c
index 8b5cbc5..09ab2fa 100644
--- a/pager.c
+++ b/pager.c
@@ -102,7 +102,7 @@ void setup_pager(void)
 
 	/* this makes sure that the parent terminates after the pager */
 	sigchain_push_common(wait_for_pager_signal);
-	atexit(wait_for_pager);
+	cmd_atexit(wait_for_pager);
 }
 
 int pager_in_use(void)
diff --git a/read-cache.c b/read-cache.c
index 6f0057f..8b10c92 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2104,7 +2104,7 @@ static int write_shared_index(struct index_state *istate,
 		return do_write_locked_index(istate, lock, flags);
 	}
 	if (!installed_handler) {
-		atexit(remove_temporary_sharedindex);
+		cmd_atexit(remove_temporary_sharedindex);
 		sigchain_push_common(remove_temporary_sharedindex_on_signal);
 	}
 	move_cache_to_base_index(istate);
diff --git a/run-command.c b/run-command.c
index 35a3ebf..a8a6374 100644
--- a/run-command.c
+++ b/run-command.c
@@ -45,7 +45,7 @@ static void mark_child_for_cleanup(pid_t pid)
 	children_to_clean = p;
 
 	if (!installed_child_cleanup_handler) {
-		atexit(cleanup_children_on_exit);
+		cmd_atexit(cleanup_children_on_exit);
 		sigchain_push_common(cleanup_children_on_signal);
 		installed_child_cleanup_handler = 1;
 	}
@@ -624,6 +624,48 @@ static int async_die_is_recursing(void)
 	return ret != NULL;
 }
 
+int cmd_atexit(void (*handler)(void))
+{
+	return atexit(handler);
+}
+
+#else
+
+static struct {
+	void (**handlers)(void);
+	size_t nr;
+	size_t alloc;
+} cmd_atexit_hdlrs;
+
+static int cmd_atexit_installed;
+
+static void cmd_atexit_dispatch()
+{
+	size_t i;
+
+	for (i=cmd_atexit_hdlrs.nr ; i ; i--)
+		cmd_atexit_hdlrs.handlers[i-1]();
+}
+
+static void cmd_atexit_clear()
+{
+	free(cmd_atexit_hdlrs.handlers);
+	memset(&cmd_atexit_hdlrs, 0, sizeof(cmd_atexit_hdlrs));
+	cmd_atexit_installed = 0;
+}
+
+int cmd_atexit(void (*handler)(void))
+{
+	ALLOC_GROW(cmd_atexit_hdlrs.handlers, cmd_atexit_hdlrs.nr + 1, cmd_atexit_hdlrs.alloc);
+	cmd_atexit_hdlrs.handlers[cmd_atexit_hdlrs.nr++] = handler;
+	if (!cmd_atexit_installed) {
+		if (atexit(&cmd_atexit_dispatch))
+			return -1;
+		cmd_atexit_installed = 1;
+	}
+	return 0;
+}
+
 #endif
 
 int start_async(struct async *async)
@@ -682,6 +724,7 @@ int start_async(struct async *async)
 			close(fdin[1]);
 		if (need_out)
 			close(fdout[0]);
+		cmd_atexit_clear();
 		exit(!!async->proc(proc_in, proc_out, async->data));
 	}
 
diff --git a/shallow.c b/shallow.c
index de07709..881580b 100644
--- a/shallow.c
+++ b/shallow.c
@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)
 
 const char *setup_temporary_shallow(const struct sha1_array *extra)
 {
-	static int installed_handler;
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
 		strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX"));
 		fd = xmkstemp(temporary_shallow.buf);
 
-		if (!installed_handler) {
-			atexit(remove_temporary_shallow);
-			sigchain_push_common(remove_temporary_shallow_on_signal);
-		}
+		cmd_atexit(remove_temporary_shallow);
+		sigchain_push_common(remove_temporary_shallow_on_signal);
 
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
 			die_errno("failed to write to %s",
diff --git a/trace.c b/trace.c
index e583dc6..36c5076 100644
--- a/trace.c
+++ b/trace.c
@@ -420,7 +420,7 @@ void trace_command_performance(const char **argv)
 		return;
 
 	if (!command_start_time)
-		atexit(print_command_performance_atexit);
+		cmd_atexit(print_command_performance_atexit);
 
 	strbuf_reset(&command_line);
 	sq_quote_argv(&command_line, argv, 0);
-- 
2.0.4
Duy Nguyen· Oct 13, 2014, 00:56 UTC · re: Etienne Buira · lore

Re: [PATCH v2] Handle atexit list internaly fo unthreaded builds

On Sun, Oct 12, 2014 at 4:09 PM, Etienne Buira <etienne.buira@gmail.com> wrote:
> Replace atexit()s calls with cmd_atexit that is atexit() on threaded
> builds, but handles the callbacks list internally for unthreaded builds.

Maybe hide this in git-compat-util.h and "#define atexit(x) cmd_atexit(x)"? cmd_ is usually for commands' "main" functions. Maybe rename it to git_atexit().

-- 
Duy
Etienne Buira· Oct 13, 2014, 15:19 UTC · re: Duy Nguyen · lore

Re: [PATCH v2] Handle atexit list internaly for unthreaded builds

On Mon, Oct 13, 2014 at 2:56 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 6 quoted lines
> On Sun, Oct 12, 2014 at 4:09 PM, Etienne Buira <etienne.buira@gmail.com> wrote:
> > Replace atexit()s calls with cmd_atexit that is atexit() on threaded
> > builds, but handles the callbacks list internally for unthreaded builds.
> 
> Maybe hide this in git-compat-util.h and "#define atexit(x)
> cmd_atexit(x)"?
Updated.
> cmd_ is usually for commands' "main" functions. Maybe
> rename it to git_atexit().
Indeed, renamed. Thank you.
Etienne Buira· Oct 13, 2014, 15:19 UTC · re: Etienne Buira · lore

[PATCH v3] Handle atexit list internaly for unthreaded builds

Wrap atexit()s calls on unthreaded builds to handle callback list internally.

This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of the parent process). That led to remove temporary and lock files too early.

Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it)

V2: remove an obvious mistake
V3: wrap, rename and add define in git-compat-util.h
Thanks-to: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Etienne Buira <etienne.buira@gmail.com>
---
 builtin/clone.c   |  5 -----
 git-compat-util.h |  5 +++++
 run-command.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 shallow.c         |  7 ++-----
 4 files changed, 49 insertions(+), 10 deletions(-)
Show changes to 4 files +49 −10

builtin/clone.c, git-compat-util.h, run-command.c, shallow.c

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..e122f33 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 
 static const char *junk_work_tree;
 static const char *junk_git_dir;
-static pid_t junk_pid;
 static enum {
 	JUNK_LEAVE_NONE,
 	JUNK_LEAVE_REPO,
@@ -417,8 +416,6 @@ static void remove_junk(void)
 		break;
 	}
 
-	if (getpid() != junk_pid)
-		return;
 	if (junk_git_dir) {
 		strbuf_addstr(&sb, junk_git_dir);
 		remove_dir_recursively(&sb, 0);
@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct refspec *refspec;
 	const char *fetch_pattern;
 
-	junk_pid = getpid();
-
 	packet_trace_identity("clone");
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
diff --git a/git-compat-util.h b/git-compat-util.h
index f587749..6dd63dd 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);
 const char *inet_ntop(int af, const void *src, char *dst, size_t size);
 #endif
 
+#ifdef NO_PTHREADS
+#define atexit git_atexit
+extern int git_atexit(void (*handler)(void));
+#endif
+
 extern void release_pack_memory(size_t);
 
 typedef void (*try_to_free_t)(size_t);
diff --git a/run-command.c b/run-command.c
index 35a3ebf..f8dc969 100644
--- a/run-command.c
+++ b/run-command.c
@@ -624,6 +624,47 @@ static int async_die_is_recursing(void)
 	return ret != NULL;
 }
 
+#else
+
+static struct {
+	void (**handlers)(void);
+	size_t nr;
+	size_t alloc;
+} git_atexit_hdlrs;
+
+static int git_atexit_installed;
+
+static void git_atexit_dispatch()
+{
+	size_t i;
+
+	for (i=git_atexit_hdlrs.nr ; i ; i--)
+		git_atexit_hdlrs.handlers[i-1]();
+}
+
+static void git_atexit_clear()
+{
+	free(git_atexit_hdlrs.handlers);
+	memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));
+	git_atexit_installed = 0;
+}
+
+#define tmp_atexit atexit
+#undef atexit
+int git_atexit(void (*handler)(void))
+{
+	ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
+	git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;
+	if (!git_atexit_installed) {
+		if (atexit(&git_atexit_dispatch))
+			return -1;
+		git_atexit_installed = 1;
+	}
+	return 0;
+}
+#define atexit tmp_atexit
+#undef tmp_atexit
+
 #endif
 
 int start_async(struct async *async)
@@ -682,6 +723,7 @@ int start_async(struct async *async)
 			close(fdin[1]);
 		if (need_out)
 			close(fdout[0]);
+		git_atexit_clear();
 		exit(!!async->proc(proc_in, proc_out, async->data));
 	}
 
diff --git a/shallow.c b/shallow.c
index de07709..f067811 100644
--- a/shallow.c
+++ b/shallow.c
@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)
 
 const char *setup_temporary_shallow(const struct sha1_array *extra)
 {
-	static int installed_handler;
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
 		strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX"));
 		fd = xmkstemp(temporary_shallow.buf);
 
-		if (!installed_handler) {
-			atexit(remove_temporary_shallow);
-			sigchain_push_common(remove_temporary_shallow_on_signal);
-		}
+		atexit(remove_temporary_shallow);
+		sigchain_push_common(remove_temporary_shallow_on_signal);
 
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
 			die_errno("failed to write to %s",
-- 
2.0.4
Andreas Schwab· Oct 13, 2014, 16:37 UTC · re: Etienne Buira · lore

Re: [PATCH v3] Handle atexit list internaly for unthreaded builds

Etienne Buira <etienne.buira@gmail.com> writes:
> +#define tmp_atexit atexit
> +#define atexit tmp_atexit
> +#undef tmp_atexit
What is this supposed to do?
Andreas.
-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Etienne Buira· Oct 13, 2014, 18:35 UTC · re: Andreas Schwab · lore

[PATCH v4] Handle atexit list internaly for unthreaded builds

Wrap atexit()s calls on unthreaded builds to handle callback list internally.

This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of the parent process). That led to remove temporary and lock files too early.

Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it)

V4: fix bogus preprocessor directives
Thanks-to: Duy Nguyen <pclouds@gmail.com>
Thanks-to: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Etienne Buira <etienne.buira@gmail.com>
---
 builtin/clone.c   |  5 -----
 git-compat-util.h |  5 +++++
 run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++
 shallow.c         |  7 ++-----
 4 files changed, 47 insertions(+), 10 deletions(-)
Show changes to 4 files +47 −10

builtin/clone.c, git-compat-util.h, run-command.c, shallow.c

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..e122f33 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 
 static const char *junk_work_tree;
 static const char *junk_git_dir;
-static pid_t junk_pid;
 static enum {
 	JUNK_LEAVE_NONE,
 	JUNK_LEAVE_REPO,
@@ -417,8 +416,6 @@ static void remove_junk(void)
 		break;
 	}
 
-	if (getpid() != junk_pid)
-		return;
 	if (junk_git_dir) {
 		strbuf_addstr(&sb, junk_git_dir);
 		remove_dir_recursively(&sb, 0);
@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct refspec *refspec;
 	const char *fetch_pattern;
 
-	junk_pid = getpid();
-
 	packet_trace_identity("clone");
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
diff --git a/git-compat-util.h b/git-compat-util.h
index f587749..6dd63dd 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);
 const char *inet_ntop(int af, const void *src, char *dst, size_t size);
 #endif
 
+#ifdef NO_PTHREADS
+#define atexit git_atexit
+extern int git_atexit(void (*handler)(void));
+#endif
+
 extern void release_pack_memory(size_t);
 
 typedef void (*try_to_free_t)(size_t);
diff --git a/run-command.c b/run-command.c
index 35a3ebf..0f9a9b0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -624,6 +624,45 @@ static int async_die_is_recursing(void)
 	return ret != NULL;
 }
 
+#else
+
+static struct {
+	void (**handlers)(void);
+	size_t nr;
+	size_t alloc;
+} git_atexit_hdlrs;
+
+static int git_atexit_installed;
+
+static void git_atexit_dispatch()
+{
+	size_t i;
+
+	for (i=git_atexit_hdlrs.nr ; i ; i--)
+		git_atexit_hdlrs.handlers[i-1]();
+}
+
+static void git_atexit_clear()
+{
+	free(git_atexit_hdlrs.handlers);
+	memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));
+	git_atexit_installed = 0;
+}
+
+#undef atexit
+int git_atexit(void (*handler)(void))
+{
+	ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
+	git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;
+	if (!git_atexit_installed) {
+		if (atexit(&git_atexit_dispatch))
+			return -1;
+		git_atexit_installed = 1;
+	}
+	return 0;
+}
+#define atexit git_atexit
+
 #endif
 
 int start_async(struct async *async)
@@ -682,6 +721,7 @@ int start_async(struct async *async)
 			close(fdin[1]);
 		if (need_out)
 			close(fdout[0]);
+		git_atexit_clear();
 		exit(!!async->proc(proc_in, proc_out, async->data));
 	}
 
diff --git a/shallow.c b/shallow.c
index de07709..f067811 100644
--- a/shallow.c
+++ b/shallow.c
@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)
 
 const char *setup_temporary_shallow(const struct sha1_array *extra)
 {
-	static int installed_handler;
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
 		strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX"));
 		fd = xmkstemp(temporary_shallow.buf);
 
-		if (!installed_handler) {
-			atexit(remove_temporary_shallow);
-			sigchain_push_common(remove_temporary_shallow_on_signal);
-		}
+		atexit(remove_temporary_shallow);
+		sigchain_push_common(remove_temporary_shallow_on_signal);
 
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
 			die_errno("failed to write to %s",
-- 
2.0.4
Junio C Hamano· Oct 13, 2014, 20:00 UTC · re: Etienne Buira · lore

Re: [PATCH v4] Handle atexit list internaly for unthreaded builds

Etienne Buira <etienne.buira@gmail.com> writes:
Show 7 quoted lines
> Wrap atexit()s calls on unthreaded builds to handle callback list
> internally.
>
> This is needed because on unthreaded builds, asyncs inherits parent's
> atexit() list, that gets run as soon as the async exit()s (and again at
> the end of the parent process). That led to remove temporary and lock
> files too early.

... that does not explain what you did to builtin/clone.c at all. Care to enlighten us, please?

Show 5 quoted lines
>
> Fixes test 5537 (temporary shallow file vanished before unpack-objects
> could open it)
>
> V4: fix bogus preprocessor directives

Please do not write this "V4:" line in the log message. People who read "git log" output and find this description would not know anything about the previous faulty ones. Putting it _after_ the three-dash line below will help the reviewers a lot.

>
> Thanks-to: Duy Nguyen <pclouds@gmail.com>
> Thanks-to: Andreas Schwab <schwab@linux-m68k.org>
Usually we spell these "Helped-by: " instead.
> Signed-off-by: Etienne Buira <etienne.buira@gmail.com>
> ---
Thanks.
Show 135 quoted lines
>  builtin/clone.c   |  5 -----
>  git-compat-util.h |  5 +++++
>  run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++
>  shallow.c         |  7 ++-----
>  4 files changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index bbd169c..e122f33 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)
>  
>  static const char *junk_work_tree;
>  static const char *junk_git_dir;
> -static pid_t junk_pid;
>  static enum {
>  	JUNK_LEAVE_NONE,
>  	JUNK_LEAVE_REPO,
> @@ -417,8 +416,6 @@ static void remove_junk(void)
>  		break;
>  	}
>  
> -	if (getpid() != junk_pid)
> -		return;
>  	if (junk_git_dir) {
>  		strbuf_addstr(&sb, junk_git_dir);
>  		remove_dir_recursively(&sb, 0);
> @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	struct refspec *refspec;
>  	const char *fetch_pattern;
>  
> -	junk_pid = getpid();
> -
>  	packet_trace_identity("clone");
>  	argc = parse_options(argc, argv, prefix, builtin_clone_options,
>  			     builtin_clone_usage, 0);
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f587749..6dd63dd 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);
>  const char *inet_ntop(int af, const void *src, char *dst, size_t size);
>  #endif
>  
> +#ifdef NO_PTHREADS
> +#define atexit git_atexit
> +extern int git_atexit(void (*handler)(void));
> +#endif
> +
>  extern void release_pack_memory(size_t);
>  
>  typedef void (*try_to_free_t)(size_t);
> diff --git a/run-command.c b/run-command.c
> index 35a3ebf..0f9a9b0 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -624,6 +624,45 @@ static int async_die_is_recursing(void)
>  	return ret != NULL;
>  }
>  
> +#else
> +
> +static struct {
> +	void (**handlers)(void);
> +	size_t nr;
> +	size_t alloc;
> +} git_atexit_hdlrs;
> +
> +static int git_atexit_installed;
> +
> +static void git_atexit_dispatch()
> +{
> +	size_t i;
> +
> +	for (i=git_atexit_hdlrs.nr ; i ; i--)
> +		git_atexit_hdlrs.handlers[i-1]();
> +}
> +
> +static void git_atexit_clear()
> +{
> +	free(git_atexit_hdlrs.handlers);
> +	memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));
> +	git_atexit_installed = 0;
> +}
> +
> +#undef atexit
> +int git_atexit(void (*handler)(void))
> +{
> +	ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
> +	git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;
> +	if (!git_atexit_installed) {
> +		if (atexit(&git_atexit_dispatch))
> +			return -1;
> +		git_atexit_installed = 1;
> +	}
> +	return 0;
> +}
> +#define atexit git_atexit
> +
>  #endif
>  
>  int start_async(struct async *async)
> @@ -682,6 +721,7 @@ int start_async(struct async *async)
>  			close(fdin[1]);
>  		if (need_out)
>  			close(fdout[0]);
> +		git_atexit_clear();
>  		exit(!!async->proc(proc_in, proc_out, async->data));
>  	}
>  
> diff --git a/shallow.c b/shallow.c
> index de07709..f067811 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)
>  
>  const char *setup_temporary_shallow(const struct sha1_array *extra)
>  {
> -	static int installed_handler;
>  	struct strbuf sb = STRBUF_INIT;
>  	int fd;
>  
> @@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
>  		strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX"));
>  		fd = xmkstemp(temporary_shallow.buf);
>  
> -		if (!installed_handler) {
> -			atexit(remove_temporary_shallow);
> -			sigchain_push_common(remove_temporary_shallow_on_signal);
> -		}
> +		atexit(remove_temporary_shallow);
> +		sigchain_push_common(remove_temporary_shallow_on_signal);
>  
>  		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
>  			die_errno("failed to write to %s",
Etienne Buira· Oct 14, 2014, 15:02 UTC · re: Junio C Hamano · lore

Re: [PATCH v4] Handle atexit list internaly for unthreaded builds

On Mon, Oct 13, 2014 at 10:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> Etienne Buira <etienne.buira@gmail.com> writes:
>
>> Wrap atexit()s calls on unthreaded builds to handle callback list
>> internally.
>>
>> This is needed because on unthreaded builds, asyncs inherits parent's
>> atexit() list, that gets run as soon as the async exit()s (and again at
>> the end of the parent process). That led to remove temporary and lock
>> files too early.
>
> ... that does not explain what you did to builtin/clone.c at all.
> Care to enlighten us, please?
Checking current pid against the one that registered the atexit()
routine (what builtin/clone.c currently does) is another way to guard
against the same issue, but it needs to store a pid for each atexit()
call whenever code called after might use async.
From what I've seen, two out of all atexit() calls were guarded like that:
- builtin/clone.c:cmd_clone
- lockfile.c:lock_file (overlooked it at first, would you mind to
s/temporary and lock files/temporary files/ the commit message if no
other round is needed? I'll do it otherwise).

So the changes in builtin/clone.c are just there because the patch makes this check redundant (still needed in lockfile.c, as it loops over a list that might refer to parent process's lockfiles).

Show 156 quoted lines
>> Fixes test 5537 (temporary shallow file vanished before unpack-objects
>> could open it)
>>
>> V4: fix bogus preprocessor directives
>
> Please do not write this "V4:" line in the log message.  People who
> read "git log" output and find this description would not know
> anything about the previous faulty ones.  Putting it _after_ the
> three-dash line below will help the reviewers a lot.
>
>>
>> Thanks-to: Duy Nguyen <pclouds@gmail.com>
>> Thanks-to: Andreas Schwab <schwab@linux-m68k.org>
>
> Usually we spell these "Helped-by: " instead.
>
>> Signed-off-by: Etienne Buira <etienne.buira@gmail.com>
>> ---
>
> Thanks.
>
>>  builtin/clone.c   |  5 -----
>>  git-compat-util.h |  5 +++++
>>  run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++
>>  shallow.c         |  7 ++-----
>>  4 files changed, 47 insertions(+), 10 deletions(-)
>>
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index bbd169c..e122f33 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)
>>
>>  static const char *junk_work_tree;
>>  static const char *junk_git_dir;
>> -static pid_t junk_pid;
>>  static enum {
>>       JUNK_LEAVE_NONE,
>>       JUNK_LEAVE_REPO,
>> @@ -417,8 +416,6 @@ static void remove_junk(void)
>>               break;
>>       }
>>
>> -     if (getpid() != junk_pid)
>> -             return;
>>       if (junk_git_dir) {
>>               strbuf_addstr(&sb, junk_git_dir);
>>               remove_dir_recursively(&sb, 0);
>> @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>>       struct refspec *refspec;
>>       const char *fetch_pattern;
>>
>> -     junk_pid = getpid();
>> -
>>       packet_trace_identity("clone");
>>       argc = parse_options(argc, argv, prefix, builtin_clone_options,
>>                            builtin_clone_usage, 0);
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index f587749..6dd63dd 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);
>>  const char *inet_ntop(int af, const void *src, char *dst, size_t size);
>>  #endif
>>
>> +#ifdef NO_PTHREADS
>> +#define atexit git_atexit
>> +extern int git_atexit(void (*handler)(void));
>> +#endif
>> +
>>  extern void release_pack_memory(size_t);
>>
>>  typedef void (*try_to_free_t)(size_t);
>> diff --git a/run-command.c b/run-command.c
>> index 35a3ebf..0f9a9b0 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -624,6 +624,45 @@ static int async_die_is_recursing(void)
>>       return ret != NULL;
>>  }
>>
>> +#else
>> +
>> +static struct {
>> +     void (**handlers)(void);
>> +     size_t nr;
>> +     size_t alloc;
>> +} git_atexit_hdlrs;
>> +
>> +static int git_atexit_installed;
>> +
>> +static void git_atexit_dispatch()
>> +{
>> +     size_t i;
>> +
>> +     for (i=git_atexit_hdlrs.nr ; i ; i--)
>> +             git_atexit_hdlrs.handlers[i-1]();
>> +}
>> +
>> +static void git_atexit_clear()
>> +{
>> +     free(git_atexit_hdlrs.handlers);
>> +     memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));
>> +     git_atexit_installed = 0;
>> +}
>> +
>> +#undef atexit
>> +int git_atexit(void (*handler)(void))
>> +{
>> +     ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
>> +     git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;
>> +     if (!git_atexit_installed) {
>> +             if (atexit(&git_atexit_dispatch))
>> +                     return -1;
>> +             git_atexit_installed = 1;
>> +     }
>> +     return 0;
>> +}
>> +#define atexit git_atexit
>> +
>>  #endif
>>
>>  int start_async(struct async *async)
>> @@ -682,6 +721,7 @@ int start_async(struct async *async)
>>                       close(fdin[1]);
>>               if (need_out)
>>                       close(fdout[0]);
>> +             git_atexit_clear();
>>               exit(!!async->proc(proc_in, proc_out, async->data));
>>       }
>>
>> diff --git a/shallow.c b/shallow.c
>> index de07709..f067811 100644
>> --- a/shallow.c
>> +++ b/shallow.c
>> @@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)
>>
>>  const char *setup_temporary_shallow(const struct sha1_array *extra)
>>  {
>> -     static int installed_handler;
>>       struct strbuf sb = STRBUF_INIT;
>>       int fd;
>>
>> @@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
>>               strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX"));
>>               fd = xmkstemp(temporary_shallow.buf);
>>
>> -             if (!installed_handler) {
>> -                     atexit(remove_temporary_shallow);
>> -                     sigchain_push_common(remove_temporary_shallow_on_signal);
>> -             }
>> +             atexit(remove_temporary_shallow);
>> +             sigchain_push_common(remove_temporary_shallow_on_signal);
>>
>>               if (write_in_full(fd, sb.buf, sb.len) != sb.len)
>>                       die_errno("failed to write to %s",
Etienne Buira· Oct 18, 2014, 12:31 UTC · re: Etienne Buira · lore

[PATCH v5] Handle atexit list internaly for unthreaded builds

Wrap atexit()s calls on unthreaded builds to handle callback list internally.

This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of async's parent process). That led to remove temporary files too early.

Also remove a by-atexit-callback guard against this kind of issue in clone.c, as this patch makes it redundant.

Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it)

BTW remove an unused variable in shallow.c.
Helped-by: Duy Nguyen <pclouds@gmail.com>
Helped-by: Andreas Schwab <schwab@linux-m68k.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Etienne Buira <etienne.buira@gmail.com>
---
 builtin/clone.c   |  5 -----
 git-compat-util.h |  5 +++++
 run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++
 shallow.c         |  7 ++-----
 4 files changed, 47 insertions(+), 10 deletions(-)
V5: update commit message
Show changes to 4 files +47 −10

builtin/clone.c, git-compat-util.h, run-command.c, shallow.c

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..e122f33 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)
 
 static const char *junk_work_tree;
 static const char *junk_git_dir;
-static pid_t junk_pid;
 static enum {
 	JUNK_LEAVE_NONE,
 	JUNK_LEAVE_REPO,
@@ -417,8 +416,6 @@ static void remove_junk(void)
 		break;
 	}
 
-	if (getpid() != junk_pid)
-		return;
 	if (junk_git_dir) {
 		strbuf_addstr(&sb, junk_git_dir);
 		remove_dir_recursively(&sb, 0);
@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct refspec *refspec;
 	const char *fetch_pattern;
 
-	junk_pid = getpid();
-
 	packet_trace_identity("clone");
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
diff --git a/git-compat-util.h b/git-compat-util.h
index f587749..6dd63dd 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);
 const char *inet_ntop(int af, const void *src, char *dst, size_t size);
 #endif
 
+#ifdef NO_PTHREADS
+#define atexit git_atexit
+extern int git_atexit(void (*handler)(void));
+#endif
+
 extern void release_pack_memory(size_t);
 
 typedef void (*try_to_free_t)(size_t);
diff --git a/run-command.c b/run-command.c
index 35a3ebf..0f9a9b0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -624,6 +624,45 @@ static int async_die_is_recursing(void)
 	return ret != NULL;
 }
 
+#else
+
+static struct {
+	void (**handlers)(void);
+	size_t nr;
+	size_t alloc;
+} git_atexit_hdlrs;
+
+static int git_atexit_installed;
+
+static void git_atexit_dispatch()
+{
+	size_t i;
+
+	for (i=git_atexit_hdlrs.nr ; i ; i--)
+		git_atexit_hdlrs.handlers[i-1]();
+}
+
+static void git_atexit_clear()
+{
+	free(git_atexit_hdlrs.handlers);
+	memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));
+	git_atexit_installed = 0;
+}
+
+#undef atexit
+int git_atexit(void (*handler)(void))
+{
+	ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);
+	git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;
+	if (!git_atexit_installed) {
+		if (atexit(&git_atexit_dispatch))
+			return -1;
+		git_atexit_installed = 1;
+	}
+	return 0;
+}
+#define atexit git_atexit
+
 #endif
 
 int start_async(struct async *async)
@@ -682,6 +721,7 @@ int start_async(struct async *async)
 			close(fdin[1]);
 		if (need_out)
 			close(fdout[0]);
+		git_atexit_clear();
 		exit(!!async->proc(proc_in, proc_out, async->data));
 	}
 
diff --git a/shallow.c b/shallow.c
index de07709..f067811 100644
--- a/shallow.c
+++ b/shallow.c
@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)
 
 const char *setup_temporary_shallow(const struct sha1_array *extra)
 {
-	static int installed_handler;
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
 		strbuf_addstr(&temporary_shallow, git_path("shallow_XXXXXX"));
 		fd = xmkstemp(temporary_shallow.buf);
 
-		if (!installed_handler) {
-			atexit(remove_temporary_shallow);
-			sigchain_push_common(remove_temporary_shallow_on_signal);
-		}
+		atexit(remove_temporary_shallow);
+		sigchain_push_common(remove_temporary_shallow_on_signal);
 
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
 			die_errno("failed to write to %s",
-- 
2.0.4

← back to recent threads