config: implement basic transactional support When a configuration file is locked, any updates made to it will be done to the in-memory copy of the file. This allows for multiple updates to happen while we hold the lock, preventing races during complex config-file manipulation.
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 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264
diff --git a/include/git2/sys/config.h b/include/git2/sys/config.h
index 044e344..4dad6da 100644
--- a/include/git2/sys/config.h
+++ b/include/git2/sys/config.h
@@ -67,6 +67,20 @@ struct git_config_backend {
int (*iterator)(git_config_iterator **, struct git_config_backend *);
/** Produce a read-only version of this backend */
int (*snapshot)(struct git_config_backend **, struct git_config_backend *);
+ /**
+ * Lock this backend.
+ *
+ * Prevent any writes to the data store backing this
+ * backend. Any updates must not be visible to any other
+ * readers.
+ */
+ int (*lock)(struct git_config_backend *);
+ /**
+ * Unlock the data store backing this backend. If success is
+ * true, the changes should be committed, otherwise rolled
+ * back.
+ */
+ int (*unlock)(struct git_config_backend *, int success);
void (*free)(struct git_config_backend *);
};
#define GIT_CONFIG_BACKEND_VERSION 1
diff --git a/src/config_file.c b/src/config_file.c
index fd03c20..a3fec1b 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -105,6 +105,10 @@ typedef struct {
git_array_t(struct reader) readers;
+ bool locked;
+ git_filebuf locked_buf;
+ git_buf locked_content;
+
char *file_path;
} diskfile_backend;
@@ -685,6 +689,42 @@ static int config_snapshot(git_config_backend **out, git_config_backend *in)
return git_config_file__snapshot(out, b);
}
+static int config_lock(git_config_backend *_cfg)
+{
+ diskfile_backend *cfg = (diskfile_backend *) _cfg;
+ int error;
+
+ if ((error = git_filebuf_open(&cfg->locked_buf, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0)
+ return error;
+
+ error = git_futils_readbuffer(&cfg->locked_content, cfg->file_path);
+ if (error < 0 && error != GIT_ENOTFOUND) {
+ git_filebuf_cleanup(&cfg->locked_buf);
+ return error;
+ }
+
+ cfg->locked = true;
+ return 0;
+
+}
+
+static int config_unlock(git_config_backend *_cfg, int success)
+{
+ diskfile_backend *cfg = (diskfile_backend *) _cfg;
+ int error = 0;
+
+ if (success) {
+ git_filebuf_write(&cfg->locked_buf, cfg->locked_content.ptr, cfg->locked_content.size);
+ error = git_filebuf_commit(&cfg->locked_buf);
+ }
+
+ git_filebuf_cleanup(&cfg->locked_buf);
+ git_buf_free(&cfg->locked_content);
+ cfg->locked = false;
+
+ return error;
+}
+
int git_config_file__ondisk(git_config_backend **out, const char *path)
{
diskfile_backend *backend;
@@ -706,6 +746,8 @@ int git_config_file__ondisk(git_config_backend **out, const char *path)
backend->header.parent.del_multivar = config_delete_multivar;
backend->header.parent.iterator = config_iterator_new;
backend->header.parent.snapshot = config_snapshot;
+ backend->header.parent.lock = config_lock;
+ backend->header.parent.unlock = config_unlock;
backend->header.parent.free = backend_free;
*out = (git_config_backend *)backend;
@@ -750,6 +792,21 @@ static int config_delete_readonly(git_config_backend *cfg, const char *name)
return config_error_readonly();
}
+static int config_lock_readonly(git_config_backend *_cfg)
+{
+ GIT_UNUSED(_cfg);
+
+ return config_error_readonly();
+}
+
+static int config_unlock_readonly(git_config_backend *_cfg, int success)
+{
+ GIT_UNUSED(_cfg);
+ GIT_UNUSED(success);
+
+ return config_error_readonly();
+}
+
static void backend_readonly_free(git_config_backend *_backend)
{
diskfile_backend *backend = (diskfile_backend *)_backend;
@@ -803,6 +860,8 @@ int git_config_file__snapshot(git_config_backend **out, diskfile_backend *in)
backend->header.parent.del = config_delete_readonly;
backend->header.parent.del_multivar = config_delete_multivar_readonly;
backend->header.parent.iterator = config_iterator_new;
+ backend->header.parent.lock = config_lock_readonly;
+ backend->header.parent.unlock = config_unlock_readonly;
backend->header.parent.free = backend_readonly_free;
*out = (git_config_backend *)backend;
@@ -1801,15 +1860,19 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
struct reader *reader = git_array_get(cfg->readers, 0);
struct write_data write_data;
- /* Lock the file */
- if ((result = git_filebuf_open(
- &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) {
+ if (cfg->locked) {
+ result = git_buf_puts(&reader->buffer, git_buf_cstr(&cfg->locked_content));
+ } else {
+ /* Lock the file */
+ if ((result = git_filebuf_open(
+ &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) {
git_buf_free(&reader->buffer);
return result;
- }
+ }
- /* We need to read in our own config file */
- result = git_futils_readbuffer(&reader->buffer, cfg->file_path);
+ /* We need to read in our own config file */
+ result = git_futils_readbuffer(&reader->buffer, cfg->file_path);
+ }
/* Initialise the reading position */
if (result == GIT_ENOTFOUND) {
@@ -1840,20 +1903,26 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
git__free(section);
if (result < 0) {
- git_buf_free(&buf);
git_filebuf_cleanup(&file);
goto done;
}
- git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf));
+ if (cfg->locked) {
+ size_t len = buf.asize;
+ /* Update our copy with the modified contents */
+ git_buf_free(&cfg->locked_content);
+ git_buf_attach(&cfg->locked_content, git_buf_detach(&buf), len);
+ } else {
+ git_filebuf_write(&file, git_buf_cstr(&buf), git_buf_len(&buf));
- /* refresh stats - if this errors, then commit will error too */
- (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file);
+ /* refresh stats - if this errors, then commit will error too */
+ (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file);
- result = git_filebuf_commit(&file);
- git_buf_free(&reader->buffer);
+ result = git_filebuf_commit(&file);
+ }
done:
+ git_buf_free(&buf);
git_buf_free(&reader->buffer);
return result;
}
diff --git a/src/config_file.h b/src/config_file.h
index 0d8bf74..1c52892 100644
--- a/src/config_file.h
+++ b/src/config_file.h
@@ -55,6 +55,16 @@ GIT_INLINE(int) git_config_file_foreach_match(
return git_config_backend_foreach_match(cfg, regexp, fn, data);
}
+GIT_INLINE(int) git_config_file_lock(git_config_backend *cfg)
+{
+ return cfg->lock(cfg);
+}
+
+GIT_INLINE(int) git_config_file_unlock(git_config_backend *cfg, int success)
+{
+ return cfg->unlock(cfg, success);
+}
+
extern int git_config_file_normalize_section(char *start, char *end);
#endif
diff --git a/tests/config/write.c b/tests/config/write.c
index 2e7b818..5446b95 100644
--- a/tests/config/write.c
+++ b/tests/config/write.c
@@ -1,6 +1,9 @@
#include "clar_libgit2.h"
#include "buffer.h"
#include "fileops.h"
+#include "git2/sys/config.h"
+#include "config_file.h"
+#include "config.h"
void test_config_write__initialize(void)
{
@@ -630,3 +633,46 @@ void test_config_write__to_file_with_only_comment(void)
git_buf_free(&result);
}
+void test_config_write__locking(void)
+{
+ git_config_backend *cfg, *cfg2;
+ git_config_entry *entry;
+ const char *filename = "locked-file";
+
+ /* Open the config and lock it */
+ cl_git_mkfile(filename, "[section]\n\tname = value\n");
+ cl_git_pass(git_config_file__ondisk(&cfg, filename));
+ cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP));
+ cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name"));
+ cl_assert_equal_s("value", entry->value);
+ git_config_entry_free(entry);
+ cl_git_pass(git_config_file_lock(cfg));
+
+ /* Change entries in the locked backend */
+ cl_git_pass(git_config_file_set_string(cfg, "section.name", "other value"));
+ cl_git_pass(git_config_file_set_string(cfg, "section2.name3", "more value"));
+
+ /* We can see that the file we read from hasn't changed */
+ cl_git_pass(git_config_file__ondisk(&cfg2, filename));
+ cl_git_pass(git_config_file_open(cfg2, GIT_CONFIG_LEVEL_APP));
+ cl_git_pass(git_config_file_get_string(&entry, cfg2, "section.name"));
+ cl_assert_equal_s("value", entry->value);
+ git_config_entry_free(entry);
+ cl_git_fail_with(GIT_ENOTFOUND, git_config_file_get_string(&entry, cfg2, "section2.name3"));
+ git_config_file_free(cfg2);
+
+ git_config_file_unlock(cfg, true);
+ git_config_file_free(cfg);
+
+ /* Now that we've unlocked it, we should see both updates */
+ cl_git_pass(git_config_file__ondisk(&cfg, filename));
+ cl_git_pass(git_config_file_open(cfg, GIT_CONFIG_LEVEL_APP));
+ cl_git_pass(git_config_file_get_string(&entry, cfg, "section.name"));
+ cl_assert_equal_s("other value", entry->value);
+ git_config_entry_free(entry);
+ cl_git_pass(git_config_file_get_string(&entry, cfg, "section2.name3"));
+ cl_assert_equal_s("more value", entry->value);
+ git_config_entry_free(entry);
+
+ git_config_file_free(cfg);
+}