On Mon, Mar 14, 2022 at 12:24:12AM +0100, Omar Polo wrote:
> Omar Polo <op@omarpolo.com> wrote:
> > Stefan Sperling <stsp@stsp.name> wrote:
> > > Profiling shows that many calls to readdir(3) of the objects/pack/
> > > directory can cause significant overhead during 'gotadmin pack -a'
> > > if the repository has many pack files that need to be searched.
> > > 
> > > This patch caches the list of known pack index files when the
> > > repository is opened. This speeds up searches by skipping readdir(3)
> > > if the pack index cache cannot prevent us from hitting the disk.
> > > 
> > > Pack files are not really expected to appear or disappear out
> > > of the blue while the repository is in use, even though this
> > > is possible. But even readdir(3) followed open(2) of an entry
> > > could suffer from this. If this becomes a problem it was already
> > > possible to hit with the old code, and could only be fixed by
> > > catching relevant errors and retrying.
> > > 
> > > ok?
> > 
> > makes sense and the diff is almost straightforward.
> 
> I take back what I said, this broke at least the `send' command.
> 
> 	% got send
> 	Connecting to "origin" git@omarpolo.com
> 	packing 1 reference; 3 objects
> 	Segmentation fault
> 
> without leaving any core file around, and also part of the regress
> suite.
> 
> (don't have time to debug right now, sorry, it's pretty late, but wanted
> to share this.)

Thanks! Not sure how this slipped through my testing, I believe I
did run the test suite on the diff.

There is an unintialized 'err' variable in the existing code.
Initializing this variable should resolve the problem:

-----------------------------------------------
commit 7114caa4e6f02478c216aa1b4dbe16f22aa38afa (search-packidx-readdir)
from: Stefan Sperling <stsp@stsp.name>
date: Mon Mar 14 13:04:27 2022 UTC
 
 fix uninitialized error variable in find_pack_for_reuse()
 
diff f208811a6e934112206f1b12bd5eb1223774a5cd 3714d5a61c20ff19b836b9e8106e3d7c0a2d3476
blob - 633d93bf0e00b96fca90fbb00e8974e0c4925f37
blob + 273f38d8a8d75455bcb4a4eebf66c1d7dae0df8e
--- lib/pack_create.c
+++ lib/pack_create.c
@@ -489,7 +489,7 @@ static const struct got_error *
 find_pack_for_reuse(struct got_packidx **best_packidx,
     struct got_repository *repo)
 {
-	const struct got_error *err;
+	const struct got_error *err = NULL;
 	struct got_pathlist_entry *pe;
 	const char *best_packidx_path = NULL;
 	int nobj_max = 0;