From: Junio C Hamano Date: Tue, 5 Feb 2019 22:26:16 +0000 (-0800) Subject: Merge branch 'ph/pack-objects-mutex-fix' X-Git-Tag: v2.21.0-rc0~39 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/d243a323a545da68b87149e885f2e440f0b13725?hp=-c Merge branch 'ph/pack-objects-mutex-fix' "git pack-objects" incorrectly used uninitialized mutex, which has been corrected. * ph/pack-objects-mutex-fix: pack-objects: merge read_lock and lock in packing_data struct pack-objects: move read mutex to packing_data struct --- d243a323a545da68b87149e885f2e440f0b13725 diff --combined builtin/pack-objects.c index 0a70d04604,cfef48217f..8eb5391c08 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@@ -1334,7 -1334,7 +1334,7 @@@ static void add_pbase_object(struct tre if (cmp < 0) return; if (name[cmplen] != '/') { - add_object_entry(entry.oid, + add_object_entry(&entry.oid, object_type(entry.mode), fullname, 1); return; @@@ -1345,7 -1345,7 +1345,7 @@@ const char *down = name+cmplen+1; int downlen = name_cmp_len(down); - tree = pbase_tree_get(entry.oid); + tree = pbase_tree_get(&entry.oid); if (!tree) return; init_tree_desc(&sub, tree->tree_data, tree->tree_size); @@@ -1953,11 -1953,6 +1953,6 @@@ static int delta_cacheable(unsigned lon return 0; } - /* Protect access to object database */ - static pthread_mutex_t read_mutex; - #define read_lock() pthread_mutex_lock(&read_mutex) - #define read_unlock() pthread_mutex_unlock(&read_mutex) - /* Protect delta_cache_size */ static pthread_mutex_t cache_mutex; #define cache_lock() pthread_mutex_lock(&cache_mutex) @@@ -1993,11 -1988,11 +1988,11 @@@ unsigned long oe_get_size_slow(struct p unsigned long used, avail, size; if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) { - read_lock(); + packing_data_lock(&to_pack); if (oid_object_info(the_repository, &e->idx.oid, &size) < 0) die(_("unable to get size of %s"), oid_to_hex(&e->idx.oid)); - read_unlock(); + packing_data_unlock(&to_pack); return size; } @@@ -2005,7 -2000,7 +2000,7 @@@ if (!p) BUG("when e->type is a delta, it must belong to a pack"); - read_lock(); + packing_data_lock(&to_pack); w_curs = NULL; buf = use_pack(p, &w_curs, e->in_pack_offset, &avail); used = unpack_object_header_buffer(buf, avail, &type, &size); @@@ -2014,7 -2009,7 +2009,7 @@@ oid_to_hex(&e->idx.oid)); unuse_pack(&w_curs); - read_unlock(); + packing_data_unlock(&to_pack); return size; } @@@ -2076,9 -2071,9 +2071,9 @@@ static int try_delta(struct unpacked *t /* Load data if not already done */ if (!trg->data) { - read_lock(); + packing_data_lock(&to_pack); trg->data = read_object_file(&trg_entry->idx.oid, &type, &sz); - read_unlock(); + packing_data_unlock(&to_pack); if (!trg->data) die(_("object %s cannot be read"), oid_to_hex(&trg_entry->idx.oid)); @@@ -2089,9 -2084,9 +2084,9 @@@ *mem_usage += sz; } if (!src->data) { - read_lock(); + packing_data_lock(&to_pack); src->data = read_object_file(&src_entry->idx.oid, &type, &sz); - read_unlock(); + packing_data_unlock(&to_pack); if (!src->data) { if (src_entry->preferred_base) { static int warned = 0; @@@ -2337,9 -2332,9 +2332,9 @@@ static void find_deltas(struct object_e static void try_to_free_from_threads(size_t size) { - read_lock(); + packing_data_lock(&to_pack); release_pack_memory(size); - read_unlock(); + packing_data_unlock(&to_pack); } static try_to_free_t old_try_to_free_routine; @@@ -2381,7 -2376,6 +2376,6 @@@ static pthread_cond_t progress_cond */ static void init_threaded_search(void) { - init_recursive_mutex(&read_mutex); pthread_mutex_init(&cache_mutex, NULL); pthread_mutex_init(&progress_mutex, NULL); pthread_cond_init(&progress_cond, NULL); @@@ -2392,7 -2386,6 +2386,6 @@@ static void cleanup_threaded_search(voi { set_try_to_free_routine(old_try_to_free_routine); pthread_cond_destroy(&progress_cond); - pthread_mutex_destroy(&read_mutex); pthread_mutex_destroy(&cache_mutex); pthread_mutex_destroy(&progress_mutex); } @@@ -2610,7 -2603,7 +2603,7 @@@ static void prepare_pack(int window, in unsigned n; if (use_delta_islands) - resolve_tree_islands(progress, &to_pack); + resolve_tree_islands(the_repository, progress, &to_pack); get_object_details(); @@@ -3084,16 -3077,14 +3077,16 @@@ static void record_recent_commit(struc static void get_object_list(int ac, const char **av) { struct rev_info revs; + struct setup_revision_opt s_r_opt = { + .allow_exclude_promisor_objects = 1, + }; char line[1000]; int flags = 0; int save_warning; repo_init_revisions(the_repository, &revs, NULL); save_commit_buffer = 0; - revs.allow_exclude_promisor_objects_opt = 1; - setup_revisions(ac, av, &revs, NULL); + setup_revisions(ac, av, &revs, &s_r_opt); /* make sure shallows are read */ is_repository_shallow(the_repository); @@@ -3133,7 -3124,7 +3126,7 @@@ return; if (use_delta_islands) - load_delta_islands(); + load_delta_islands(the_repository); if (prepare_revision_walk(&revs)) die(_("revision walk setup failed")); @@@ -3472,7 -3463,7 +3465,7 @@@ int cmd_pack_objects(int argc, const ch } } - prepare_packing_data(&to_pack); + prepare_packing_data(the_repository, &to_pack); if (progress) progress_state = start_progress(_("Enumerating objects"), 0); diff --combined pack-objects.c index 9c45842df3,a1dc5eb726..e7cd337bee --- a/pack-objects.c +++ b/pack-objects.c @@@ -99,7 -99,7 +99,7 @@@ static void prepare_in_pack_by_idx(stru * (i.e. in_pack_idx also zero) should return NULL. */ mapping[cnt++] = NULL; - for (p = get_all_packs(the_repository); p; p = p->next, cnt++) { + for (p = get_all_packs(pdata->repo); p; p = p->next, cnt++) { if (cnt == nr) { free(mapping); return; @@@ -133,10 -133,8 +133,10 @@@ void oe_map_new_pack(struct packing_dat } /* assume pdata is already zero'd by caller */ -void prepare_packing_data(struct packing_data *pdata) +void prepare_packing_data(struct repository *r, struct packing_data *pdata) { + pdata->repo = r; + if (git_env_bool("GIT_TEST_FULL_IN_PACK_ARRAY", 0)) { /* * do not initialize in_pack_by_idx[] to force the @@@ -150,9 -148,7 +150,7 @@@ 1U << OE_SIZE_BITS); pdata->oe_delta_size_limit = git_env_ulong("GIT_TEST_OE_DELTA_SIZE", 1UL << OE_DELTA_SIZE_BITS); - #ifndef NO_PTHREADS - pthread_mutex_init(&pdata->lock, NULL); - #endif + init_recursive_mutex(&pdata->odb_lock); } struct object_entry *packlist_alloc(struct packing_data *pdata, diff --combined pack-objects.h index 3cd8d1f00a,1667cbad8f..6bfacc7d2c --- a/pack-objects.h +++ b/pack-objects.h @@@ -5,8 -5,6 +5,8 @@@ #include "thread-utils.h" #include "pack.h" +struct repository; + #define DEFAULT_DELTA_CACHE_SIZE (256 * 1024 * 1024) #define OE_DFS_STATE_BITS 2 @@@ -129,7 -127,6 +129,7 @@@ struct object_entry }; struct packing_data { + struct repository *repo; struct object_entry *objects; uint32_t nr_objects, nr_alloc; @@@ -148,7 -145,11 +148,11 @@@ struct packed_git **in_pack_by_idx; struct packed_git **in_pack; - pthread_mutex_t lock; + /* + * During packing with multiple threads, protect the in-core + * object database from concurrent accesses. + */ + pthread_mutex_t odb_lock; /* * This list contains entries for bases which we know the other side @@@ -166,15 -167,16 +170,16 @@@ unsigned char *layer; }; -void prepare_packing_data(struct packing_data *pdata); +void prepare_packing_data(struct repository *r, struct packing_data *pdata); + /* Protect access to object database */ static inline void packing_data_lock(struct packing_data *pdata) { - pthread_mutex_lock(&pdata->lock); + pthread_mutex_lock(&pdata->odb_lock); } static inline void packing_data_unlock(struct packing_data *pdata) { - pthread_mutex_unlock(&pdata->lock); + pthread_mutex_unlock(&pdata->odb_lock); } struct object_entry *packlist_alloc(struct packing_data *pdata,