]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Merge branch 'ph/pack-objects-mutex-fix'
authorJunio C Hamano <gitster@pobox.com>
Tue, 5 Feb 2019 22:26:16 +0000 (14:26 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 5 Feb 2019 22:26:16 +0000 (14:26 -0800)
"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

1  2 
builtin/pack-objects.c
pack-objects.c
pack-objects.h

diff --combined builtin/pack-objects.c
index 0a70d046043ec9b7ae2e604a7ad098d22d36ed59,cfef48217f9523dbc33fae4b12e97ffc467c39dd..8eb5391c089d1add157cb677db037e30cd9ada16
@@@ -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;
                        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;
        }
  
        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);
                    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));
                *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);
                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 9c45842df389270baf73f4aa5b65df2004c9ba4d,a1dc5eb7263008696d45e98038c16e4bb1d7da7e..e7cd337bee53cb02282a14c8aa88f2342edd2eb2
@@@ -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
                                             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 3cd8d1f00a95978a5a8c86805506b9927aefcd0e,1667cbad8f6a5637b8948de9bf43ffae9689f131..6bfacc7d2cedd7f399d7807b7746b6ed66737eed
@@@ -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;
  
        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
        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,