Commit aa78c9ba778d8c7c2c62f875b974ae53ba90d12b
2014-04-01T10:22:51
Minor submodule cache locking improvements This improvement the management of the lock around submodule cache updates slightly, using the lock to make sure that foreach can safely make a snapshot of all existing submodules and making sure that git_submodule_add_setup also grabs a lock before inserting the new submodule. Cache initialization / refresh should already have been holding the lock correctly as it adds submodules.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113
diff --git a/src/submodule.c b/src/submodule.c
index 96b445a..5061cad 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -236,40 +236,62 @@ int git_submodule_lookup(
return error;
}
+static void submodule_free_dup(void *sm)
+{
+ git_submodule_free(sm);
+}
+
int git_submodule_foreach(
git_repository *repo,
int (*callback)(git_submodule *sm, const char *name, void *payload),
void *payload)
{
int error;
+ size_t i;
git_submodule *sm;
- git_vector seen = GIT_VECTOR_INIT;
- git_vector_set_cmp(&seen, submodule_cmp);
+ git_submodule_cache *cache;
+ git_vector snapshot = GIT_VECTOR_INIT;
assert(repo && callback);
if ((error = submodule_cache_init(repo, CACHE_REFRESH)) < 0)
return error;
- git_strmap_foreach_value(repo->_submodules->submodules, sm, {
- /* Usually the following will not come into play - it just prevents
- * us from issuing a callback twice for a submodule where the name
- * and path are not the same.
- */
- if (GIT_REFCOUNT_VAL(sm) > 1) {
- if (git_vector_bsearch(NULL, &seen, sm) != GIT_ENOTFOUND)
- continue;
- if ((error = git_vector_insert(&seen, sm)) < 0)
+ cache = repo->_submodules;
+
+ if (git_mutex_lock(&cache->lock) < 0) {
+ giterr_set(GITERR_OS, "Unable to acquire lock on submodule cache");
+ return -1;
+ }
+
+ if (!(error = git_vector_init(
+ &snapshot, kh_size(cache->submodules), submodule_cmp))) {
+
+ git_strmap_foreach_value(cache->submodules, sm, {
+ if ((error = git_vector_insert(&snapshot, sm)) < 0)
break;
- }
+ GIT_REFCOUNT_INC(sm);
+ });
+ }
+ git_mutex_unlock(&cache->lock);
+
+ if (error < 0)
+ goto done;
+
+ git_vector_uniq(&snapshot, submodule_free_dup);
+
+ git_vector_foreach(&snapshot, i, sm) {
if ((error = callback(sm, sm->name, payload)) != 0) {
giterr_set_after_callback(error);
break;
}
- });
+ }
- git_vector_free(&seen);
+done:
+ git_vector_foreach(&snapshot, i, sm)
+ git_submodule_free(sm);
+ git_vector_free(&snapshot);
return error;
}
@@ -387,10 +409,18 @@ int git_submodule_add_setup(
/* add submodule to hash and "reload" it */
+ if (git_mutex_lock(&repo->_submodules->lock) < 0) {
+ giterr_set(GITERR_OS, "Unable to acquire lock on submodule cache");
+ error = -1;
+ goto cleanup;
+ }
+
if (!(error = submodule_get(&sm, repo->_submodules, path, NULL)) &&
!(error = git_submodule_reload(sm, false)))
error = git_submodule_init(sm, false);
+ git_mutex_unlock(&repo->_submodules->lock);
+
cleanup:
if (error && sm) {
git_submodule_free(sm);
diff --git a/src/vector.c b/src/vector.c
index e5d8919..c2c67e6 100644
--- a/src/vector.c
+++ b/src/vector.c
@@ -54,7 +54,7 @@ int git_vector_dup(git_vector *v, const git_vector *src, git_vector_cmp cmp)
bytes = src->length * sizeof(void *);
v->_alloc_size = src->length;
- v->_cmp = cmp;
+ v->_cmp = cmp ? cmp : src->_cmp;
v->length = src->length;
v->flags = src->flags;
if (cmp != src->_cmp)