Commit e6e8530aa6c8773dd523fd6fe07629b9481a8fee
2014-04-14T12:31:17
Lock attribute file while reparsing data I don't love this approach, but achieving thread-safety for attribute and ignore data while reloading files would require a larger rewrite in order to avoid this. If an attribute or ignore file is out of date, this holds a lock on the file while we are reloading the data so that another thread won't try to reload the data at the same time.
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 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199
diff --git a/src/attr_file.c b/src/attr_file.c
index 66f71ad..574de25 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -9,8 +9,13 @@
static void attr_file_free(git_attr_file *file)
{
- git_attr_file__clear_rules(file);
+ bool unlock = !git_mutex_lock(&file->lock);
+ git_attr_file__clear_rules(file, false);
git_pool_clear(&file->pool);
+ if (unlock)
+ git_mutex_unlock(&file->lock);
+ git_mutex_free(&file->lock);
+
git__memzero(file, sizeof(*file));
git__free(file);
}
@@ -23,6 +28,12 @@ int git_attr_file__new(
git_attr_file *attrs = git__calloc(1, sizeof(git_attr_file));
GITERR_CHECK_ALLOC(attrs);
+ if (git_mutex_init(&attrs->lock) < 0) {
+ giterr_set(GITERR_OS, "Failed to initialize lock");
+ git__free(attrs);
+ return -1;
+ }
+
if (git_pool_init(&attrs->pool, 1, 0) < 0) {
attr_file_free(attrs);
return -1;
@@ -35,14 +46,24 @@ int git_attr_file__new(
return 0;
}
-void git_attr_file__clear_rules(git_attr_file *file)
+int git_attr_file__clear_rules(git_attr_file *file, bool need_lock)
{
unsigned int i;
git_attr_rule *rule;
+ if (need_lock && git_mutex_lock(&file->lock) < 0) {
+ giterr_set(GITERR_OS, "Failed to lock attribute file");
+ return -1;
+ }
+
git_vector_foreach(&file->rules, i, rule)
git_attr_rule__free(rule);
git_vector_free(&file->rules);
+
+ if (need_lock)
+ git_mutex_unlock(&file->lock);
+
+ return 0;
}
void git_attr_file__free(git_attr_file *file)
@@ -162,6 +183,11 @@ int git_attr_file__parse_buffer(
!git__suffixcmp(attrs->ce->path, "/" GIT_ATTR_FILE))
context = attrs->ce->path;
+ if (git_mutex_lock(&attrs->lock) < 0) {
+ giterr_set(GITERR_OS, "Failed to lock attribute file");
+ return -1;
+ }
+
while (!error && *scan) {
/* allocate rule if needed */
if (!rule) {
@@ -198,6 +224,7 @@ int git_attr_file__parse_buffer(
}
}
+ git_mutex_unlock(&attrs->lock);
git_attr_rule__free(rule);
return error;
diff --git a/src/attr_file.h b/src/attr_file.h
index f92ce3c..9b4b872 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -66,6 +66,7 @@ typedef struct {
struct git_attr_file {
git_refcount rc;
+ git_mutex lock;
git_attr_cache_entry *ce;
git_attr_cache_source source;
git_vector rules; /* vector of <rule*> or <fnmatch*> */
@@ -115,7 +116,8 @@ int git_attr_file__parse_buffer(
const char *data,
void *payload);
-void git_attr_file__clear_rules(git_attr_file *file);
+int git_attr_file__clear_rules(
+ git_attr_file *file, bool need_lock);
int git_attr_file__lookup_one(
git_attr_file *file,
diff --git a/src/ignore.c b/src/ignore.c
index 1f9df8a..fbebd9a 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -32,6 +32,11 @@ static int parse_ignore_file(
!git__suffixcmp(attrs->ce->path, "/" GIT_IGNORE_FILE))
context = attrs->ce->path;
+ if (git_mutex_lock(&attrs->lock) < 0) {
+ giterr_set(GITERR_OS, "Failed to lock attribute file");
+ return -1;
+ }
+
while (!error && *scan) {
if (!match) {
match = git__calloc(1, sizeof(*match));
@@ -63,6 +68,7 @@ static int parse_ignore_file(
}
}
+ git_mutex_unlock(&attrs->lock);
git__free(match);
return error;
@@ -247,12 +253,12 @@ void git_ignore__free(git_ignores *ignores)
}
static bool ignore_lookup_in_rules(
- git_vector *rules, git_attr_path *path, int *ignored)
+ git_attr_file *file, git_attr_path *path, int *ignored)
{
size_t j;
git_attr_fnmatch *match;
- git_vector_rforeach(rules, j, match) {
+ git_vector_rforeach(&file->rules, j, match) {
if (git_attr_fnmatch__match(match, path)) {
*ignored = ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0);
return true;
@@ -274,19 +280,18 @@ int git_ignore__lookup(
return -1;
/* first process builtins - success means path was found */
- if (ignore_lookup_in_rules(
- &ignores->ign_internal->rules, &path, ignored))
+ if (ignore_lookup_in_rules(ignores->ign_internal, &path, ignored))
goto cleanup;
/* next process files in the path */
git_vector_foreach(&ignores->ign_path, i, file) {
- if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+ if (ignore_lookup_in_rules(file, &path, ignored))
goto cleanup;
}
/* last process global ignores */
git_vector_foreach(&ignores->ign_global, i, file) {
- if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+ if (ignore_lookup_in_rules(file, &path, ignored))
goto cleanup;
}
@@ -319,10 +324,9 @@ int git_ignore_clear_internal_rules(
git_attr_file *ign_internal;
if (!(error = get_internal_ignores(&ign_internal, repo))) {
- git_attr_file__clear_rules(ign_internal);
-
- error = parse_ignore_file(
- repo, ign_internal, GIT_IGNORE_DEFAULT_RULES, NULL);
+ if (!(error = git_attr_file__clear_rules(ign_internal, true)))
+ error = parse_ignore_file(
+ repo, ign_internal, GIT_IGNORE_DEFAULT_RULES, NULL);
git_attr_file__free(ign_internal);
}
@@ -371,19 +375,18 @@ int git_ignore_path_is_ignored(
break;
/* first process builtins - success means path was found */
- if (ignore_lookup_in_rules(
- &ignores.ign_internal->rules, &path, ignored))
+ if (ignore_lookup_in_rules(ignores.ign_internal, &path, ignored))
goto cleanup;
/* next process files in the path */
git_vector_foreach(&ignores.ign_path, i, file) {
- if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+ if (ignore_lookup_in_rules(file, &path, ignored))
goto cleanup;
}
/* last process global ignores */
git_vector_foreach(&ignores.ign_global, i, file) {
- if (ignore_lookup_in_rules(&file->rules, &path, ignored))
+ if (ignore_lookup_in_rules(file, &path, ignored))
goto cleanup;
}