From: Junio C Hamano <gitster@pobox•com>
To: Etienne Buira <etienne.buira@gmail•com>
Cc: pclouds@gmail•com, schwab@linux-m68k•org, git@vger•kernel.org
Subject: Re: [PATCH v4] Handle atexit list internaly for unthreaded builds
Date: Mon, 13 Oct 2014 13:00:16 -0700 [thread overview]
Message-ID: <xmqq7g033flb.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <2724eb56a7219e0ed341e734beb32aa236119998.1413225018.git.etienne.buira@gmail.com> (Etienne Buira's message of "Mon, 13 Oct 2014 20:35:58 +0200")
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?
>
> 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",
next prev parent reply other threads:[~2014-10-13 20:00 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-11 14:53 [PATCH] Handle atexit list internaly fo unthreaded builds Etienne Buira
2014-10-12 9:09 ` [PATCH v2] " Etienne Buira
2014-10-13 0:56 ` Duy Nguyen
2014-10-13 15:19 ` [PATCH v2] Handle atexit list internaly for " Etienne Buira
2014-10-13 15:19 ` [PATCH v3] " Etienne Buira
2014-10-13 16:37 ` Andreas Schwab
2014-10-13 18:35 ` [PATCH v4] " Etienne Buira
2014-10-13 20:00 ` Junio C Hamano [this message]
2014-10-14 15:02 ` Etienne Buira
2014-10-18 12:31 ` [PATCH v5] " Etienne Buira
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq7g033flb.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=etienne.buira@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=schwab@linux-m68k$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox