{"thread":{"id":"37713","subject":"[PATCH] Handle atexit list internaly fo unthreaded builds","startedAt":"2014-10-11T14:53:38Z","lastAt":"2014-10-18T12:31:15Z","messageCount":10,"participants":["Etienne Buira","Duy Nguyen","Andreas Schwab","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"250510","messageId":"1413039218-12139-1-git-send-email-etienne.buira@gmail.com","threadId":"37713","inReplyTo":null,"subject":"[PATCH] Handle atexit list internaly fo unthreaded builds","fromName":"Etienne Buira","fromEmail":"etienne.buira@gmail.com","sentAt":"2014-10-11T14:53:38Z","receivedAt":"2014-10-11T14:53:38Z","isPatch":true,"sender":{"key":"etienne.buira@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11487810?v=4"},"body":"Replace atexit()s calls with cmd_atexit that is atexit() on threaded\nbuilds, but handles the callbacks list internally for unthreaded builds.\n\nThis is needed because on unthreaded builds, asyncs inherits parent's\natexit() list, that gets run as soon as the async exit()s (and again at\nthe end of the parent process). That led to remove temporary and lock\nfiles too early.\n\nFixes test 5537 (temporary shallow file vanished before unpack-objects\ncould open it)\n\nSigned-off-by: Etienne Buira <etienne.buira@gmail.com>\n---\n builtin/clone.c |  7 +------\n builtin/fetch.c |  2 +-\n builtin/gc.c    |  2 +-\n diff.c          |  2 +-\n lockfile.c      |  2 +-\n pager.c         |  2 +-\n read-cache.c    |  2 +-\n run-command.c   | 45 ++++++++++++++++++++++++++++++++++++++++++++-\n shallow.c       |  7 ++-----\n trace.c         |  2 +-\n 10 files changed, 54 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex bbd169c..2992ac0 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \n static const char *junk_work_tree;\n static const char *junk_git_dir;\n-static pid_t junk_pid;\n static enum {\n \tJUNK_LEAVE_NONE,\n \tJUNK_LEAVE_REPO,\n@@ -417,8 +416,6 @@ static void remove_junk(void)\n \t\tbreak;\n \t}\n \n-\tif (getpid() != junk_pid)\n-\t\treturn;\n \tif (junk_git_dir) {\n \t\tstrbuf_addstr(&sb, junk_git_dir);\n \t\tremove_dir_recursively(&sb, 0);\n@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct refspec *refspec;\n \tconst char *fetch_pattern;\n \n-\tjunk_pid = getpid();\n-\n \tpacket_trace_identity(\"clone\");\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t     builtin_clone_usage, 0);\n@@ -843,7 +838,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tset_git_work_tree(work_tree);\n \t}\n \tjunk_git_dir = git_dir;\n-\tatexit(remove_junk);\n+\tcmd_atexit(remove_junk);\n \tsigchain_push_common(remove_junk_on_signal);\n \n \tif (safe_create_leading_directories_const(git_dir) < 0)\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 159fb7e..1e44d58 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1095,7 +1095,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)\n \t}\n \n \tsigchain_push_common(unlock_pack_on_signal);\n-\tatexit(unlock_pack);\n+\tcmd_atexit(unlock_pack);\n \trefspec = parse_fetch_refspec(ref_nr, refs);\n \texit_code = do_fetch(gtransport, refspec, ref_nr);\n \tfree_refspec(ref_nr, refspec);\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 8d219d8..cf2defa 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -254,7 +254,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n \n \tpidfile = git_pathdup(\"gc.pid\");\n \tsigchain_push_common(remove_pidfile_on_signal);\n-\tatexit(remove_pidfile);\n+\tcmd_atexit(remove_pidfile);\n \n \treturn NULL;\n }\ndiff --git a/diff.c b/diff.c\nindex 867f034..9c6ef9a 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2833,7 +2833,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,\n \t}\n \n \tif (!remove_tempfile_installed) {\n-\t\tatexit(remove_tempfile);\n+\t\tcmd_atexit(remove_tempfile);\n \t\tsigchain_push_common(remove_tempfile_on_signal);\n \t\tremove_tempfile_installed = 1;\n \t}\ndiff --git a/lockfile.c b/lockfile.c\nindex 2564a7f..ad0d1e2 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -141,7 +141,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \tif (0 <= lk->fd) {\n \t\tif (!lock_file_list) {\n \t\t\tsigchain_push_common(remove_lock_file_on_signal);\n-\t\t\tatexit(remove_lock_file);\n+\t\t\tcmd_atexit(remove_lock_file);\n \t\t}\n \t\tlk->owner = getpid();\n \t\tif (!lk->on_list) {\ndiff --git a/pager.c b/pager.c\nindex 8b5cbc5..09ab2fa 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -102,7 +102,7 @@ void setup_pager(void)\n \n \t/* this makes sure that the parent terminates after the pager */\n \tsigchain_push_common(wait_for_pager_signal);\n-\tatexit(wait_for_pager);\n+\tcmd_atexit(wait_for_pager);\n }\n \n int pager_in_use(void)\ndiff --git a/read-cache.c b/read-cache.c\nindex 6f0057f..8b10c92 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2104,7 +2104,7 @@ static int write_shared_index(struct index_state *istate,\n \t\treturn do_write_locked_index(istate, lock, flags);\n \t}\n \tif (!installed_handler) {\n-\t\tatexit(remove_temporary_sharedindex);\n+\t\tcmd_atexit(remove_temporary_sharedindex);\n \t\tsigchain_push_common(remove_temporary_sharedindex_on_signal);\n \t}\n \tmove_cache_to_base_index(istate);\ndiff --git a/run-command.c b/run-command.c\nindex 35a3ebf..5df57b5 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -45,7 +45,7 @@ static void mark_child_for_cleanup(pid_t pid)\n \tchildren_to_clean = p;\n \n \tif (!installed_child_cleanup_handler) {\n-\t\tatexit(cleanup_children_on_exit);\n+\t\tcmd_atexit(cleanup_children_on_exit);\n \t\tsigchain_push_common(cleanup_children_on_signal);\n \t\tinstalled_child_cleanup_handler = 1;\n \t}\n@@ -624,6 +624,48 @@ static int async_die_is_recursing(void)\n \treturn ret != NULL;\n }\n \n+int cmd_atexit(void (*handler)(void))\n+{\n+\treturn atexit(handler);\n+}\n+\n+#else\n+\n+static struct {\n+\tvoid (**handlers)(void);\n+\tsize_t nr;\n+\tsize_t alloc;\n+} cmd_atexit_hdlrs;\n+\n+static int cmd_atexit_installed;\n+\n+static void cmd_atexit_dispatch()\n+{\n+\tsize_t i;\n+\n+\tfor (i=cmd_atexit_hdlrs.nr ; i ; i--)\n+\t\tcmd_atexit_hdlrs.handlers[i-1]();\n+}\n+\n+static void cmd_atexit_clear()\n+{\n+\tfree(cmd_atexit_hdlrs.handlers);\n+\tmemset(&cmd_atexit_hdlrs, 0, sizeof(cmd_atexit_hdlrs));\n+\tcmd_atexit_installed = 0;\n+}\n+\n+int cmd_atexit(void (*handler)(void))\n+{\n+\tALLOC_GROW(cmd_atexit_hdlrs.handlers, cmd_atexit_hdlrs.nr + 1, cmd_atexit_hdlrs.alloc);\n+\tcmd_atexit_hdlrs.handlers[cmd_atexit_hdlrs.nr++] = handler;\n+\tif (!cmd_atexit_installed) {\n+\t\tif (!atexit(&cmd_atexit_dispatch))\n+\t\t\treturn -1;\n+\t\tcmd_atexit_installed = 1;\n+\t}\n+\treturn 0;\n+}\n+\n #endif\n \n int start_async(struct async *async)\n@@ -682,6 +724,7 @@ int start_async(struct async *async)\n \t\t\tclose(fdin[1]);\n \t\tif (need_out)\n \t\t\tclose(fdout[0]);\n+\t\tcmd_atexit_clear();\n \t\texit(!!async->proc(proc_in, proc_out, async->data));\n \t}\n \ndiff --git a/shallow.c b/shallow.c\nindex de07709..881580b 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)\n \n const char *setup_temporary_shallow(const struct sha1_array *extra)\n {\n-\tstatic int installed_handler;\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fd;\n \n@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)\n \t\tstrbuf_addstr(&temporary_shallow, git_path(\"shallow_XXXXXX\"));\n \t\tfd = xmkstemp(temporary_shallow.buf);\n \n-\t\tif (!installed_handler) {\n-\t\t\tatexit(remove_temporary_shallow);\n-\t\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n-\t\t}\n+\t\tcmd_atexit(remove_temporary_shallow);\n+\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n \n \t\tif (write_in_full(fd, sb.buf, sb.len) != sb.len)\n \t\t\tdie_errno(\"failed to write to %s\",\ndiff --git a/trace.c b/trace.c\nindex e583dc6..36c5076 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -420,7 +420,7 @@ void trace_command_performance(const char **argv)\n \t\treturn;\n \n \tif (!command_start_time)\n-\t\tatexit(print_command_performance_atexit);\n+\t\tcmd_atexit(print_command_performance_atexit);\n \n \tstrbuf_reset(&command_line);\n \tsq_quote_argv(&command_line, argv, 0);\n-- \n1.8.5.5\n"},{"id":"250528","messageId":"1413104974-3861-1-git-send-email-etienne.buira@gmail.com","threadId":"37713","inReplyTo":"1413039218-12139-1-git-send-email-etienne.buira@gmail.com","subject":"[PATCH v2] Handle atexit list internaly fo unthreaded builds","fromName":"Etienne Buira","fromEmail":"etienne.buira@gmail.com","sentAt":"2014-10-12T09:09:34Z","receivedAt":"2014-10-12T09:09:34Z","isPatch":true,"sender":{"key":"etienne.buira@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11487810?v=4"},"body":"Replace atexit()s calls with cmd_atexit that is atexit() on threaded\nbuilds, but handles the callbacks list internally for unthreaded builds.\n\nThis is needed because on unthreaded builds, asyncs inherits parent's\natexit() list, that gets run as soon as the async exit()s (and again at\nthe end of the parent process). That led to remove temporary and lock\nfiles too early.\n\nFixes test 5537 (temporary shallow file vanished before unpack-objects\ncould open it)\n\nV2: remove an obvious mistake\n\nSigned-off-by: Etienne Buira <etienne.buira@gmail.com>\n---\n builtin/clone.c |  7 +------\n builtin/fetch.c |  2 +-\n builtin/gc.c    |  2 +-\n diff.c          |  2 +-\n lockfile.c      |  2 +-\n pager.c         |  2 +-\n read-cache.c    |  2 +-\n run-command.c   | 45 ++++++++++++++++++++++++++++++++++++++++++++-\n shallow.c       |  7 ++-----\n trace.c         |  2 +-\n 10 files changed, 54 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex bbd169c..2992ac0 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \n static const char *junk_work_tree;\n static const char *junk_git_dir;\n-static pid_t junk_pid;\n static enum {\n \tJUNK_LEAVE_NONE,\n \tJUNK_LEAVE_REPO,\n@@ -417,8 +416,6 @@ static void remove_junk(void)\n \t\tbreak;\n \t}\n \n-\tif (getpid() != junk_pid)\n-\t\treturn;\n \tif (junk_git_dir) {\n \t\tstrbuf_addstr(&sb, junk_git_dir);\n \t\tremove_dir_recursively(&sb, 0);\n@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct refspec *refspec;\n \tconst char *fetch_pattern;\n \n-\tjunk_pid = getpid();\n-\n \tpacket_trace_identity(\"clone\");\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t     builtin_clone_usage, 0);\n@@ -843,7 +838,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tset_git_work_tree(work_tree);\n \t}\n \tjunk_git_dir = git_dir;\n-\tatexit(remove_junk);\n+\tcmd_atexit(remove_junk);\n \tsigchain_push_common(remove_junk_on_signal);\n \n \tif (safe_create_leading_directories_const(git_dir) < 0)\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 159fb7e..1e44d58 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1095,7 +1095,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)\n \t}\n \n \tsigchain_push_common(unlock_pack_on_signal);\n-\tatexit(unlock_pack);\n+\tcmd_atexit(unlock_pack);\n \trefspec = parse_fetch_refspec(ref_nr, refs);\n \texit_code = do_fetch(gtransport, refspec, ref_nr);\n \tfree_refspec(ref_nr, refspec);\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 8d219d8..cf2defa 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -254,7 +254,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n \n \tpidfile = git_pathdup(\"gc.pid\");\n \tsigchain_push_common(remove_pidfile_on_signal);\n-\tatexit(remove_pidfile);\n+\tcmd_atexit(remove_pidfile);\n \n \treturn NULL;\n }\ndiff --git a/diff.c b/diff.c\nindex 867f034..9c6ef9a 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2833,7 +2833,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,\n \t}\n \n \tif (!remove_tempfile_installed) {\n-\t\tatexit(remove_tempfile);\n+\t\tcmd_atexit(remove_tempfile);\n \t\tsigchain_push_common(remove_tempfile_on_signal);\n \t\tremove_tempfile_installed = 1;\n \t}\ndiff --git a/lockfile.c b/lockfile.c\nindex 2564a7f..ad0d1e2 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -141,7 +141,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)\n \tif (0 <= lk->fd) {\n \t\tif (!lock_file_list) {\n \t\t\tsigchain_push_common(remove_lock_file_on_signal);\n-\t\t\tatexit(remove_lock_file);\n+\t\t\tcmd_atexit(remove_lock_file);\n \t\t}\n \t\tlk->owner = getpid();\n \t\tif (!lk->on_list) {\ndiff --git a/pager.c b/pager.c\nindex 8b5cbc5..09ab2fa 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -102,7 +102,7 @@ void setup_pager(void)\n \n \t/* this makes sure that the parent terminates after the pager */\n \tsigchain_push_common(wait_for_pager_signal);\n-\tatexit(wait_for_pager);\n+\tcmd_atexit(wait_for_pager);\n }\n \n int pager_in_use(void)\ndiff --git a/read-cache.c b/read-cache.c\nindex 6f0057f..8b10c92 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2104,7 +2104,7 @@ static int write_shared_index(struct index_state *istate,\n \t\treturn do_write_locked_index(istate, lock, flags);\n \t}\n \tif (!installed_handler) {\n-\t\tatexit(remove_temporary_sharedindex);\n+\t\tcmd_atexit(remove_temporary_sharedindex);\n \t\tsigchain_push_common(remove_temporary_sharedindex_on_signal);\n \t}\n \tmove_cache_to_base_index(istate);\ndiff --git a/run-command.c b/run-command.c\nindex 35a3ebf..a8a6374 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -45,7 +45,7 @@ static void mark_child_for_cleanup(pid_t pid)\n \tchildren_to_clean = p;\n \n \tif (!installed_child_cleanup_handler) {\n-\t\tatexit(cleanup_children_on_exit);\n+\t\tcmd_atexit(cleanup_children_on_exit);\n \t\tsigchain_push_common(cleanup_children_on_signal);\n \t\tinstalled_child_cleanup_handler = 1;\n \t}\n@@ -624,6 +624,48 @@ static int async_die_is_recursing(void)\n \treturn ret != NULL;\n }\n \n+int cmd_atexit(void (*handler)(void))\n+{\n+\treturn atexit(handler);\n+}\n+\n+#else\n+\n+static struct {\n+\tvoid (**handlers)(void);\n+\tsize_t nr;\n+\tsize_t alloc;\n+} cmd_atexit_hdlrs;\n+\n+static int cmd_atexit_installed;\n+\n+static void cmd_atexit_dispatch()\n+{\n+\tsize_t i;\n+\n+\tfor (i=cmd_atexit_hdlrs.nr ; i ; i--)\n+\t\tcmd_atexit_hdlrs.handlers[i-1]();\n+}\n+\n+static void cmd_atexit_clear()\n+{\n+\tfree(cmd_atexit_hdlrs.handlers);\n+\tmemset(&cmd_atexit_hdlrs, 0, sizeof(cmd_atexit_hdlrs));\n+\tcmd_atexit_installed = 0;\n+}\n+\n+int cmd_atexit(void (*handler)(void))\n+{\n+\tALLOC_GROW(cmd_atexit_hdlrs.handlers, cmd_atexit_hdlrs.nr + 1, cmd_atexit_hdlrs.alloc);\n+\tcmd_atexit_hdlrs.handlers[cmd_atexit_hdlrs.nr++] = handler;\n+\tif (!cmd_atexit_installed) {\n+\t\tif (atexit(&cmd_atexit_dispatch))\n+\t\t\treturn -1;\n+\t\tcmd_atexit_installed = 1;\n+\t}\n+\treturn 0;\n+}\n+\n #endif\n \n int start_async(struct async *async)\n@@ -682,6 +724,7 @@ int start_async(struct async *async)\n \t\t\tclose(fdin[1]);\n \t\tif (need_out)\n \t\t\tclose(fdout[0]);\n+\t\tcmd_atexit_clear();\n \t\texit(!!async->proc(proc_in, proc_out, async->data));\n \t}\n \ndiff --git a/shallow.c b/shallow.c\nindex de07709..881580b 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)\n \n const char *setup_temporary_shallow(const struct sha1_array *extra)\n {\n-\tstatic int installed_handler;\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fd;\n \n@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)\n \t\tstrbuf_addstr(&temporary_shallow, git_path(\"shallow_XXXXXX\"));\n \t\tfd = xmkstemp(temporary_shallow.buf);\n \n-\t\tif (!installed_handler) {\n-\t\t\tatexit(remove_temporary_shallow);\n-\t\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n-\t\t}\n+\t\tcmd_atexit(remove_temporary_shallow);\n+\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n \n \t\tif (write_in_full(fd, sb.buf, sb.len) != sb.len)\n \t\t\tdie_errno(\"failed to write to %s\",\ndiff --git a/trace.c b/trace.c\nindex e583dc6..36c5076 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -420,7 +420,7 @@ void trace_command_performance(const char **argv)\n \t\treturn;\n \n \tif (!command_start_time)\n-\t\tatexit(print_command_performance_atexit);\n+\t\tcmd_atexit(print_command_performance_atexit);\n \n \tstrbuf_reset(&command_line);\n \tsq_quote_argv(&command_line, argv, 0);\n-- \n2.0.4\n"},{"id":"250534","messageId":"CACsJy8CbhS=dv3fHvyTv0b-jazh3XS+nswmz_0AsLeHqko794g@mail.gmail.com","threadId":"37713","inReplyTo":"1413104974-3861-1-git-send-email-etienne.buira@gmail.com","subject":"Re: [PATCH v2] Handle atexit list internaly fo unthreaded builds","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-10-13T00:56:05Z","receivedAt":"2014-10-13T00:56:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Oct 12, 2014 at 4:09 PM, Etienne Buira <etienne.buira@gmail.com> wrote:\n> Replace atexit()s calls with cmd_atexit that is atexit() on threaded\n> builds, but handles the callbacks list internally for unthreaded builds.\n\nMaybe hide this in git-compat-util.h and \"#define atexit(x)\ncmd_atexit(x)\"? cmd_ is usually for commands' \"main\" functions. Maybe\nrename it to git_atexit().\n-- \nDuy\n"},{"id":"250539","messageId":"1413213552-13769-1-git-send-email-etienne.buira@gmail.com","threadId":"37713","inReplyTo":"CACsJy8CbhS=dv3fHvyTv0b-jazh3XS+nswmz_0AsLeHqko794g@mail.gmail.com","subject":"Re: [PATCH v2] Handle atexit list internaly for unthreaded builds","fromName":"Etienne Buira","fromEmail":"etienne.buira@gmail.com","sentAt":"2014-10-13T15:19:11Z","receivedAt":"2014-10-13T15:19:11Z","isPatch":true,"sender":{"key":"etienne.buira@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11487810?v=4"},"body":"On Mon, Oct 13, 2014 at 2:56 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Sun, Oct 12, 2014 at 4:09 PM, Etienne Buira <etienne.buira@gmail.com> wrote:\n> > Replace atexit()s calls with cmd_atexit that is atexit() on threaded\n> > builds, but handles the callbacks list internally for unthreaded builds.\n> \n> Maybe hide this in git-compat-util.h and \"#define atexit(x)\n> cmd_atexit(x)\"?\n\nUpdated.\n\n> cmd_ is usually for commands' \"main\" functions. Maybe\n> rename it to git_atexit().\n\nIndeed, renamed. Thank you.\n"},{"id":"250540","messageId":"eb384a98a5c5642c4d3ccf0cf4b74b62e268e100.1413213400.git.etienne.buira@gmail.com","threadId":"37713","inReplyTo":"1413213552-13769-1-git-send-email-etienne.buira@gmail.com","subject":"[PATCH v3] Handle atexit list internaly for unthreaded builds","fromName":"Etienne Buira","fromEmail":"etienne.buira@gmail.com","sentAt":"2014-10-13T15:19:12Z","receivedAt":"2014-10-13T15:19:12Z","isPatch":true,"sender":{"key":"etienne.buira@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11487810?v=4"},"body":"Wrap atexit()s calls on unthreaded builds to handle callback list\ninternally.\n\nThis is needed because on unthreaded builds, asyncs inherits parent's\natexit() list, that gets run as soon as the async exit()s (and again at\nthe end of the parent process). That led to remove temporary and lock\nfiles too early.\n\nFixes test 5537 (temporary shallow file vanished before unpack-objects\ncould open it)\n\nV2: remove an obvious mistake\nV3: wrap, rename and add define in git-compat-util.h\n\nThanks-to: Duy Nguyen <pclouds@gmail.com>\nSigned-off-by: Etienne Buira <etienne.buira@gmail.com>\n---\n builtin/clone.c   |  5 -----\n git-compat-util.h |  5 +++++\n run-command.c     | 42 ++++++++++++++++++++++++++++++++++++++++++\n shallow.c         |  7 ++-----\n 4 files changed, 49 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex bbd169c..e122f33 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \n static const char *junk_work_tree;\n static const char *junk_git_dir;\n-static pid_t junk_pid;\n static enum {\n \tJUNK_LEAVE_NONE,\n \tJUNK_LEAVE_REPO,\n@@ -417,8 +416,6 @@ static void remove_junk(void)\n \t\tbreak;\n \t}\n \n-\tif (getpid() != junk_pid)\n-\t\treturn;\n \tif (junk_git_dir) {\n \t\tstrbuf_addstr(&sb, junk_git_dir);\n \t\tremove_dir_recursively(&sb, 0);\n@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct refspec *refspec;\n \tconst char *fetch_pattern;\n \n-\tjunk_pid = getpid();\n-\n \tpacket_trace_identity(\"clone\");\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t     builtin_clone_usage, 0);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f587749..6dd63dd 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);\n const char *inet_ntop(int af, const void *src, char *dst, size_t size);\n #endif\n \n+#ifdef NO_PTHREADS\n+#define atexit git_atexit\n+extern int git_atexit(void (*handler)(void));\n+#endif\n+\n extern void release_pack_memory(size_t);\n \n typedef void (*try_to_free_t)(size_t);\ndiff --git a/run-command.c b/run-command.c\nindex 35a3ebf..f8dc969 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -624,6 +624,47 @@ static int async_die_is_recursing(void)\n \treturn ret != NULL;\n }\n \n+#else\n+\n+static struct {\n+\tvoid (**handlers)(void);\n+\tsize_t nr;\n+\tsize_t alloc;\n+} git_atexit_hdlrs;\n+\n+static int git_atexit_installed;\n+\n+static void git_atexit_dispatch()\n+{\n+\tsize_t i;\n+\n+\tfor (i=git_atexit_hdlrs.nr ; i ; i--)\n+\t\tgit_atexit_hdlrs.handlers[i-1]();\n+}\n+\n+static void git_atexit_clear()\n+{\n+\tfree(git_atexit_hdlrs.handlers);\n+\tmemset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));\n+\tgit_atexit_installed = 0;\n+}\n+\n+#define tmp_atexit atexit\n+#undef atexit\n+int git_atexit(void (*handler)(void))\n+{\n+\tALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n+\tgit_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;\n+\tif (!git_atexit_installed) {\n+\t\tif (atexit(&git_atexit_dispatch))\n+\t\t\treturn -1;\n+\t\tgit_atexit_installed = 1;\n+\t}\n+\treturn 0;\n+}\n+#define atexit tmp_atexit\n+#undef tmp_atexit\n+\n #endif\n \n int start_async(struct async *async)\n@@ -682,6 +723,7 @@ int start_async(struct async *async)\n \t\t\tclose(fdin[1]);\n \t\tif (need_out)\n \t\t\tclose(fdout[0]);\n+\t\tgit_atexit_clear();\n \t\texit(!!async->proc(proc_in, proc_out, async->data));\n \t}\n \ndiff --git a/shallow.c b/shallow.c\nindex de07709..f067811 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)\n \n const char *setup_temporary_shallow(const struct sha1_array *extra)\n {\n-\tstatic int installed_handler;\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fd;\n \n@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)\n \t\tstrbuf_addstr(&temporary_shallow, git_path(\"shallow_XXXXXX\"));\n \t\tfd = xmkstemp(temporary_shallow.buf);\n \n-\t\tif (!installed_handler) {\n-\t\t\tatexit(remove_temporary_shallow);\n-\t\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n-\t\t}\n+\t\tatexit(remove_temporary_shallow);\n+\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n \n \t\tif (write_in_full(fd, sb.buf, sb.len) != sb.len)\n \t\t\tdie_errno(\"failed to write to %s\",\n-- \n2.0.4\n"},{"id":"250542","messageId":"87bnpg9b99.fsf@igel.home","threadId":"37713","inReplyTo":"eb384a98a5c5642c4d3ccf0cf4b74b62e268e100.1413213400.git.etienne.buira@gmail.com","subject":"Re: [PATCH v3] Handle atexit list internaly for unthreaded builds","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-10-13T16:37:22Z","receivedAt":"2014-10-13T16:37:22Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Etienne Buira <etienne.buira@gmail.com> writes:\n\n> +#define tmp_atexit atexit\n\n> +#define atexit tmp_atexit\n> +#undef tmp_atexit\n\nWhat is this supposed to do?\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"250561","messageId":"2724eb56a7219e0ed341e734beb32aa236119998.1413225018.git.etienne.buira@gmail.com","threadId":"37713","inReplyTo":"87bnpg9b99.fsf@igel.home","subject":"[PATCH v4] Handle atexit list internaly for unthreaded builds","fromName":"Etienne Buira","fromEmail":"etienne.buira@gmail.com","sentAt":"2014-10-13T18:35:58Z","receivedAt":"2014-10-13T18:35:58Z","isPatch":true,"sender":{"key":"etienne.buira@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11487810?v=4"},"body":"Wrap atexit()s calls on unthreaded builds to handle callback list\ninternally.\n\nThis is needed because on unthreaded builds, asyncs inherits parent's\natexit() list, that gets run as soon as the async exit()s (and again at\nthe end of the parent process). That led to remove temporary and lock\nfiles too early.\n\nFixes test 5537 (temporary shallow file vanished before unpack-objects\ncould open it)\n\nV4: fix bogus preprocessor directives\n\nThanks-to: Duy Nguyen <pclouds@gmail.com>\nThanks-to: Andreas Schwab <schwab@linux-m68k.org>\nSigned-off-by: Etienne Buira <etienne.buira@gmail.com>\n---\n builtin/clone.c   |  5 -----\n git-compat-util.h |  5 +++++\n run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++\n shallow.c         |  7 ++-----\n 4 files changed, 47 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex bbd169c..e122f33 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \n static const char *junk_work_tree;\n static const char *junk_git_dir;\n-static pid_t junk_pid;\n static enum {\n \tJUNK_LEAVE_NONE,\n \tJUNK_LEAVE_REPO,\n@@ -417,8 +416,6 @@ static void remove_junk(void)\n \t\tbreak;\n \t}\n \n-\tif (getpid() != junk_pid)\n-\t\treturn;\n \tif (junk_git_dir) {\n \t\tstrbuf_addstr(&sb, junk_git_dir);\n \t\tremove_dir_recursively(&sb, 0);\n@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct refspec *refspec;\n \tconst char *fetch_pattern;\n \n-\tjunk_pid = getpid();\n-\n \tpacket_trace_identity(\"clone\");\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t     builtin_clone_usage, 0);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f587749..6dd63dd 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);\n const char *inet_ntop(int af, const void *src, char *dst, size_t size);\n #endif\n \n+#ifdef NO_PTHREADS\n+#define atexit git_atexit\n+extern int git_atexit(void (*handler)(void));\n+#endif\n+\n extern void release_pack_memory(size_t);\n \n typedef void (*try_to_free_t)(size_t);\ndiff --git a/run-command.c b/run-command.c\nindex 35a3ebf..0f9a9b0 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -624,6 +624,45 @@ static int async_die_is_recursing(void)\n \treturn ret != NULL;\n }\n \n+#else\n+\n+static struct {\n+\tvoid (**handlers)(void);\n+\tsize_t nr;\n+\tsize_t alloc;\n+} git_atexit_hdlrs;\n+\n+static int git_atexit_installed;\n+\n+static void git_atexit_dispatch()\n+{\n+\tsize_t i;\n+\n+\tfor (i=git_atexit_hdlrs.nr ; i ; i--)\n+\t\tgit_atexit_hdlrs.handlers[i-1]();\n+}\n+\n+static void git_atexit_clear()\n+{\n+\tfree(git_atexit_hdlrs.handlers);\n+\tmemset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));\n+\tgit_atexit_installed = 0;\n+}\n+\n+#undef atexit\n+int git_atexit(void (*handler)(void))\n+{\n+\tALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n+\tgit_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;\n+\tif (!git_atexit_installed) {\n+\t\tif (atexit(&git_atexit_dispatch))\n+\t\t\treturn -1;\n+\t\tgit_atexit_installed = 1;\n+\t}\n+\treturn 0;\n+}\n+#define atexit git_atexit\n+\n #endif\n \n int start_async(struct async *async)\n@@ -682,6 +721,7 @@ int start_async(struct async *async)\n \t\t\tclose(fdin[1]);\n \t\tif (need_out)\n \t\t\tclose(fdout[0]);\n+\t\tgit_atexit_clear();\n \t\texit(!!async->proc(proc_in, proc_out, async->data));\n \t}\n \ndiff --git a/shallow.c b/shallow.c\nindex de07709..f067811 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)\n \n const char *setup_temporary_shallow(const struct sha1_array *extra)\n {\n-\tstatic int installed_handler;\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fd;\n \n@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)\n \t\tstrbuf_addstr(&temporary_shallow, git_path(\"shallow_XXXXXX\"));\n \t\tfd = xmkstemp(temporary_shallow.buf);\n \n-\t\tif (!installed_handler) {\n-\t\t\tatexit(remove_temporary_shallow);\n-\t\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n-\t\t}\n+\t\tatexit(remove_temporary_shallow);\n+\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n \n \t\tif (write_in_full(fd, sb.buf, sb.len) != sb.len)\n \t\t\tdie_errno(\"failed to write to %s\",\n-- \n2.0.4\n"},{"id":"250572","messageId":"xmqq7g033flb.fsf@gitster.dls.corp.google.com","threadId":"37713","inReplyTo":"2724eb56a7219e0ed341e734beb32aa236119998.1413225018.git.etienne.buira@gmail.com","subject":"Re: [PATCH v4] Handle atexit list internaly for unthreaded builds","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-13T20:00:16Z","receivedAt":"2014-10-13T20:00:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Etienne Buira <etienne.buira@gmail.com> writes:\n\n> Wrap atexit()s calls on unthreaded builds to handle callback list\n> internally.\n>\n> This is needed because on unthreaded builds, asyncs inherits parent's\n> atexit() list, that gets run as soon as the async exit()s (and again at\n> the end of the parent process). That led to remove temporary and lock\n> files too early.\n\n... that does not explain what you did to builtin/clone.c at all.\nCare to enlighten us, please?\n\n>\n> Fixes test 5537 (temporary shallow file vanished before unpack-objects\n> could open it)\n>\n> V4: fix bogus preprocessor directives\n\nPlease do not write this \"V4:\" line in the log message.  People who\nread \"git log\" output and find this description would not know\nanything about the previous faulty ones.  Putting it _after_ the\nthree-dash line below will help the reviewers a lot.\n\n>\n> Thanks-to: Duy Nguyen <pclouds@gmail.com>\n> Thanks-to: Andreas Schwab <schwab@linux-m68k.org>\n\nUsually we spell these \"Helped-by: \" instead.\n\n> Signed-off-by: Etienne Buira <etienne.buira@gmail.com>\n> ---\n\nThanks.\n\n>  builtin/clone.c   |  5 -----\n>  git-compat-util.h |  5 +++++\n>  run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++\n>  shallow.c         |  7 ++-----\n>  4 files changed, 47 insertions(+), 10 deletions(-)\n>\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index bbd169c..e122f33 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n>  \n>  static const char *junk_work_tree;\n>  static const char *junk_git_dir;\n> -static pid_t junk_pid;\n>  static enum {\n>  \tJUNK_LEAVE_NONE,\n>  \tJUNK_LEAVE_REPO,\n> @@ -417,8 +416,6 @@ static void remove_junk(void)\n>  \t\tbreak;\n>  \t}\n>  \n> -\tif (getpid() != junk_pid)\n> -\t\treturn;\n>  \tif (junk_git_dir) {\n>  \t\tstrbuf_addstr(&sb, junk_git_dir);\n>  \t\tremove_dir_recursively(&sb, 0);\n> @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n>  \tstruct refspec *refspec;\n>  \tconst char *fetch_pattern;\n>  \n> -\tjunk_pid = getpid();\n> -\n>  \tpacket_trace_identity(\"clone\");\n>  \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n>  \t\t\t     builtin_clone_usage, 0);\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index f587749..6dd63dd 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);\n>  const char *inet_ntop(int af, const void *src, char *dst, size_t size);\n>  #endif\n>  \n> +#ifdef NO_PTHREADS\n> +#define atexit git_atexit\n> +extern int git_atexit(void (*handler)(void));\n> +#endif\n> +\n>  extern void release_pack_memory(size_t);\n>  \n>  typedef void (*try_to_free_t)(size_t);\n> diff --git a/run-command.c b/run-command.c\n> index 35a3ebf..0f9a9b0 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -624,6 +624,45 @@ static int async_die_is_recursing(void)\n>  \treturn ret != NULL;\n>  }\n>  \n> +#else\n> +\n> +static struct {\n> +\tvoid (**handlers)(void);\n> +\tsize_t nr;\n> +\tsize_t alloc;\n> +} git_atexit_hdlrs;\n> +\n> +static int git_atexit_installed;\n> +\n> +static void git_atexit_dispatch()\n> +{\n> +\tsize_t i;\n> +\n> +\tfor (i=git_atexit_hdlrs.nr ; i ; i--)\n> +\t\tgit_atexit_hdlrs.handlers[i-1]();\n> +}\n> +\n> +static void git_atexit_clear()\n> +{\n> +\tfree(git_atexit_hdlrs.handlers);\n> +\tmemset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));\n> +\tgit_atexit_installed = 0;\n> +}\n> +\n> +#undef atexit\n> +int git_atexit(void (*handler)(void))\n> +{\n> +\tALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n> +\tgit_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;\n> +\tif (!git_atexit_installed) {\n> +\t\tif (atexit(&git_atexit_dispatch))\n> +\t\t\treturn -1;\n> +\t\tgit_atexit_installed = 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +#define atexit git_atexit\n> +\n>  #endif\n>  \n>  int start_async(struct async *async)\n> @@ -682,6 +721,7 @@ int start_async(struct async *async)\n>  \t\t\tclose(fdin[1]);\n>  \t\tif (need_out)\n>  \t\t\tclose(fdout[0]);\n> +\t\tgit_atexit_clear();\n>  \t\texit(!!async->proc(proc_in, proc_out, async->data));\n>  \t}\n>  \n> diff --git a/shallow.c b/shallow.c\n> index de07709..f067811 100644\n> --- a/shallow.c\n> +++ b/shallow.c\n> @@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)\n>  \n>  const char *setup_temporary_shallow(const struct sha1_array *extra)\n>  {\n> -\tstatic int installed_handler;\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \tint fd;\n>  \n> @@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)\n>  \t\tstrbuf_addstr(&temporary_shallow, git_path(\"shallow_XXXXXX\"));\n>  \t\tfd = xmkstemp(temporary_shallow.buf);\n>  \n> -\t\tif (!installed_handler) {\n> -\t\t\tatexit(remove_temporary_shallow);\n> -\t\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n> -\t\t}\n> +\t\tatexit(remove_temporary_shallow);\n> +\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n>  \n>  \t\tif (write_in_full(fd, sb.buf, sb.len) != sb.len)\n>  \t\t\tdie_errno(\"failed to write to %s\",\n"},{"id":"250612","messageId":"CAMcFKVyMOO4iTq_70fqd0Jjq4WNzz9YrGYNSCZmVx5F28KXicQ@mail.gmail.com","threadId":"37713","inReplyTo":"xmqq7g033flb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4] Handle atexit list internaly for unthreaded builds","fromName":"Etienne Buira","fromEmail":"etienne.buira@gmail.com","sentAt":"2014-10-14T15:02:27Z","receivedAt":"2014-10-14T15:02:27Z","isPatch":true,"sender":{"key":"etienne.buira@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11487810?v=4"},"body":"On Mon, Oct 13, 2014 at 10:00 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Etienne Buira <etienne.buira@gmail.com> writes:\n>\n>> Wrap atexit()s calls on unthreaded builds to handle callback list\n>> internally.\n>>\n>> This is needed because on unthreaded builds, asyncs inherits parent's\n>> atexit() list, that gets run as soon as the async exit()s (and again at\n>> the end of the parent process). That led to remove temporary and lock\n>> files too early.\n>\n> ... that does not explain what you did to builtin/clone.c at all.\n> Care to enlighten us, please?\n\nChecking current pid against the one that registered the atexit()\nroutine (what builtin/clone.c currently does) is another way to guard\nagainst the same issue, but it needs to store a pid for each atexit()\ncall whenever code called after might use async.\nFrom what I've seen, two out of all atexit() calls were guarded like that:\n- builtin/clone.c:cmd_clone\n- lockfile.c:lock_file (overlooked it at first, would you mind to\ns/temporary and lock files/temporary files/ the commit message if no\nother round is needed? I'll do it otherwise).\n\nSo the changes in builtin/clone.c are just there because the patch\nmakes this check redundant (still needed in lockfile.c, as it loops\nover a list that might refer to parent process's lockfiles).\n\n>> Fixes test 5537 (temporary shallow file vanished before unpack-objects\n>> could open it)\n>>\n>> V4: fix bogus preprocessor directives\n>\n> Please do not write this \"V4:\" line in the log message.  People who\n> read \"git log\" output and find this description would not know\n> anything about the previous faulty ones.  Putting it _after_ the\n> three-dash line below will help the reviewers a lot.\n>\n>>\n>> Thanks-to: Duy Nguyen <pclouds@gmail.com>\n>> Thanks-to: Andreas Schwab <schwab@linux-m68k.org>\n>\n> Usually we spell these \"Helped-by: \" instead.\n>\n>> Signed-off-by: Etienne Buira <etienne.buira@gmail.com>\n>> ---\n>\n> Thanks.\n>\n>>  builtin/clone.c   |  5 -----\n>>  git-compat-util.h |  5 +++++\n>>  run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++\n>>  shallow.c         |  7 ++-----\n>>  4 files changed, 47 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/builtin/clone.c b/builtin/clone.c\n>> index bbd169c..e122f33 100644\n>> --- a/builtin/clone.c\n>> +++ b/builtin/clone.c\n>> @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n>>\n>>  static const char *junk_work_tree;\n>>  static const char *junk_git_dir;\n>> -static pid_t junk_pid;\n>>  static enum {\n>>       JUNK_LEAVE_NONE,\n>>       JUNK_LEAVE_REPO,\n>> @@ -417,8 +416,6 @@ static void remove_junk(void)\n>>               break;\n>>       }\n>>\n>> -     if (getpid() != junk_pid)\n>> -             return;\n>>       if (junk_git_dir) {\n>>               strbuf_addstr(&sb, junk_git_dir);\n>>               remove_dir_recursively(&sb, 0);\n>> @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n>>       struct refspec *refspec;\n>>       const char *fetch_pattern;\n>>\n>> -     junk_pid = getpid();\n>> -\n>>       packet_trace_identity(\"clone\");\n>>       argc = parse_options(argc, argv, prefix, builtin_clone_options,\n>>                            builtin_clone_usage, 0);\n>> diff --git a/git-compat-util.h b/git-compat-util.h\n>> index f587749..6dd63dd 100644\n>> --- a/git-compat-util.h\n>> +++ b/git-compat-util.h\n>> @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);\n>>  const char *inet_ntop(int af, const void *src, char *dst, size_t size);\n>>  #endif\n>>\n>> +#ifdef NO_PTHREADS\n>> +#define atexit git_atexit\n>> +extern int git_atexit(void (*handler)(void));\n>> +#endif\n>> +\n>>  extern void release_pack_memory(size_t);\n>>\n>>  typedef void (*try_to_free_t)(size_t);\n>> diff --git a/run-command.c b/run-command.c\n>> index 35a3ebf..0f9a9b0 100644\n>> --- a/run-command.c\n>> +++ b/run-command.c\n>> @@ -624,6 +624,45 @@ static int async_die_is_recursing(void)\n>>       return ret != NULL;\n>>  }\n>>\n>> +#else\n>> +\n>> +static struct {\n>> +     void (**handlers)(void);\n>> +     size_t nr;\n>> +     size_t alloc;\n>> +} git_atexit_hdlrs;\n>> +\n>> +static int git_atexit_installed;\n>> +\n>> +static void git_atexit_dispatch()\n>> +{\n>> +     size_t i;\n>> +\n>> +     for (i=git_atexit_hdlrs.nr ; i ; i--)\n>> +             git_atexit_hdlrs.handlers[i-1]();\n>> +}\n>> +\n>> +static void git_atexit_clear()\n>> +{\n>> +     free(git_atexit_hdlrs.handlers);\n>> +     memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));\n>> +     git_atexit_installed = 0;\n>> +}\n>> +\n>> +#undef atexit\n>> +int git_atexit(void (*handler)(void))\n>> +{\n>> +     ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n>> +     git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;\n>> +     if (!git_atexit_installed) {\n>> +             if (atexit(&git_atexit_dispatch))\n>> +                     return -1;\n>> +             git_atexit_installed = 1;\n>> +     }\n>> +     return 0;\n>> +}\n>> +#define atexit git_atexit\n>> +\n>>  #endif\n>>\n>>  int start_async(struct async *async)\n>> @@ -682,6 +721,7 @@ int start_async(struct async *async)\n>>                       close(fdin[1]);\n>>               if (need_out)\n>>                       close(fdout[0]);\n>> +             git_atexit_clear();\n>>               exit(!!async->proc(proc_in, proc_out, async->data));\n>>       }\n>>\n>> diff --git a/shallow.c b/shallow.c\n>> index de07709..f067811 100644\n>> --- a/shallow.c\n>> +++ b/shallow.c\n>> @@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)\n>>\n>>  const char *setup_temporary_shallow(const struct sha1_array *extra)\n>>  {\n>> -     static int installed_handler;\n>>       struct strbuf sb = STRBUF_INIT;\n>>       int fd;\n>>\n>> @@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)\n>>               strbuf_addstr(&temporary_shallow, git_path(\"shallow_XXXXXX\"));\n>>               fd = xmkstemp(temporary_shallow.buf);\n>>\n>> -             if (!installed_handler) {\n>> -                     atexit(remove_temporary_shallow);\n>> -                     sigchain_push_common(remove_temporary_shallow_on_signal);\n>> -             }\n>> +             atexit(remove_temporary_shallow);\n>> +             sigchain_push_common(remove_temporary_shallow_on_signal);\n>>\n>>               if (write_in_full(fd, sb.buf, sb.len) != sb.len)\n>>                       die_errno(\"failed to write to %s\",\n"},{"id":"250807","messageId":"f75aaa6d99622317894351849d23a32b36a5039c.1413635268.git.etienne.buira@gmail.com","threadId":"37713","inReplyTo":"CAMcFKVyMOO4iTq_70fqd0Jjq4WNzz9YrGYNSCZmVx5F28KXicQ@mail.gmail.com","subject":"[PATCH v5] Handle atexit list internaly for unthreaded builds","fromName":"Etienne Buira","fromEmail":"etienne.buira@gmail.com","sentAt":"2014-10-18T12:31:15Z","receivedAt":"2014-10-18T12:31:15Z","isPatch":true,"sender":{"key":"etienne.buira@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11487810?v=4"},"body":"Wrap atexit()s calls on unthreaded builds to handle callback list\ninternally.\n\nThis is needed because on unthreaded builds, asyncs inherits parent's\natexit() list, that gets run as soon as the async exit()s (and again at\nthe end of async's parent process). That led to remove temporary files\ntoo early.\n\nAlso remove a by-atexit-callback guard against this kind of issue in\nclone.c, as this patch makes it redundant.\n\nFixes test 5537 (temporary shallow file vanished before unpack-objects\ncould open it)\n\nBTW remove an unused variable in shallow.c.\n\nHelped-by: Duy Nguyen <pclouds@gmail.com>\nHelped-by: Andreas Schwab <schwab@linux-m68k.org>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Etienne Buira <etienne.buira@gmail.com>\n---\n builtin/clone.c   |  5 -----\n git-compat-util.h |  5 +++++\n run-command.c     | 40 ++++++++++++++++++++++++++++++++++++++++\n shallow.c         |  7 ++-----\n 4 files changed, 47 insertions(+), 10 deletions(-)\n\nV5: update commit message\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex bbd169c..e122f33 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \n static const char *junk_work_tree;\n static const char *junk_git_dir;\n-static pid_t junk_pid;\n static enum {\n \tJUNK_LEAVE_NONE,\n \tJUNK_LEAVE_REPO,\n@@ -417,8 +416,6 @@ static void remove_junk(void)\n \t\tbreak;\n \t}\n \n-\tif (getpid() != junk_pid)\n-\t\treturn;\n \tif (junk_git_dir) {\n \t\tstrbuf_addstr(&sb, junk_git_dir);\n \t\tremove_dir_recursively(&sb, 0);\n@@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct refspec *refspec;\n \tconst char *fetch_pattern;\n \n-\tjunk_pid = getpid();\n-\n \tpacket_trace_identity(\"clone\");\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t     builtin_clone_usage, 0);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f587749..6dd63dd 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);\n const char *inet_ntop(int af, const void *src, char *dst, size_t size);\n #endif\n \n+#ifdef NO_PTHREADS\n+#define atexit git_atexit\n+extern int git_atexit(void (*handler)(void));\n+#endif\n+\n extern void release_pack_memory(size_t);\n \n typedef void (*try_to_free_t)(size_t);\ndiff --git a/run-command.c b/run-command.c\nindex 35a3ebf..0f9a9b0 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -624,6 +624,45 @@ static int async_die_is_recursing(void)\n \treturn ret != NULL;\n }\n \n+#else\n+\n+static struct {\n+\tvoid (**handlers)(void);\n+\tsize_t nr;\n+\tsize_t alloc;\n+} git_atexit_hdlrs;\n+\n+static int git_atexit_installed;\n+\n+static void git_atexit_dispatch()\n+{\n+\tsize_t i;\n+\n+\tfor (i=git_atexit_hdlrs.nr ; i ; i--)\n+\t\tgit_atexit_hdlrs.handlers[i-1]();\n+}\n+\n+static void git_atexit_clear()\n+{\n+\tfree(git_atexit_hdlrs.handlers);\n+\tmemset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));\n+\tgit_atexit_installed = 0;\n+}\n+\n+#undef atexit\n+int git_atexit(void (*handler)(void))\n+{\n+\tALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n+\tgit_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;\n+\tif (!git_atexit_installed) {\n+\t\tif (atexit(&git_atexit_dispatch))\n+\t\t\treturn -1;\n+\t\tgit_atexit_installed = 1;\n+\t}\n+\treturn 0;\n+}\n+#define atexit git_atexit\n+\n #endif\n \n int start_async(struct async *async)\n@@ -682,6 +721,7 @@ int start_async(struct async *async)\n \t\t\tclose(fdin[1]);\n \t\tif (need_out)\n \t\t\tclose(fdout[0]);\n+\t\tgit_atexit_clear();\n \t\texit(!!async->proc(proc_in, proc_out, async->data));\n \t}\n \ndiff --git a/shallow.c b/shallow.c\nindex de07709..f067811 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -226,7 +226,6 @@ static void remove_temporary_shallow_on_signal(int signo)\n \n const char *setup_temporary_shallow(const struct sha1_array *extra)\n {\n-\tstatic int installed_handler;\n \tstruct strbuf sb = STRBUF_INIT;\n \tint fd;\n \n@@ -237,10 +236,8 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)\n \t\tstrbuf_addstr(&temporary_shallow, git_path(\"shallow_XXXXXX\"));\n \t\tfd = xmkstemp(temporary_shallow.buf);\n \n-\t\tif (!installed_handler) {\n-\t\t\tatexit(remove_temporary_shallow);\n-\t\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n-\t\t}\n+\t\tatexit(remove_temporary_shallow);\n+\t\tsigchain_push_common(remove_temporary_shallow_on_signal);\n \n \t\tif (write_in_full(fd, sb.buf, sb.len) != sb.len)\n \t\t\tdie_errno(\"failed to write to %s\",\n-- \n2.0.4\n"}]}