Commit 56b203a5e0a5348700d85479b0070289d37c2cf0
2019-10-24T12:20:27
config_file: keep reference to config entries when creating iterator When creating a configuration file iterator, then we first refresh the backend and then afterwards duplicate all refreshed configuration entries into the iterator in order to avoid seeing any concurrent modifications of the entries while iterating. The duplication of entries is not guarded, though, as we do not increase the refcount of the entries that we duplicate right now. This opens us up for a race, as another thread may concurrently refresh the repository configuration and thus swap out the current set of entries. As we didn't increase the refcount, this may lead to the entries being free'd while we iterate over them in the first thread. Fix the issue by properly handling the lifecycle of the backend's entries via `config_file_entries_take` and `git_config_entries_free`, respectively.
diff --git a/src/config_file.c b/src/config_file.c
index 4d1bcde..c9e3649 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -250,17 +250,19 @@ static int config_file_iterator(
struct git_config_backend *backend)
{
config_file_backend *b = GIT_CONTAINER_OF(backend, config_file_backend, parent);
- git_config_entries *entries = NULL;
+ git_config_entries *dupped = NULL, *entries = NULL;
int error;
if ((error = config_file_refresh(backend)) < 0 ||
- (error = git_config_entries_dup(&entries, b->entries)) < 0 ||
- (error = git_config_entries_iterator_new(iter, entries)) < 0)
+ (error = config_file_entries_take(&entries, b)) < 0 ||
+ (error = git_config_entries_dup(&dupped, entries)) < 0 ||
+ (error = git_config_entries_iterator_new(iter, dupped)) < 0)
goto out;
out:
/* Let iterator delete duplicated entries when it's done */
git_config_entries_free(entries);
+ git_config_entries_free(dupped);
return error;
}