On Mon, Feb 4, 2019 at 7:06 AM brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > When we write an alternate shallow file in update_shallow, we write it > into the lock file. The string stored in alternate_shallow_file is > copied from the lock file path, but it is freed the moment that the lock > file is closed, since we call strbuf_release to free that path. > > This used to work, since we did not invoke git index-pack more than > once, but now that we do, we reuse the freed memory. Ensure we reset the > value to NULL to avoid using freed memory. git index-pack will read the > repository's shallow file, which will have been updated with the correct > information. It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow lock unnecessarily - 2019-01-10). I believe this is the same problem and a full solution was suggested but not implemented in that commit. The problem with dangling alternate_shallow_file is also from that commit. When line_received is false at the end of receive_shallow_info(), we should clear alternate_shallow_file. I'm still debating myself whether we should clear alternate_shallow_file in receive_shallow_info() in addition to your changes (which is good hygiene anyway) to keep the setup steps of do_fetch_pack() and do_fetch_pack_v2() aligned. pack protocol v1 does this clearing alternate_shallow_file (near the end of do_fetch_pack()) so it will not have the same dangling pointer problem even if we do two fetches in the same process. But from the Tan's commit description, I thought v1 probably faces the same "lock twice not supported". But luckily it's never triggered in practice. When backfill_tags() is called, if I read it correctly, we either disable shallow (TRANS_OPT_DEPTH set to zero) or we start a second process. In either case, the lock will not be acquired twice by the same process. > Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> > --- > fetch-pack.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fetch-pack.c b/fetch-pack.c > index 577faa6229..2d76287674 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args, > rollback_lock_file(&shallow_lock); > } else > commit_lock_file(&shallow_lock); > + alternate_shallow_file = NULL; > return; > } > > @@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args, > &alternate_shallow_file, > &extra); > commit_lock_file(&shallow_lock); > + alternate_shallow_file = NULL; > } > oid_array_clear(&extra); > return; > @@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args, > commit_lock_file(&shallow_lock); > oid_array_clear(&extra); > oid_array_clear(&ref); > + alternate_shallow_file = NULL; > return; > } > -- Duy
