diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 8870a210ec2e94..b1f8efb30e671c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -543,6 +543,15 @@ static void batch_object_write(const char *obj_name, if (opt->objects_filter.choice == LOFC_BLOB_LIMIT) data->info.sizep = &data->size; + /* + * With "--batch-all-objects --unordered" we read straight + * from the pack the walk handed us. If a concurrent repack + * removed it, fall back to a normal lookup; is_pack_valid() + * pins the pack's fd, so a passing check stays valid even if + * the pack is unlinked right after. + */ + if (pack && !is_pack_valid(pack)) + pack = NULL; if (pack) ret = packed_object_info(NULL, pack, offset, &data->info); else @@ -1008,10 +1017,27 @@ static int batch_one_object_oi(const struct object_id *oid, return payload->callback(oid, NULL, 0, payload->payload); } -static void batch_each_object(struct batch_options *opt, - for_each_object_fn callback, - unsigned flags, - void *_payload) +/* + * Open every pack index up front so the enumeration's object set is + * fixed: an index mmap survives unlink() of the .idx and pack fd + * pressure (close_one_pack() closes only the pack fd). This narrows + * the race with concurrent repacks, like f6b262581a88 (fsck: snapshot + * default refs before object walk, 2026-01-09). + */ +static int snapshot_pack_indexes(void) +{ + struct packed_git *p; + + repo_for_each_pack(the_repository, p) + if (open_pack_index(p)) + return -1; + return 0; +} + +static int batch_each_object(struct batch_options *opt, + for_each_object_fn callback, + unsigned flags, + void *_payload) { struct for_each_object_payload payload = { .callback = callback, @@ -1026,8 +1052,8 @@ static void batch_each_object(struct batch_options *opt, .filter = &opt->objects_filter, }; - odb_for_each_object_ext(the_repository->objects, &oi, - batch_one_object_oi, &payload, &opts); + return odb_for_each_object_ext(the_repository->objects, &oi, + batch_one_object_oi, &payload, &opts); } static int batch_objects(struct batch_options *opt) @@ -1075,6 +1101,11 @@ static int batch_objects(struct batch_options *opt) disable_replace_refs(); + if (snapshot_pack_indexes()) { + strbuf_release(&output); + return error(_("unable to enumerate all objects")); + } + cb.opt = opt; cb.expand = &data; cb.scratch = &output; @@ -1084,20 +1115,22 @@ static int batch_objects(struct batch_options *opt) cb.seen = &seen; - batch_each_object(opt, batch_unordered_object, - ODB_FOR_EACH_OBJECT_PACK_ORDER, &cb); + retval = batch_each_object(opt, batch_unordered_object, + ODB_FOR_EACH_OBJECT_PACK_ORDER, &cb); oidset_clear(&seen); } else { struct oid_array sa = OID_ARRAY_INIT; - batch_each_object(opt, collect_object, 0, &sa); + retval = batch_each_object(opt, collect_object, 0, &sa); oid_array_for_each_unique(&sa, batch_object_cb, &cb); oid_array_clear(&sa); } strbuf_release(&output); + if (retval) + return error(_("unable to enumerate all objects")); return 0; } diff --git a/t/t5313-pack-bounds-checks.sh b/t/t5313-pack-bounds-checks.sh index 5be01260d7760f..3edcc0936a1a47 100755 --- a/t/t5313-pack-bounds-checks.sh +++ b/t/t5313-pack-bounds-checks.sh @@ -100,9 +100,12 @@ test_expect_success 'matched bogus object count' ' clear_base && # Unlike above, we should notice early that the .idx is totally - # bogus, and not even enumerate its contents. - git cat-file --batch-all-objects --batch-check >actual && + # bogus, report failure, and not enumerate its contents. + test_must_fail git cat-file --batch-all-objects \ + --batch-check >actual 2>err && test_must_be_empty actual && + test_grep "non-monotonic index" err && + test_grep "unable to enumerate all objects" err && # But as before, we can do the same object-access checks. test_must_fail git cat-file blob $object &&