Re: [PATCH] tmp-objdir: do not opendir() when handling a signal
- From
Jeff King <peff@peff.net>
- Date
- Sep 27, 2022, 19:03 UTC
- Message-ID
- <YzNI8tEpREMLNUpg@coredump.intra.peff.net>
- In-Reply-To
- <1B4D6262-532A-401E-B1A2-709BEECA4B67@gmail.com>
On Tue, Sep 27, 2022 at 09:50:43AM -0400, John Cai wrote:
Show 16 quoted lines
> > diff --git a/tmp-objdir.c b/tmp-objdir.c
> > index a8be92bca1..10549e95db 100644
> > --- a/tmp-objdir.c
> > +++ b/tmp-objdir.c
> > @@ -169,7 +169,6 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix)
> > the_tmp_objdir = t;
> > if (!installed_handlers) {
> > atexit(remove_tmp_objdir);
> > - sigchain_push_common(remove_tmp_objdir_on_signal);
> > installed_handlers++;
> > }
>
> This makes sense and is a clean solution. I guess the only case where we would
> benefit in calling into remove_tmp_objdir_on_signal() is if the temp dir exists
> but is empty. I'm not sure how often this would happen to make it worth it?
> Probably not...It's more or less never, as the first thing we do after creating it is call tmp_objdir_setup(), which creates the pack subdirectory. We _could_ try removing them both manually, like:
diff --git a/tmp-objdir.c b/tmp-objdir.c index adf6033549..509eb58363 100644 --- a/tmp-objdir.c +++ b/tmp-objdir.c @@ -49,7 +49,17 @@ static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal) * have pre-grown t->path sufficiently so that this * doesn't happen in practice. */ - err = remove_dir_recursively(&t->path, 0); + if (!on_signal) + err = remove_dir_recursively(&t->path, 0); + else { + size_t orig_len = t->path.len; + + strbuf_addstr(&t->path, "/pack"); + rmdir(t->path.buf); + strbuf_setlen(&t->path, orig_len); + + rmdir(t->path.buf); + } /* * When we are cleaning up due to a signal, we won't bother but even that would rarely do anything useful. As soon as the child index-pack receives even a single byte, we'll have an actual pack tmpfile with an unknown name, which will cause rmdir() to fail (ditto for unpack-objects, though it would probably write as soon as we've received a whole single object). If we can't enumerate the list of objects via readdir() or similar, I think there's really not much we can do. -Peff