Etienne Buira <etienne.bu...@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 <pclo...@gmail.com>
> Thanks-to: Andreas Schwab <sch...@linux-m68k.org>

Usually we spell these "Helped-by: " instead.

> Signed-off-by: Etienne Buira <etienne.bu...@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",
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to