Re: [PATCH] tmp-objdir: do not opendir() when handling a signal

[Jeff King <peff@peff.net>]

On Tue, Sep 27, 2022 at 09:50:43AM -0400, John Cai wrote:

> > 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