From 0f0f565507565520759bffc22976c583497ec01f Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 19 Aug 2013 10:42:48 -0700 Subject: [PATCH 01/19] Don't try to pack symbolic refs If there were symbolic refs among the loose refs then the code to create packed-refs would fail trying to parse the OID out of them (where Git just skips trying to pack them). This fixes it. --- src/refdb_fs.c | 11 ++++++++++- tests-clar/refs/pack.c | 30 ++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index acd82594b..658bebb61 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -316,6 +316,12 @@ static int loose_lookup_to_packfile( if (reference_read(&ref_file, NULL, backend->path, name, NULL) < 0) return -1; + /* skip symbolic refs */ + if (!git__prefixcmp(git_buf_cstr(&ref_file), GIT_SYMREF)) { + git_buf_free(&ref_file); + return 0; + } + git_buf_rtrim(&ref_file); name_len = strlen(name); @@ -355,6 +361,9 @@ static int _dirent_loose_load(void *data, git_buf *full_path) if (loose_lookup_to_packfile(&ref, backend, file_path) < 0) return -1; + if (!ref) + return 0; + git_strmap_insert2( backend->refcache.packfile, ref->name, ref, old_ref, err); if (err < 0) { @@ -460,7 +469,7 @@ static int loose_lookup( if (error < 0) goto done; - if (git__prefixcmp((const char *)(ref_file.ptr), GIT_SYMREF) == 0) { + if (git__prefixcmp(git_buf_cstr(&ref_file), GIT_SYMREF) == 0) { git_buf_rtrim(&ref_file); if ((target = loose_parse_symbolic(&ref_file)) == NULL) { diff --git a/tests-clar/refs/pack.c b/tests-clar/refs/pack.c index d8d5cc6d0..6019ed75a 100644 --- a/tests-clar/refs/pack.c +++ b/tests-clar/refs/pack.c @@ -32,7 +32,7 @@ static void packall(void) void test_refs_pack__empty(void) { - // create a packfile for an empty folder + /* create a packfile for an empty folder */ git_buf temp_path = GIT_BUF_INIT; cl_git_pass(git_buf_join_n(&temp_path, '/', 3, git_repository_path(g_repo), GIT_REFS_HEADS_DIR, "empty_dir")); @@ -44,7 +44,7 @@ void test_refs_pack__empty(void) void test_refs_pack__loose(void) { - // create a packfile from all the loose rn a repo + /* create a packfile from all the loose rn a repo */ git_reference *reference; git_buf temp_path = GIT_BUF_INIT; @@ -77,3 +77,29 @@ void test_refs_pack__loose(void) git_reference_free(reference); git_buf_free(&temp_path); } + +void test_refs_pack__symbolic(void) +{ + /* create a packfile from loose refs skipping symbolic refs */ + int i; + git_oid head; + git_reference *ref; + char name[128]; + + cl_git_pass(git_reference_name_to_id(&head, g_repo, "HEAD")); + + /* make a bunch of references */ + + for (i = 0; i < 100; ++i) { + snprintf(name, sizeof(name), "refs/heads/symbolic-%03d", i); + cl_git_pass(git_reference_symbolic_create( + &ref, g_repo, name, "refs/heads/master", 0)); + git_reference_free(ref); + + snprintf(name, sizeof(name), "refs/heads/direct-%03d", i); + cl_git_pass(git_reference_create(&ref, g_repo, name, &head, 0)); + git_reference_free(ref); + } + + packall(); +} From 0b7cdc02637bcc8491153a476460c9feab33f8ee Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 20 Aug 2013 15:18:48 -0700 Subject: [PATCH 02/19] Add sorted cache data type This adds a convenient new data type for caching the contents of file in memory when each item in that file corresponds to a name and you need to both be able to lookup items by name and iterate over them in some sorted order. The new data type has locks in place to manage usage in a threaded environment. --- src/sortedcache.c | 297 +++++++++++++++++++++++++++++++++ src/sortedcache.h | 101 ++++++++++++ src/vector.h | 5 + tests-clar/core/sortedcache.c | 298 ++++++++++++++++++++++++++++++++++ 4 files changed, 701 insertions(+) create mode 100644 src/sortedcache.c create mode 100644 src/sortedcache.h create mode 100644 tests-clar/core/sortedcache.c diff --git a/src/sortedcache.c b/src/sortedcache.c new file mode 100644 index 000000000..6015d616d --- /dev/null +++ b/src/sortedcache.c @@ -0,0 +1,297 @@ +#include "sortedcache.h" + +GIT__USE_STRMAP; + +int git_sortedcache_new( + git_sortedcache **out, + size_t item_path_offset, + git_sortedcache_free_item_fn free_item, + void *free_item_payload, + git_vector_cmp item_cmp, + const char *path) +{ + git_sortedcache *sc; + size_t pathlen; + + pathlen = path ? strlen(path) : 0; + + sc = git__calloc(sizeof(git_sortedcache) + pathlen + 1, 1); + GITERR_CHECK_ALLOC(sc); + + if (git_pool_init(&sc->pool, 1, 0) < 0 || + git_vector_init(&sc->items, 4, item_cmp) < 0 || + (sc->map = git_strmap_alloc()) == NULL) + goto fail; + + if (git_mutex_init(&sc->lock)) { + giterr_set(GITERR_OS, "Failed to initialize mutex"); + goto fail; + } + + sc->item_path_offset = item_path_offset; + sc->free_item = free_item; + sc->free_item_payload = free_item_payload; + GIT_REFCOUNT_INC(sc); + if (pathlen) + memcpy(sc->path, path, pathlen); + + *out = sc; + return 0; + +fail: + if (sc->map) + git_strmap_free(sc->map); + git_vector_free(&sc->items); + git_pool_clear(&sc->pool); + git__free(sc); + return -1; +} + +void git_sortedcache_incref(git_sortedcache *sc) +{ + GIT_REFCOUNT_INC(sc); +} + +static void sortedcache_clear(git_sortedcache *sc) +{ + git_strmap_clear(sc->map); + + if (sc->free_item) { + size_t i; + void *item; + + git_vector_foreach(&sc->items, i, item) { + sc->free_item(sc->free_item_payload, item); + } + } + + git_vector_clear(&sc->items); + + git_pool_clear(&sc->pool); +} + +static void sortedcache_free(git_sortedcache *sc) +{ + if (git_mutex_lock(&sc->lock) < 0) { + giterr_set(GITERR_OS, "Unable to acquire mutex lock for free"); + return; + } + + sortedcache_clear(sc); + + git_vector_free(&sc->items); + git_strmap_free(sc->map); + + git_mutex_unlock(&sc->lock); + git_mutex_free(&sc->lock); + + git__free(sc); +} + +void git_sortedcache_free(git_sortedcache *sc) +{ + if (!sc) + return; + GIT_REFCOUNT_DEC(sc, sortedcache_free); +} + +static int sortedcache_copy_item(void *payload, void *tgt_item, void *src_item) +{ + git_sortedcache *sc = payload; + /* path will already have been copied by upsert */ + memcpy(tgt_item, src_item, sc->item_path_offset); + return 0; +} + +/* copy a sorted cache */ +int git_sortedcache_copy( + git_sortedcache **out, + git_sortedcache *src, + int (*copy_item)(void *payload, void *tgt_item, void *src_item), + void *payload) +{ + git_sortedcache *tgt; + size_t i; + void *src_item, *tgt_item; + + if (!copy_item) { + copy_item = sortedcache_copy_item; + payload = src; + } + + if (git_sortedcache_new( + &tgt, src->item_path_offset, + src->free_item, src->free_item_payload, + src->items._cmp, src->path) < 0) + return -1; + + if (git_sortedcache_lock(src) < 0) { + git_sortedcache_free(tgt); + return -1; + } + + if (git_sortedcache_lock(tgt) < 0) + goto fail; + + git_vector_foreach(&src->items, i, src_item) { + if (git_sortedcache_upsert( + &tgt_item, tgt, ((char *)src_item) + src->item_path_offset) < 0) + goto fail; + if (copy_item(payload, tgt_item, src_item) < 0) + goto fail; + } + + git_sortedcache_unlock(tgt); + git_sortedcache_unlock(src); + + *out = tgt; + return 0; + +fail: + git_sortedcache_unlock(src); + git_sortedcache_free(tgt); + return -1; +} + +/* release all items in sorted cache */ +void git_sortedcache_clear(git_sortedcache *sc, bool lock) +{ + if (lock && git_mutex_lock(&sc->lock) < 0) { + giterr_set(GITERR_OS, "Unable to acquire mutex lock for clear"); + return; + } + + sortedcache_clear(sc); + + if (lock) + git_mutex_unlock(&sc->lock); +} + +/* check file stamp to see if reload is required */ +bool git_sortedcache_out_of_date(git_sortedcache *sc) +{ + return (git_futils_filestamp_check(&sc->stamp, sc->path) != 0); +} + +/* lock sortedcache while making modifications */ +int git_sortedcache_lock(git_sortedcache *sc) +{ + if (git_mutex_lock(&sc->lock) < 0) { + giterr_set(GITERR_OS, "Unable to acquire mutex lock"); + return -1; + } + return 0; +} + +/* unlock sorted cache when done with modifications */ +int git_sortedcache_unlock(git_sortedcache *sc) +{ + git_vector_sort(&sc->items); + git_mutex_unlock(&sc->lock); + return 0; +} + +/* if the file has changed, lock cache and load file contents into buf; + * returns <0 on error, >0 if file has not changed + */ +int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf) +{ + int error, fd; + + if ((error = git_sortedcache_lock(sc)) < 0) + return error; + + if ((error = git_futils_filestamp_check(&sc->stamp, sc->path)) <= 0) + goto unlock; + + if (!git__is_sizet(sc->stamp.size)) { + giterr_set(GITERR_INVALID, "Unable to load file larger than size_t"); + error = -1; + goto unlock; + } + + if ((fd = git_futils_open_ro(sc->path)) < 0) { + error = fd; + goto unlock; + } + + if (buf) + error = git_futils_readbuffer_fd(buf, fd, (size_t)sc->stamp.size); + + (void)p_close(fd); + + if (error < 0) + goto unlock; + + return 1; /* return 1 -> file needs reload and was successfully loaded */ + +unlock: + git_sortedcache_unlock(sc); + return error; +} + +/* find and/or insert item, returning pointer to item data */ +int git_sortedcache_upsert( + void **out, git_sortedcache *sc, const char *key) +{ + int error = 0; + khiter_t pos; + void *item; + size_t keylen; + char *item_key; + + pos = git_strmap_lookup_index(sc->map, key); + if (git_strmap_valid_index(sc->map, pos)) { + item = git_strmap_value_at(sc->map, pos); + goto done; + } + + keylen = strlen(key); + item = git_pool_mallocz(&sc->pool, sc->item_path_offset + keylen + 1); + GITERR_CHECK_ALLOC(item); + + /* one strange thing is that even if the vector or hash table insert + * fail, there is no way to free the pool item so we just abandon it + */ + + item_key = ((char *)item) + sc->item_path_offset; + memcpy(item_key, key, keylen); + + pos = kh_put(str, sc->map, item_key, &error); + if (error < 0) + goto done; + + if (!error) + kh_key(sc->map, pos) = item_key; + kh_val(sc->map, pos) = item; + + error = git_vector_insert(&sc->items, item); + if (error < 0) + git_strmap_delete_at(sc->map, pos); + +done: + if (out) + *out = !error ? item : NULL; + return error; +} + +/* lookup item by key */ +void *git_sortedcache_lookup(const git_sortedcache *sc, const char *key) +{ + khiter_t pos = git_strmap_lookup_index(sc->map, key); + if (git_strmap_valid_index(sc->map, pos)) + return git_strmap_value_at(sc->map, pos); + return NULL; +} + +/* find out how many items are in the cache */ +size_t git_sortedcache_entrycount(const git_sortedcache *sc) +{ + return git_vector_length(&sc->items); +} + +/* lookup item by index */ +void *git_sortedcache_entry(const git_sortedcache *sc, size_t pos) +{ + return git_vector_get(&sc->items, pos); +} diff --git a/src/sortedcache.h b/src/sortedcache.h new file mode 100644 index 000000000..5d0d8f5a7 --- /dev/null +++ b/src/sortedcache.h @@ -0,0 +1,101 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_sorted_cache_h__ +#define INCLUDE_sorted_cache_h__ + +#include "util.h" +#include "fileops.h" +#include "vector.h" +#include "thread-utils.h" +#include "pool.h" +#include "strmap.h" + +/* + * The purpose of this data structure is to cache the parsed contents of a + * file where each item in the file can be identified by a key string and + * you want to both look them up by name and traverse them in sorted + * order. Each item is assumed to itself end in a GIT_FLEX_ARRAY. + */ + +typedef void (*git_sortedcache_free_item_fn)(void *payload, void *item); + +typedef struct { + git_refcount rc; + git_mutex lock; + size_t item_path_offset; + git_sortedcache_free_item_fn free_item; + void *free_item_payload; + git_pool pool; + git_vector items; + git_strmap *map; + git_futils_filestamp stamp; + char path[GIT_FLEX_ARRAY]; +} git_sortedcache; + +/* create a new sortedcache + * + * even though every sortedcache stores items with a GIT_FLEX_ARRAY at + * the end containing their key string, you have to provide the item_cmp + * sorting function because the sorting function doesn't get a payload + * and therefore can't know the offset to the item key string. :-( + */ +int git_sortedcache_new( + git_sortedcache **out, + size_t item_path_offset, /* use offsetof() macro */ + git_sortedcache_free_item_fn free_item, + void *free_item_payload, + git_vector_cmp item_cmp, + const char *path); + +/* copy a sorted cache + * + * - copy_item can be NULL to memcpy + * - locks src while copying + */ +int git_sortedcache_copy( + git_sortedcache **out, + git_sortedcache *src, + int (*copy_item)(void *payload, void *tgt_item, void *src_item), + void *payload); + +/* free sorted cache (first calling free_item callbacks) */ +void git_sortedcache_free(git_sortedcache *sc); + +/* increment reference count */ +void git_sortedcache_incref(git_sortedcache *sc); + +/* release all items in sorted cache - lock during clear if lock is true */ +void git_sortedcache_clear(git_sortedcache *sc, bool lock); + +/* check file stamp to see if reload is required */ +bool git_sortedcache_out_of_date(git_sortedcache *sc); + +/* lock sortedcache while making modifications */ +int git_sortedcache_lock(git_sortedcache *sc); + +/* unlock sorted cache when done with modifications */ +int git_sortedcache_unlock(git_sortedcache *sc); + +/* if the file has changed, lock cache and load file contents into buf; + * @return 0 if up-to-date, 1 if out-of-date, <0 on error + */ +int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf); + +/* find and/or insert item, returning pointer to item data - lock first */ +int git_sortedcache_upsert( + void **out, git_sortedcache *sc, const char *key); + +/* lookup item by key */ +void *git_sortedcache_lookup(const git_sortedcache *sc, const char *key); + +/* find out how many items are in the cache */ +size_t git_sortedcache_entrycount(const git_sortedcache *sc); + +/* lookup item by index */ +void *git_sortedcache_entry(const git_sortedcache *sc, size_t pos); + +#endif diff --git a/src/vector.h b/src/vector.h index 1bda9c93d..ad1c34ea1 100644 --- a/src/vector.h +++ b/src/vector.h @@ -55,6 +55,11 @@ GIT_INLINE(void *) git_vector_get(const git_vector *v, size_t position) #define GIT_VECTOR_GET(V,I) ((I) < (V)->length ? (V)->contents[(I)] : NULL) +GIT_INLINE(size_t) git_vector_length(const git_vector *v) +{ + return v->length; +} + GIT_INLINE(void *) git_vector_last(const git_vector *v) { return (v->length > 0) ? git_vector_get(v, v->length - 1) : NULL; diff --git a/tests-clar/core/sortedcache.c b/tests-clar/core/sortedcache.c new file mode 100644 index 000000000..f192af31d --- /dev/null +++ b/tests-clar/core/sortedcache.c @@ -0,0 +1,298 @@ +#include "clar_libgit2.h" +#include "sortedcache.h" + +static int name_only_cmp(const void *a, const void *b) +{ + return strcmp(a, b); +} + +void test_core_sortedcache__name_only(void) +{ + git_sortedcache *sc; + void *item; + + cl_git_pass(git_sortedcache_new( + &sc, 0, NULL, NULL, name_only_cmp, NULL)); + + cl_git_pass(git_sortedcache_lock(sc)); + cl_git_pass(git_sortedcache_upsert(&item, sc, "aaa")); + cl_git_pass(git_sortedcache_upsert(&item, sc, "bbb")); + cl_git_pass(git_sortedcache_upsert(&item, sc, "zzz")); + cl_git_pass(git_sortedcache_upsert(&item, sc, "mmm")); + cl_git_pass(git_sortedcache_upsert(&item, sc, "iii")); + cl_git_pass(git_sortedcache_unlock(sc)); + + cl_assert_equal_sz(5, git_sortedcache_entrycount(sc)); + + cl_assert((item = git_sortedcache_lookup(sc, "aaa")) != NULL); + cl_assert_equal_s("aaa", item); + cl_assert((item = git_sortedcache_lookup(sc, "mmm")) != NULL); + cl_assert_equal_s("mmm", item); + cl_assert((item = git_sortedcache_lookup(sc, "zzz")) != NULL); + cl_assert_equal_s("zzz", item); + cl_assert(git_sortedcache_lookup(sc, "qqq") == NULL); + + cl_assert((item = git_sortedcache_entry(sc, 0)) != NULL); + cl_assert_equal_s("aaa", item); + cl_assert((item = git_sortedcache_entry(sc, 1)) != NULL); + cl_assert_equal_s("bbb", item); + cl_assert((item = git_sortedcache_entry(sc, 2)) != NULL); + cl_assert_equal_s("iii", item); + cl_assert((item = git_sortedcache_entry(sc, 3)) != NULL); + cl_assert_equal_s("mmm", item); + cl_assert((item = git_sortedcache_entry(sc, 4)) != NULL); + cl_assert_equal_s("zzz", item); + cl_assert(git_sortedcache_entry(sc, 5) == NULL); + + git_sortedcache_clear(sc, true); + + cl_assert_equal_sz(0, git_sortedcache_entrycount(sc)); + cl_assert(git_sortedcache_entry(sc, 0) == NULL); + cl_assert(git_sortedcache_lookup(sc, "aaa") == NULL); + cl_assert(git_sortedcache_entry(sc, 0) == NULL); + + git_sortedcache_free(sc); +} + +typedef struct { + int value; + char smaller_value; + char path[GIT_FLEX_ARRAY]; +} sortedcache_test_struct; + +static int sortedcache_test_struct_cmp(const void *a_, const void *b_) +{ + const sortedcache_test_struct *a = a_, *b = b_; + return strcmp(a->path, b->path); +} + +static void sortedcache_test_struct_free(void *payload, void *item_) +{ + sortedcache_test_struct *item = item_; + int *count = payload; + (*count)++; + item->smaller_value = 0; +} + +void test_core_sortedcache__in_memory(void) +{ + git_sortedcache *sc; + sortedcache_test_struct *item; + int free_count = 0; + + cl_git_pass(git_sortedcache_new( + &sc, offsetof(sortedcache_test_struct, path), + sortedcache_test_struct_free, &free_count, + sortedcache_test_struct_cmp, NULL)); + + cl_git_pass(git_sortedcache_lock(sc)); + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "aaa")); + item->value = 10; + item->smaller_value = 1; + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "bbb")); + item->value = 20; + item->smaller_value = 2; + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "zzz")); + item->value = 30; + item->smaller_value = 26; + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "mmm")); + item->value = 40; + item->smaller_value = 14; + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "iii")); + item->value = 50; + item->smaller_value = 9; + cl_git_pass(git_sortedcache_unlock(sc)); + + cl_assert_equal_sz(5, git_sortedcache_entrycount(sc)); + + cl_assert((item = git_sortedcache_lookup(sc, "aaa")) != NULL); + cl_assert_equal_s("aaa", item->path); + cl_assert_equal_i(10, item->value); + cl_assert((item = git_sortedcache_lookup(sc, "mmm")) != NULL); + cl_assert_equal_s("mmm", item->path); + cl_assert_equal_i(40, item->value); + cl_assert((item = git_sortedcache_lookup(sc, "zzz")) != NULL); + cl_assert_equal_s("zzz", item->path); + cl_assert_equal_i(30, item->value); + cl_assert(git_sortedcache_lookup(sc, "abc") == NULL); + + cl_assert((item = git_sortedcache_entry(sc, 0)) != NULL); + cl_assert_equal_s("aaa", item->path); + cl_assert_equal_i(10, item->value); + cl_assert((item = git_sortedcache_entry(sc, 1)) != NULL); + cl_assert_equal_s("bbb", item->path); + cl_assert_equal_i(20, item->value); + cl_assert((item = git_sortedcache_entry(sc, 2)) != NULL); + cl_assert_equal_s("iii", item->path); + cl_assert_equal_i(50, item->value); + cl_assert((item = git_sortedcache_entry(sc, 3)) != NULL); + cl_assert_equal_s("mmm", item->path); + cl_assert_equal_i(40, item->value); + cl_assert((item = git_sortedcache_entry(sc, 4)) != NULL); + cl_assert_equal_s("zzz", item->path); + cl_assert_equal_i(30, item->value); + cl_assert(git_sortedcache_entry(sc, 5) == NULL); + + cl_assert_equal_i(0, free_count); + + git_sortedcache_clear(sc, true); + + cl_assert_equal_i(5, free_count); + + cl_assert_equal_sz(0, git_sortedcache_entrycount(sc)); + cl_assert(git_sortedcache_entry(sc, 0) == NULL); + cl_assert(git_sortedcache_lookup(sc, "aaa") == NULL); + cl_assert(git_sortedcache_entry(sc, 0) == NULL); + + free_count = 0; + + cl_git_pass(git_sortedcache_lock(sc)); + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "testing")); + item->value = 10; + item->smaller_value = 3; + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "again")); + item->value = 20; + item->smaller_value = 1; + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "final")); + item->value = 30; + item->smaller_value = 2; + cl_git_pass(git_sortedcache_unlock(sc)); + + cl_assert_equal_sz(3, git_sortedcache_entrycount(sc)); + + cl_assert((item = git_sortedcache_lookup(sc, "testing")) != NULL); + cl_assert_equal_s("testing", item->path); + cl_assert_equal_i(10, item->value); + cl_assert((item = git_sortedcache_lookup(sc, "again")) != NULL); + cl_assert_equal_s("again", item->path); + cl_assert_equal_i(20, item->value); + cl_assert((item = git_sortedcache_lookup(sc, "final")) != NULL); + cl_assert_equal_s("final", item->path); + cl_assert_equal_i(30, item->value); + cl_assert(git_sortedcache_lookup(sc, "zzz") == NULL); + + cl_assert((item = git_sortedcache_entry(sc, 0)) != NULL); + cl_assert_equal_s("again", item->path); + cl_assert_equal_i(20, item->value); + cl_assert((item = git_sortedcache_entry(sc, 1)) != NULL); + cl_assert_equal_s("final", item->path); + cl_assert_equal_i(30, item->value); + cl_assert((item = git_sortedcache_entry(sc, 2)) != NULL); + cl_assert_equal_s("testing", item->path); + cl_assert_equal_i(10, item->value); + cl_assert(git_sortedcache_entry(sc, 3) == NULL); + + git_sortedcache_free(sc); + + cl_assert_equal_i(3, free_count); +} + +static void sortedcache_test_reload(git_sortedcache *sc) +{ + int count = 0; + git_buf buf = GIT_BUF_INIT; + char *scan, *after; + sortedcache_test_struct *item; + + cl_assert(git_sortedcache_lockandload(sc, &buf) > 0); + + git_sortedcache_clear(sc, false); /* clear once we already have lock */ + + for (scan = buf.ptr; *scan; scan = after + 1) { + int val = strtol(scan, &after, 0); + cl_assert(after > scan); + scan = after; + + for (scan = after; git__isspace(*scan); ++scan) /* find start */; + for (after = scan; *after && *after != '\n'; ++after) /* find eol */; + *after = '\0'; + + cl_git_pass(git_sortedcache_upsert((void **)&item, sc, scan)); + + item->value = val; + item->smaller_value = (char)(count++); + } + + cl_git_pass(git_sortedcache_unlock(sc)); + + git_buf_free(&buf); +} + +void test_core_sortedcache__on_disk(void) +{ + git_sortedcache *sc; + sortedcache_test_struct *item; + int free_count = 0; + + cl_git_mkfile("cacheitems.txt", "10 abc\n20 bcd\n30 cde\n"); + + cl_git_pass(git_sortedcache_new( + &sc, offsetof(sortedcache_test_struct, path), + sortedcache_test_struct_free, &free_count, + sortedcache_test_struct_cmp, "cacheitems.txt")); + + /* should need to reload the first time */ + + sortedcache_test_reload(sc); + + /* test what we loaded */ + + cl_assert_equal_sz(3, git_sortedcache_entrycount(sc)); + + cl_assert((item = git_sortedcache_lookup(sc, "abc")) != NULL); + cl_assert_equal_s("abc", item->path); + cl_assert_equal_i(10, item->value); + cl_assert((item = git_sortedcache_lookup(sc, "cde")) != NULL); + cl_assert_equal_s("cde", item->path); + cl_assert_equal_i(30, item->value); + cl_assert(git_sortedcache_lookup(sc, "aaa") == NULL); + + cl_assert((item = git_sortedcache_entry(sc, 0)) != NULL); + cl_assert_equal_s("abc", item->path); + cl_assert_equal_i(10, item->value); + cl_assert((item = git_sortedcache_entry(sc, 1)) != NULL); + cl_assert_equal_s("bcd", item->path); + cl_assert_equal_i(20, item->value); + cl_assert(git_sortedcache_entry(sc, 3) == NULL); + + /* should not need to reload this time */ + + cl_assert_equal_i(0, git_sortedcache_lockandload(sc, NULL)); + + /* rewrite ondisk file and reload */ + + cl_assert_equal_i(0, free_count); + + cl_git_rewritefile( + "cacheitems.txt", "100 abc\n200 zzz\n500 aaa\n10 final\n"); + sortedcache_test_reload(sc); + + cl_assert_equal_i(3, free_count); + + /* test what we loaded */ + + cl_assert_equal_sz(4, git_sortedcache_entrycount(sc)); + + cl_assert((item = git_sortedcache_lookup(sc, "abc")) != NULL); + cl_assert_equal_s("abc", item->path); + cl_assert_equal_i(100, item->value); + cl_assert((item = git_sortedcache_lookup(sc, "final")) != NULL); + cl_assert_equal_s("final", item->path); + cl_assert_equal_i(10, item->value); + cl_assert(git_sortedcache_lookup(sc, "cde") == NULL); + + cl_assert((item = git_sortedcache_entry(sc, 0)) != NULL); + cl_assert_equal_s("aaa", item->path); + cl_assert_equal_i(500, item->value); + cl_assert((item = git_sortedcache_entry(sc, 2)) != NULL); + cl_assert_equal_s("final", item->path); + cl_assert_equal_i(10, item->value); + cl_assert((item = git_sortedcache_entry(sc, 3)) != NULL); + cl_assert_equal_s("zzz", item->path); + cl_assert_equal_i(200, item->value); + + git_sortedcache_free(sc); + + cl_assert_equal_i(7, free_count); +} + From a4977169e1e1920ed33f10a77e5dd8706f103f4f Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 21 Aug 2013 14:09:38 -0700 Subject: [PATCH 03/19] Add sortedcache APIs to lookup index and remove This adds two other APIs that I need to the sortedcache type. --- src/sortedcache.c | 59 +++++++++++++++++++++++++++++++++++ src/sortedcache.h | 7 +++++ tests-clar/core/sortedcache.c | 52 ++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/src/sortedcache.c b/src/sortedcache.c index 6015d616d..2efa3c4e9 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -295,3 +295,62 @@ void *git_sortedcache_entry(const git_sortedcache *sc, size_t pos) { return git_vector_get(&sc->items, pos); } + +struct sortedcache_magic_key { + size_t offset; + const char *key; +}; + +static int sortedcache_magic_cmp(const void *key, const void *value) +{ + const struct sortedcache_magic_key *magic = key; + const char *value_key = ((const char *)value) + magic->offset; + return strcmp(magic->key, value_key); +} + +/* lookup index of item by key */ +int git_sortedcache_lookup_index( + size_t *out, git_sortedcache *sc, const char *key) +{ + struct sortedcache_magic_key magic; + + magic.offset = sc->item_path_offset; + magic.key = key; + + return git_vector_bsearch2(out, &sc->items, sortedcache_magic_cmp, &magic); +} + +/* remove entry from cache */ +int git_sortedcache_remove(git_sortedcache *sc, size_t pos, bool lock) +{ + int error = 0; + char *item; + khiter_t mappos; + + if (lock && git_sortedcache_lock(sc) < 0) + return -1; + + /* because of pool allocation, this can't actually remove the item, + * but we can remove it from the items vector and the hash table. + */ + + if ((item = git_vector_get(&sc->items, pos)) == NULL) { + giterr_set(GITERR_INVALID, "Removing item out of range"); + error = GIT_ENOTFOUND; + goto done; + } + + (void)git_vector_remove(&sc->items, pos); + + mappos = git_strmap_lookup_index(sc->map, item + sc->item_path_offset); + git_strmap_delete_at(sc->map, mappos); + + if (sc->free_item) + sc->free_item(sc->free_item_payload, item); + +done: + if (lock) + git_sortedcache_unlock(sc); + return error; +} + diff --git a/src/sortedcache.h b/src/sortedcache.h index 5d0d8f5a7..f63ad645b 100644 --- a/src/sortedcache.h +++ b/src/sortedcache.h @@ -98,4 +98,11 @@ size_t git_sortedcache_entrycount(const git_sortedcache *sc); /* lookup item by index */ void *git_sortedcache_entry(const git_sortedcache *sc, size_t pos); +/* lookup index of item by key */ +int git_sortedcache_lookup_index( + size_t *out, git_sortedcache *sc, const char *key); + +/* remove entry from cache */ +int git_sortedcache_remove(git_sortedcache *sc, size_t pos, bool lock); + #endif diff --git a/tests-clar/core/sortedcache.c b/tests-clar/core/sortedcache.c index f192af31d..91415943d 100644 --- a/tests-clar/core/sortedcache.c +++ b/tests-clar/core/sortedcache.c @@ -10,6 +10,7 @@ void test_core_sortedcache__name_only(void) { git_sortedcache *sc; void *item; + size_t pos; cl_git_pass(git_sortedcache_new( &sc, 0, NULL, NULL, name_only_cmp, NULL)); @@ -44,6 +45,15 @@ void test_core_sortedcache__name_only(void) cl_assert_equal_s("zzz", item); cl_assert(git_sortedcache_entry(sc, 5) == NULL); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "aaa")); + cl_assert_equal_sz(0, pos); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "iii")); + cl_assert_equal_sz(2, pos); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "zzz")); + cl_assert_equal_sz(4, pos); + cl_assert_equal_i( + GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "abc")); + git_sortedcache_clear(sc, true); cl_assert_equal_sz(0, git_sortedcache_entrycount(sc)); @@ -182,6 +192,34 @@ void test_core_sortedcache__in_memory(void) cl_assert_equal_i(10, item->value); cl_assert(git_sortedcache_entry(sc, 3) == NULL); + { + size_t pos; + + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "again")); + cl_assert_equal_sz(0, pos); + cl_git_pass(git_sortedcache_remove(sc, pos, true)); + cl_assert_equal_i( + GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "again")); + + cl_assert_equal_sz(2, git_sortedcache_entrycount(sc)); + + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "testing")); + cl_assert_equal_sz(1, pos); + cl_git_pass(git_sortedcache_remove(sc, pos, true)); + cl_assert_equal_i( + GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "testing")); + + cl_assert_equal_sz(1, git_sortedcache_entrycount(sc)); + + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "final")); + cl_assert_equal_sz(0, pos); + cl_git_pass(git_sortedcache_remove(sc, pos, true)); + cl_assert_equal_i( + GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "final")); + + cl_assert_equal_sz(0, git_sortedcache_entrycount(sc)); + } + git_sortedcache_free(sc); cl_assert_equal_i(3, free_count); @@ -223,6 +261,7 @@ void test_core_sortedcache__on_disk(void) git_sortedcache *sc; sortedcache_test_struct *item; int free_count = 0; + size_t pos; cl_git_mkfile("cacheitems.txt", "10 abc\n20 bcd\n30 cde\n"); @@ -291,6 +330,19 @@ void test_core_sortedcache__on_disk(void) cl_assert_equal_s("zzz", item->path); cl_assert_equal_i(200, item->value); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "aaa")); + cl_assert_equal_sz(0, pos); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "abc")); + cl_assert_equal_sz(1, pos); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "final")); + cl_assert_equal_sz(2, pos); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "zzz")); + cl_assert_equal_sz(3, pos); + cl_assert_equal_i( + GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "missing")); + cl_assert_equal_i( + GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "cde")); + git_sortedcache_free(sc); cl_assert_equal_i(7, free_count); From 24c71f14b4dcb696d9d87330322c12fd2c185317 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 21 Aug 2013 14:10:27 -0700 Subject: [PATCH 04/19] Add internal ref set_name fn instead of realloc The refdb_fs implementation calls realloc directly on a reference object when it wants to rename it. It is not a public object, so this doesn't mess with the immutability of references, but it does assume certain constraints on the reference representation. This commit wraps that assumption in an isolated API to isolate it. --- src/refs.c | 11 +++++++++++ src/refs.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/src/refs.c b/src/refs.c index c0e460cc3..e59b34aae 100644 --- a/src/refs.c +++ b/src/refs.c @@ -88,6 +88,17 @@ git_reference *git_reference__alloc( return ref; } +git_reference *git_reference__set_name( + git_reference *ref, const char *name) +{ + size_t namelen = strlen(name); + git_reference *rewrite = + git__realloc(ref, sizeof(git_reference) + namelen + 1); + if (rewrite != NULL) + memcpy(rewrite->name, name, namelen + 1); + return rewrite; +} + void git_reference_free(git_reference *reference) { if (reference == NULL) diff --git a/src/refs.h b/src/refs.h index f487ee3fc..4bb49d02a 100644 --- a/src/refs.h +++ b/src/refs.h @@ -61,6 +61,8 @@ struct git_reference { char name[0]; }; +git_reference *git_reference__set_name(git_reference *ref, const char *name); + int git_reference__normalize_name_lax(char *buffer_out, size_t out_size, const char *name); int git_reference__normalize_name(git_buf *buf, const char *name, unsigned int flags); int git_reference__update_terminal(git_repository *repo, const char *ref_name, const git_oid *oid); From fe3727408001f9708aad09562ffae5b82c0f7171 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 21 Aug 2013 16:26:32 -0700 Subject: [PATCH 05/19] Rewrite refdb_fs using git_sortedcache object This adds thread safety to the refdb_fs by using the new git_sortedcache object and also by relaxing the handling of some filesystem errors where the fs may be changed out from under us. This also adds some new threading tests that hammer on the refdb. --- src/refdb_fs.c | 830 +++++++++++++++---------------------- tests-clar/threads/refdb.c | 200 +++++++++ 2 files changed, 524 insertions(+), 506 deletions(-) create mode 100644 tests-clar/threads/refdb.c diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 658bebb61..876e84588 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -15,6 +15,7 @@ #include "refdb.h" #include "refdb_fs.h" #include "iterator.h" +#include "sortedcache.h" #include #include @@ -53,243 +54,149 @@ typedef struct refdb_fs_backend { git_repository *repo; char *path; - git_refcache refcache; + git_sortedcache *refcache; int peeling_mode; } refdb_fs_backend; -static int reference_read( - git_buf *file_content, - time_t *mtime, - const char *repo_path, - const char *ref_name, - int *updated) +static int packref_cmp(const void *a_, const void *b_) { - git_buf path = GIT_BUF_INIT; - int result; - - assert(file_content && repo_path && ref_name); - - /* Determine the full path of the file */ - if (git_buf_joinpath(&path, repo_path, ref_name) < 0) - return -1; - - result = git_futils_readbuffer_updated(file_content, path.ptr, mtime, NULL, updated); - git_buf_free(&path); - - return result; + const struct packref *a = a_, *b = b_; + return strcmp(a->name, b->name); } -static int packed_parse_oid( - struct packref **ref_out, - const char **buffer_out, - const char *buffer_end) +static int packed_reload(refdb_fs_backend *backend) { - struct packref *ref = NULL; + int error; + git_buf packedrefs = GIT_BUF_INIT; + char *scan, *eof, *eol; - const char *buffer = *buffer_out; - const char *refname_begin, *refname_end; - - size_t refname_len; - git_oid id; - - refname_begin = (buffer + GIT_OID_HEXSZ + 1); - if (refname_begin >= buffer_end || refname_begin[-1] != ' ') - goto corrupt; - - /* Is this a valid object id? */ - if (git_oid_fromstr(&id, buffer) < 0) - goto corrupt; - - refname_end = memchr(refname_begin, '\n', buffer_end - refname_begin); - if (refname_end == NULL) - refname_end = buffer_end; - - if (refname_end[-1] == '\r') - refname_end--; - - refname_len = refname_end - refname_begin; - - ref = git__calloc(1, sizeof(struct packref) + refname_len + 1); - GITERR_CHECK_ALLOC(ref); - - memcpy(ref->name, refname_begin, refname_len); - ref->name[refname_len] = 0; - - git_oid_cpy(&ref->oid, &id); - - *ref_out = ref; - *buffer_out = refname_end + 1; - return 0; - -corrupt: - git__free(ref); - giterr_set(GITERR_REFERENCE, "The packed references file is corrupted"); - return -1; -} - -static int packed_parse_peel( - struct packref *tag_ref, - const char **buffer_out, - const char *buffer_end) -{ - const char *buffer = *buffer_out + 1; - - assert(buffer[-1] == '^'); - - /* Ensure it's not the first entry of the file */ - if (tag_ref == NULL) - goto corrupt; - - if (buffer + GIT_OID_HEXSZ > buffer_end) - goto corrupt; - - /* Is this a valid object id? */ - if (git_oid_fromstr(&tag_ref->peel, buffer) < 0) - goto corrupt; - - buffer = buffer + GIT_OID_HEXSZ; - if (*buffer == '\r') - buffer++; - - if (buffer != buffer_end) { - if (*buffer == '\n') - buffer++; - else - goto corrupt; - } - - tag_ref->flags |= PACKREF_HAS_PEEL; - *buffer_out = buffer; - return 0; - -corrupt: - giterr_set(GITERR_REFERENCE, "The packed references file is corrupted"); - return -1; -} - -static int packed_load(refdb_fs_backend *backend) -{ - int result, updated; - git_buf packfile = GIT_BUF_INIT; - const char *buffer_start, *buffer_end; - git_refcache *ref_cache = &backend->refcache; - - /* First we make sure we have allocated the hash table */ - if (ref_cache->packfile == NULL) { - ref_cache->packfile = git_strmap_alloc(); - GITERR_CHECK_ALLOC(ref_cache->packfile); - } - - if (backend->path == NULL) + if (!backend->path) return 0; - result = reference_read(&packfile, &ref_cache->packfile_time, - backend->path, GIT_PACKEDREFS_FILE, &updated); + error = git_sortedcache_lockandload(backend->refcache, &packedrefs); /* - * If we couldn't find the file, we need to clear the table and - * return. On any other error, we return that error. If everything - * went fine and the file wasn't updated, then there's nothing new - * for us here, so just return. Anything else means we need to - * refresh the packed refs. + * If we can't find the packed-refs, clear table and return. + * Any other error just gets passed through. + * If no error, and file wasn't changed, just return. + * Anything else means we need to refresh the packed refs. */ - if (result == GIT_ENOTFOUND) { - git_strmap_clear(ref_cache->packfile); - return 0; + if (error <= 0) { + if (error == GIT_ENOTFOUND) { + git_sortedcache_clear(backend->refcache, true); + giterr_clear(); + error = 0; + } + return error; } - if (result < 0) - return -1; + /* At this point, refresh the packed refs from the loaded buffer. */ - if (!updated) - return 0; + git_sortedcache_clear(backend->refcache, false); - /* - * At this point, we want to refresh the packed refs. We already - * have the contents in our buffer. - */ - git_strmap_clear(ref_cache->packfile); - - buffer_start = (const char *)packfile.ptr; - buffer_end = (const char *)(buffer_start) + packfile.size; + scan = (char *)packedrefs.ptr; + eof = scan + packedrefs.size; backend->peeling_mode = PEELING_NONE; - if (buffer_start[0] == '#') { + if (*scan == '#') { static const char *traits_header = "# pack-refs with: "; - if (git__prefixcmp(buffer_start, traits_header) == 0) { - char *traits = (char *)buffer_start + strlen(traits_header); - char *traits_end = strchr(traits, '\n'); + if (git__prefixcmp(scan, traits_header) == 0) { + scan += strlen(traits_header); + eol = strchr(scan, '\n'); - if (traits_end == NULL) + if (!eol) goto parse_failed; + *eol = '\0'; - *traits_end = '\0'; - - if (strstr(traits, " fully-peeled ") != NULL) { + if (strstr(scan, " fully-peeled ") != NULL) { backend->peeling_mode = PEELING_FULL; - } else if (strstr(traits, " peeled ") != NULL) { + } else if (strstr(scan, " peeled ") != NULL) { backend->peeling_mode = PEELING_STANDARD; } - buffer_start = traits_end + 1; + scan = eol + 1; } } - while (buffer_start < buffer_end && buffer_start[0] == '#') { - buffer_start = strchr(buffer_start, '\n'); - if (buffer_start == NULL) + while (scan < eof && *scan == '#') { + if (!(eol = strchr(scan, '\n'))) goto parse_failed; - - buffer_start++; + scan = eol + 1; } - while (buffer_start < buffer_end) { - int err; - struct packref *ref = NULL; + while (scan < eof) { + struct packref *ref; + git_oid oid; - if (packed_parse_oid(&ref, &buffer_start, buffer_end) < 0) + /* parse " \n" */ + + if (git_oid_fromstr(&oid, scan) < 0) goto parse_failed; + scan += GIT_OID_HEXSZ; - if (buffer_start[0] == '^') { - if (packed_parse_peel(ref, &buffer_start, buffer_end) < 0) + if (*scan++ != ' ') + goto parse_failed; + if (!(eol = strchr(scan, '\n'))) + goto parse_failed; + *eol = '\0'; + if (eol[-1] == '\r') + eol[-1] = '\0'; + + if (git_sortedcache_upsert((void **)&ref, backend->refcache, scan) < 0) + goto parse_failed; + scan = eol + 1; + + git_oid_cpy(&ref->oid, &oid); + + /* look for optional "^\n" */ + + if (*scan == '^') { + if (git_oid_fromstr(&oid, scan + 1) < 0) goto parse_failed; - } else if (backend->peeling_mode == PEELING_FULL || - (backend->peeling_mode == PEELING_STANDARD && - git__prefixcmp(ref->name, GIT_REFS_TAGS_DIR) == 0)) { - ref->flags |= PACKREF_CANNOT_PEEL; - } + scan += GIT_OID_HEXSZ + 1; - git_strmap_insert(ref_cache->packfile, ref->name, ref, err); - if (err < 0) - goto parse_failed; + if (scan < eof) { + if (!(eol = strchr(scan, '\n'))) + goto parse_failed; + scan = eol + 1; + } + + git_oid_cpy(&ref->peel, &oid); + ref->flags |= PACKREF_HAS_PEEL; + } + else if (backend->peeling_mode == PEELING_FULL || + (backend->peeling_mode == PEELING_STANDARD && + git__prefixcmp(ref->name, GIT_REFS_TAGS_DIR) == 0)) + ref->flags |= PACKREF_CANNOT_PEEL; } - git_buf_free(&packfile); + git_sortedcache_unlock(backend->refcache); + git_buf_free(&packedrefs); + return 0; parse_failed: - git_strmap_free(ref_cache->packfile); - ref_cache->packfile = NULL; - git_buf_free(&packfile); + giterr_set(GITERR_REFERENCE, "Corrupted packed references file"); + + git_sortedcache_clear(backend->refcache, false); + git_sortedcache_unlock(backend->refcache); + git_buf_free(&packedrefs); + return -1; } -static int loose_parse_oid(git_oid *oid, const char *filename, git_buf *file_content) +static int loose_parse_oid( + git_oid *oid, const char *filename, git_buf *file_content) { - size_t len; - const char *str; + const char *str = git_buf_cstr(file_content); - len = git_buf_len(file_content); - if (len < GIT_OID_HEXSZ) + if (git_buf_len(file_content) < GIT_OID_HEXSZ) goto corrupted; - /* str is guranteed to be zero-terminated */ - str = git_buf_cstr(file_content); - /* we need to get 40 OID characters from the file */ - if (git_oid_fromstr(oid, git_buf_cstr(file_content)) < 0) + if (git_oid_fromstr(oid, str) < 0) goto corrupted; /* If the file is longer than 40 chars, the 41st must be a space */ @@ -302,77 +209,71 @@ corrupted: return -1; } -static int loose_lookup_to_packfile( - struct packref **ref_out, - refdb_fs_backend *backend, - const char *name) +static int loose_readbuffer(git_buf *buf, const char *base, const char *path) { - git_buf ref_file = GIT_BUF_INIT; - struct packref *ref = NULL; - size_t name_len; + int error; - *ref_out = NULL; + /* build full path to file */ + if ((error = git_buf_joinpath(buf, base, path)) < 0 || + (error = git_futils_readbuffer(buf, buf->ptr)) < 0) + git_buf_free(buf); - if (reference_read(&ref_file, NULL, backend->path, name, NULL) < 0) - return -1; - - /* skip symbolic refs */ - if (!git__prefixcmp(git_buf_cstr(&ref_file), GIT_SYMREF)) { - git_buf_free(&ref_file); - return 0; - } - - git_buf_rtrim(&ref_file); - - name_len = strlen(name); - ref = git__calloc(1, sizeof(struct packref) + name_len + 1); - GITERR_CHECK_ALLOC(ref); - - memcpy(ref->name, name, name_len); - ref->name[name_len] = 0; - - if (loose_parse_oid(&ref->oid, name, &ref_file) < 0) { - git_buf_free(&ref_file); - git__free(ref); - return -1; - } - - ref->flags = PACKREF_WAS_LOOSE; - - *ref_out = ref; - git_buf_free(&ref_file); - return 0; + return error; } +static int loose_lookup_to_packfile(refdb_fs_backend *backend, const char *name) +{ + int error = 0; + git_buf ref_file = GIT_BUF_INIT; + struct packref *ref = NULL; + git_oid oid; + + /* if we fail to load the loose reference, assume someone changed + * the filesystem under us and skip it... + */ + if (loose_readbuffer(&ref_file, backend->path, name) < 0) { + giterr_clear(); + goto done; + } + + /* skip symbolic refs */ + if (!git__prefixcmp(git_buf_cstr(&ref_file), GIT_SYMREF)) + goto done; + + /* parse OID from file */ + if ((error = loose_parse_oid(&oid, name, &ref_file)) < 0) + goto done; + + git_sortedcache_lock(backend->refcache); + + if (!(error = git_sortedcache_upsert( + (void **)&ref, backend->refcache, name))) { + + git_oid_cpy(&ref->oid, &oid); + ref->flags = PACKREF_WAS_LOOSE; + } + + git_sortedcache_unlock(backend->refcache); + +done: + git_buf_free(&ref_file); + return error; +} static int _dirent_loose_load(void *data, git_buf *full_path) { refdb_fs_backend *backend = (refdb_fs_backend *)data; - void *old_ref = NULL; - struct packref *ref; const char *file_path; - int err; - if (git_path_isdir(full_path->ptr) == true) + if (git__suffixcmp(full_path->ptr, ".lock") == 0) + return 0; + + if (git_path_isdir(full_path->ptr)) return git_path_direach(full_path, _dirent_loose_load, backend); file_path = full_path->ptr + strlen(backend->path); - if (loose_lookup_to_packfile(&ref, backend, file_path) < 0) - return -1; - - if (!ref) - return 0; - - git_strmap_insert2( - backend->refcache.packfile, ref->name, ref, old_ref, err); - if (err < 0) { - git__free(ref); - return -1; - } - - git__free(old_ref); - return 0; + return loose_lookup_to_packfile(backend, file_path); } /* @@ -383,11 +284,8 @@ static int _dirent_loose_load(void *data, git_buf *full_path) */ static int packed_loadloose(refdb_fs_backend *backend) { + int error; git_buf refs_path = GIT_BUF_INIT; - int result; - - /* the packfile must have been previously loaded! */ - assert(backend->refcache.packfile); if (git_buf_joinpath(&refs_path, backend->path, GIT_REFS_DIR) < 0) return -1; @@ -397,10 +295,11 @@ static int packed_loadloose(refdb_fs_backend *backend) * This will overwrite any old packed entries with their * updated loose versions */ - result = git_path_direach(&refs_path, _dirent_loose_load, backend); + error = git_path_direach(&refs_path, _dirent_loose_load, backend); + git_buf_free(&refs_path); - return result; + return error; } static int refdb_fs_backend__exists( @@ -408,23 +307,17 @@ static int refdb_fs_backend__exists( git_refdb_backend *_backend, const char *ref_name) { - refdb_fs_backend *backend; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; git_buf ref_path = GIT_BUF_INIT; - assert(_backend); - backend = (refdb_fs_backend *)_backend; + assert(backend); - if (packed_load(backend) < 0) + if (packed_reload(backend) < 0 || + git_buf_joinpath(&ref_path, backend->path, ref_name) < 0) return -1; - if (git_buf_joinpath(&ref_path, backend->path, ref_name) < 0) - return -1; - - if (git_path_isfile(ref_path.ptr) == true || - git_strmap_exists(backend->refcache.packfile, ref_path.ptr)) - *exists = 1; - else - *exists = 0; + *exists = git_path_isfile(ref_path.ptr) || + git_sortedcache_lookup(backend->refcache, ref_name); git_buf_free(&ref_path); return 0; @@ -456,69 +349,39 @@ static int loose_lookup( refdb_fs_backend *backend, const char *ref_name) { - const char *target; - git_oid oid; git_buf ref_file = GIT_BUF_INIT; int error = 0; if (out) *out = NULL; - error = reference_read(&ref_file, NULL, backend->path, ref_name, NULL); + if ((error = loose_readbuffer(&ref_file, backend->path, ref_name)) < 0) + /* cannot read loose ref file - gah */; + else if (git__prefixcmp(git_buf_cstr(&ref_file), GIT_SYMREF) == 0) { + const char *target; - if (error < 0) - goto done; - - if (git__prefixcmp(git_buf_cstr(&ref_file), GIT_SYMREF) == 0) { git_buf_rtrim(&ref_file); - if ((target = loose_parse_symbolic(&ref_file)) == NULL) { + if (!(target = loose_parse_symbolic(&ref_file))) error = -1; - goto done; - } - - if (out) + else if (out != NULL) *out = git_reference__alloc_symbolic(ref_name, target); } else { - if ((error = loose_parse_oid(&oid, ref_name, &ref_file)) < 0) - goto done; + git_oid oid; - if (out) + if (!(error = loose_parse_oid(&oid, ref_name, &ref_file)) && + out != NULL) *out = git_reference__alloc(ref_name, &oid, NULL); } - if (out && *out == NULL) - error = -1; - -done: git_buf_free(&ref_file); return error; } -static int packed_map_entry( - struct packref **entry, - khiter_t *pos, - refdb_fs_backend *backend, - const char *ref_name) +static int ref_error_notfound(const char *name) { - git_strmap *packfile_refs; - - if (packed_load(backend) < 0) - return -1; - - /* Look up on the packfile */ - packfile_refs = backend->refcache.packfile; - - *pos = git_strmap_lookup_index(packfile_refs, ref_name); - - if (!git_strmap_valid_index(packfile_refs, *pos)) { - giterr_set(GITERR_REFERENCE, "Reference '%s' not found", ref_name); - return GIT_ENOTFOUND; - } - - *entry = git_strmap_value_at(packfile_refs, *pos); - - return 0; + giterr_set(GITERR_REFERENCE, "Reference '%s' not found", name); + return GIT_ENOTFOUND; } static int packed_lookup( @@ -526,18 +389,25 @@ static int packed_lookup( refdb_fs_backend *backend, const char *ref_name) { - struct packref *entry; - khiter_t pos; int error = 0; + struct packref *entry; - if ((error = packed_map_entry(&entry, &pos, backend, ref_name)) < 0) - return error; - - if ((*out = git_reference__alloc(ref_name, - &entry->oid, &entry->peel)) == NULL) + if (packed_reload(backend) < 0) return -1; - return 0; + git_sortedcache_lock(backend->refcache); + + entry = git_sortedcache_lookup(backend->refcache, ref_name); + if (!entry) { + error = ref_error_notfound(ref_name); + } else { + *out = git_reference__alloc(ref_name, &entry->oid, &entry->peel); + if (!*out) + error = -1; + } + + git_sortedcache_unlock(backend->refcache); + return error; } static int refdb_fs_backend__lookup( @@ -545,24 +415,22 @@ static int refdb_fs_backend__lookup( git_refdb_backend *_backend, const char *ref_name) { - refdb_fs_backend *backend; - int result; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; + int error; - assert(_backend); + assert(backend); - backend = (refdb_fs_backend *)_backend; - - if ((result = loose_lookup(out, backend, ref_name)) == 0) + if (!(error = loose_lookup(out, backend, ref_name))) return 0; /* only try to lookup this reference on the packfile if it * wasn't found on the loose refs; not if there was a critical error */ - if (result == GIT_ENOTFOUND) { + if (error == GIT_ENOTFOUND) { giterr_clear(); - result = packed_lookup(out, backend, ref_name); + error = packed_lookup(out, backend, ref_name); } - return result; + return error; } typedef struct { @@ -573,8 +441,8 @@ typedef struct { git_pool pool; git_vector loose; - unsigned int loose_pos; - khiter_t packed_pos; + size_t loose_pos; + size_t packed_pos; } refdb_fs_iter; static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) @@ -589,7 +457,6 @@ static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) { int error = 0; - git_strmap *packfile = backend->refcache.packfile; git_buf path = GIT_BUF_INIT; git_iterator *fsit = NULL; const git_index_entry *entry = NULL; @@ -597,18 +464,18 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) if (!backend->path) /* do nothing if no path for loose refs */ return 0; - if (git_buf_printf(&path, "%s/refs", backend->path) < 0) - return -1; + if ((error = git_buf_printf(&path, "%s/refs", backend->path)) < 0 || + (error = git_iterator_for_filesystem( + &fsit, git_buf_cstr(&path), 0, NULL, NULL)) < 0) { + git_buf_free(&path); + return error; + } - if ((error = git_iterator_for_filesystem( - &fsit, git_buf_cstr(&path), 0, NULL, NULL)) < 0 || - (error = git_vector_init(&iter->loose, 8, NULL)) < 0 || - (error = git_buf_sets(&path, GIT_REFS_DIR)) < 0) - goto cleanup; + error = git_buf_sets(&path, GIT_REFS_DIR); - while (!git_iterator_advance(&entry, fsit)) { + while (!error && !git_iterator_advance(&entry, fsit)) { const char *ref_name; - khiter_t pos; + struct packref *ref; char *ref_dup; git_buf_truncate(&path, strlen(GIT_REFS_DIR)); @@ -619,26 +486,23 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) (iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0)) continue; - pos = git_strmap_lookup_index(packfile, ref_name); - if (git_strmap_valid_index(packfile, pos)) { - struct packref *ref = git_strmap_value_at(packfile, pos); + git_sortedcache_lock(backend->refcache); + ref = git_sortedcache_lookup(backend->refcache, ref_name); + if (ref) ref->flags |= PACKREF_SHADOWED; - } + git_sortedcache_unlock(backend->refcache); - if (!(ref_dup = git_pool_strdup(&iter->pool, ref_name))) { + ref_dup = git_pool_strdup(&iter->pool, ref_name); + if (!ref_dup) error = -1; - goto cleanup; - } - - if ((error = git_vector_insert(&iter->loose, ref_dup)) < 0) - goto cleanup; + else + error = git_vector_insert(&iter->loose, ref_dup); } -cleanup: git_iterator_free(fsit); git_buf_free(&path); - return 0; + return error; } static int refdb_fs_backend__iterator_next( @@ -646,7 +510,7 @@ static int refdb_fs_backend__iterator_next( { refdb_fs_iter *iter = (refdb_fs_iter *)_iter; refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.db->backend; - git_strmap *packfile = backend->refcache.packfile; + struct packref *ref; while (iter->loose_pos < iter->loose.length) { const char *path = git_vector_get(&iter->loose, iter->loose_pos++); @@ -657,17 +521,12 @@ static int refdb_fs_backend__iterator_next( giterr_clear(); } - while (iter->packed_pos < kh_end(packfile)) { - struct packref *ref = NULL; + git_sortedcache_lock(backend->refcache); - while (!kh_exist(packfile, iter->packed_pos)) { - iter->packed_pos++; - if (iter->packed_pos == kh_end(packfile)) - return GIT_ITEROVER; - } - - ref = kh_val(packfile, iter->packed_pos); - iter->packed_pos++; + while (iter->packed_pos < git_sortedcache_entrycount(backend->refcache)) { + ref = git_sortedcache_entry(backend->refcache, iter->packed_pos++); + if (!ref) /* stop now if another thread deleted refs and we past end */ + break; if (ref->flags & PACKREF_SHADOWED) continue; @@ -676,12 +535,11 @@ static int refdb_fs_backend__iterator_next( continue; *out = git_reference__alloc(ref->name, &ref->oid, &ref->peel); - if (*out == NULL) - return -1; - - return 0; + git_sortedcache_unlock(backend->refcache); + return (*out != NULL) ? 0 : -1; } + git_sortedcache_unlock(backend->refcache); return GIT_ITEROVER; } @@ -690,14 +548,10 @@ static int refdb_fs_backend__iterator_next_name( { refdb_fs_iter *iter = (refdb_fs_iter *)_iter; refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.db->backend; - git_strmap *packfile = backend->refcache.packfile; while (iter->loose_pos < iter->loose.length) { const char *path = git_vector_get(&iter->loose, iter->loose_pos++); - if (git_strmap_exists(packfile, path)) - continue; - if (loose_lookup(NULL, backend, path) != 0) { giterr_clear(); continue; @@ -707,22 +561,25 @@ static int refdb_fs_backend__iterator_next_name( return 0; } - while (iter->packed_pos < kh_end(packfile)) { - while (!kh_exist(packfile, iter->packed_pos)) { - iter->packed_pos++; - if (iter->packed_pos == kh_end(packfile)) - return GIT_ITEROVER; - } + git_sortedcache_lock(backend->refcache); - *out = kh_key(packfile, iter->packed_pos); - iter->packed_pos++; + while (iter->packed_pos < git_sortedcache_entrycount(backend->refcache)) { + struct packref *ref = + git_sortedcache_entry(backend->refcache, iter->packed_pos++); + + if (ref->flags & PACKREF_SHADOWED) + continue; + + *out = ref->name; if (iter->glob && p_fnmatch(iter->glob, *out, 0) != 0) continue; + git_sortedcache_unlock(backend->refcache); return 0; } + git_sortedcache_unlock(backend->refcache); return GIT_ITEROVER; } @@ -730,18 +587,18 @@ static int refdb_fs_backend__iterator( git_reference_iterator **out, git_refdb_backend *_backend, const char *glob) { refdb_fs_iter *iter; - refdb_fs_backend *backend; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; - assert(_backend); - backend = (refdb_fs_backend *)_backend; + assert(backend); - if (packed_load(backend) < 0) + if (packed_reload(backend) < 0) return -1; iter = git__calloc(1, sizeof(refdb_fs_iter)); GITERR_CHECK_ALLOC(iter); - if (git_pool_init(&iter->pool, 1, 0) < 0) + if (git_pool_init(&iter->pool, 1, 0) < 0 || + git_vector_init(&iter->loose, 8, NULL) < 0) goto fail; if (glob != NULL && @@ -786,15 +643,16 @@ static int reference_path_available( const char* old_ref, int force) { - struct packref *this_ref; + size_t i; - if (packed_load(backend) < 0) + if (packed_reload(backend) < 0) return -1; if (!force) { int exists; - if (refdb_fs_backend__exists(&exists, (git_refdb_backend *)backend, new_ref) < 0) + if (refdb_fs_backend__exists( + &exists, (git_refdb_backend *)backend, new_ref) < 0) return -1; if (exists) { @@ -805,14 +663,21 @@ static int reference_path_available( } } - git_strmap_foreach_value(backend->refcache.packfile, this_ref, { + git_sortedcache_lock(backend->refcache); + + for (i = 0; i < git_sortedcache_entrycount(backend->refcache); ++i) { + struct packref *this_ref = + git_sortedcache_entry(backend->refcache, i); + if (!ref_is_available(old_ref, new_ref, this_ref->name)) { + git_sortedcache_unlock(backend->refcache); giterr_set(GITERR_REFERENCE, "The path to reference '%s' collides with an existing one", new_ref); return -1; } - }); + } + git_sortedcache_unlock(backend->refcache); return 0; } @@ -839,12 +704,9 @@ static int loose_write(refdb_fs_backend *backend, const git_reference *ref) if (ref->type == GIT_REF_OID) { char oid[GIT_OID_HEXSZ + 1]; - - git_oid_fmt(oid, &ref->target.oid); - oid[GIT_OID_HEXSZ] = '\0'; + git_oid_nfmt(oid, sizeof(oid), &ref->target.oid); git_filebuf_printf(&file, "%s\n", oid); - } else if (ref->type == GIT_REF_SYMBOLIC) { git_filebuf_printf(&file, GIT_SYMREF "%s\n", ref->target.symbolic); } else { @@ -854,14 +716,6 @@ static int loose_write(refdb_fs_backend *backend, const git_reference *ref) return git_filebuf_commit(&file, GIT_REFS_FILE_MODE); } -static int packed_sort(const void *a, const void *b) -{ - const struct packref *ref_a = (const struct packref *)a; - const struct packref *ref_b = (const struct packref *)b; - - return strcmp(ref_a->name, ref_b->name); -} - /* * Find out what object this reference resolves to. * @@ -914,9 +768,7 @@ static int packed_find_peel(refdb_fs_backend *backend, struct packref *ref) static int packed_write_ref(struct packref *ref, git_filebuf *file) { char oid[GIT_OID_HEXSZ + 1]; - - git_oid_fmt(oid, &ref->oid); - oid[GIT_OID_HEXSZ] = 0; + git_oid_nfmt(oid, sizeof(oid), &ref->oid); /* * For references that peel to an object in the repo, we must @@ -930,8 +782,7 @@ static int packed_write_ref(struct packref *ref, git_filebuf *file) */ if (ref->flags & PACKREF_HAS_PEEL) { char peel[GIT_OID_HEXSZ + 1]; - git_oid_fmt(peel, &ref->peel); - peel[GIT_OID_HEXSZ] = 0; + git_oid_nfmt(peel, sizeof(peel), &ref->peel); if (git_filebuf_printf(file, "%s %s\n^%s\n", oid, ref->name, peel) < 0) return -1; @@ -954,16 +805,16 @@ static int packed_write_ref(struct packref *ref, git_filebuf *file) * is well-written, because we are destructing references * here otherwise. */ -static int packed_remove_loose( - refdb_fs_backend *backend, - git_vector *packing_list) +static int packed_remove_loose(refdb_fs_backend *backend) { size_t i; git_buf full_path = GIT_BUF_INIT; int failed = 0; - for (i = 0; i < packing_list->length; ++i) { - struct packref *ref = git_vector_get(packing_list, i); + /* backend->refcache is already locked when this is called */ + + for (i = 0; i < git_sortedcache_entrycount(backend->refcache); ++i) { + struct packref *ref = git_sortedcache_entry(backend->refcache, i); if ((ref->flags & PACKREF_WAS_LOOSE) == 0) continue; @@ -971,14 +822,13 @@ static int packed_remove_loose( if (git_buf_joinpath(&full_path, backend->path, ref->name) < 0) return -1; /* critical; do not try to recover on oom */ - if (git_path_exists(full_path.ptr) == true && p_unlink(full_path.ptr) < 0) { + if (git_path_exists(full_path.ptr) && p_unlink(full_path.ptr) < 0) { if (failed) continue; giterr_set(GITERR_REFERENCE, "Failed to remove loose reference '%s' after packing: %s", full_path.ptr, strerror(errno)); - failed = 1; } @@ -1002,33 +852,14 @@ static int packed_write(refdb_fs_backend *backend) git_filebuf pack_file = GIT_FILEBUF_INIT; size_t i; git_buf pack_file_path = GIT_BUF_INIT; - git_vector packing_list; - unsigned int total_refs; - assert(backend && backend->refcache.packfile); - - total_refs = - (unsigned int)git_strmap_num_entries(backend->refcache.packfile); - - if (git_vector_init(&packing_list, total_refs, packed_sort) < 0) + /* lock the cache to updates while we do this */ + if (git_sortedcache_lock(backend->refcache) < 0) return -1; - /* Load all the packfile into a vector */ - { - struct packref *reference; - - /* cannot fail: vector already has the right size */ - git_strmap_foreach_value(backend->refcache.packfile, reference, { - git_vector_insert(&packing_list, reference); - }); - } - - /* sort the vector so the entries appear sorted on the packfile */ - git_vector_sort(&packing_list); - - /* Now we can open the file! */ - if (git_buf_joinpath(&pack_file_path, - backend->path, GIT_PACKEDREFS_FILE) < 0) + /* Open the file! */ + if (git_buf_joinpath( + &pack_file_path, backend->path, GIT_PACKEDREFS_FILE) < 0) goto cleanup_memory; if (git_filebuf_open(&pack_file, pack_file_path.ptr, 0) < 0) @@ -1040,8 +871,9 @@ static int packed_write(refdb_fs_backend *backend) if (git_filebuf_printf(&pack_file, "%s\n", GIT_PACKEDREFS_HEADER) < 0) goto cleanup_packfile; - for (i = 0; i < packing_list.length; ++i) { - struct packref *ref = (struct packref *)git_vector_get(&packing_list, i); + for (i = 0; i < git_sortedcache_entrycount(backend->refcache); ++i) { + struct packref *ref = + git_sortedcache_entry(backend->refcache, i); if (packed_find_peel(backend, ref) < 0) goto cleanup_packfile; @@ -1057,16 +889,16 @@ static int packed_write(refdb_fs_backend *backend) /* when and only when the packfile has been properly written, * we can go ahead and remove the loose refs */ - if (packed_remove_loose(backend, &packing_list) < 0) - goto cleanup_memory; + if (packed_remove_loose(backend) < 0) + goto cleanup_memory; - { - struct stat st; - if (p_stat(pack_file_path.ptr, &st) == 0) - backend->refcache.packfile_time = st.st_mtime; - } + git_sortedcache_unlock(backend->refcache); + + /* update filestamp to latest value */ + if (git_futils_filestamp_check( + &backend->refcache->stamp, pack_file_path.ptr) < 0) + giterr_clear(); - git_vector_free(&packing_list); git_buf_free(&pack_file_path); /* we're good now */ @@ -1076,7 +908,7 @@ cleanup_packfile: git_filebuf_cleanup(&pack_file); cleanup_memory: - git_vector_free(&packing_list); + git_sortedcache_unlock(backend->refcache); git_buf_free(&pack_file_path); return -1; @@ -1087,11 +919,10 @@ static int refdb_fs_backend__write( const git_reference *ref, int force) { - refdb_fs_backend *backend; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; int error; - assert(_backend); - backend = (refdb_fs_backend *)_backend; + assert(backend); error = reference_path_available(backend, ref->name, NULL, force); if (error < 0) @@ -1104,17 +935,13 @@ static int refdb_fs_backend__delete( git_refdb_backend *_backend, const char *ref_name) { - refdb_fs_backend *backend; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; git_buf loose_path = GIT_BUF_INIT; - struct packref *pack_ref; - khiter_t pack_ref_pos; + size_t pack_pos; int error = 0; bool loose_deleted = 0; - assert(_backend); - assert(ref_name); - - backend = (refdb_fs_backend *)_backend; + assert(backend && ref_name); /* If a loose reference exists, remove it from the filesystem */ if (git_buf_joinpath(&loose_path, backend->path, ref_name) < 0) @@ -1130,19 +957,23 @@ static int refdb_fs_backend__delete( if (error != 0) return error; + if (packed_reload(backend) < 0) + return -1; + /* If a packed reference exists, remove it from the packfile and repack */ - error = packed_map_entry(&pack_ref, &pack_ref_pos, backend, ref_name); + if (git_sortedcache_lock(backend->refcache) < 0) + return -1; + + if (!(error = git_sortedcache_lookup_index( + &pack_pos, backend->refcache, ref_name))) + error = git_sortedcache_remove(backend->refcache, pack_pos, false); + + git_sortedcache_unlock(backend->refcache); if (error == GIT_ENOTFOUND) - return loose_deleted ? 0 : GIT_ENOTFOUND; + return loose_deleted ? 0 : ref_error_notfound(ref_name); - if (error == 0) { - git_strmap_delete_at(backend->refcache.packfile, pack_ref_pos); - git__free(pack_ref); - error = packed_write(backend); - } - - return error; + return packed_write(backend); } static int refdb_fs_backend__rename( @@ -1152,53 +983,44 @@ static int refdb_fs_backend__rename( const char *new_name, int force) { - refdb_fs_backend *backend; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; git_reference *old, *new; int error; - assert(_backend); - backend = (refdb_fs_backend *)_backend; + assert(backend); - error = reference_path_available(backend, new_name, old_name, force); - if (error < 0) + if ((error = reference_path_available( + backend, new_name, old_name, force)) < 0 || + (error = refdb_fs_backend__lookup(&old, _backend, old_name)) < 0) return error; - error = refdb_fs_backend__lookup(&old, _backend, old_name); - if (error < 0) - return error; - - error = refdb_fs_backend__delete(_backend, old_name); - if (error < 0) { + if ((error = refdb_fs_backend__delete(_backend, old_name)) < 0) { git_reference_free(old); return error; } - new = realloc(old, sizeof(git_reference) + strlen(new_name) + 1); - memcpy(new->name, new_name, strlen(new_name) + 1); + new = git_reference__set_name(old, new_name); + if (!new) { + git_reference_free(old); + return -1; + } - error = loose_write(backend, new); - if (error < 0) { + if ((error = loose_write(backend, new)) < 0 || out == NULL) { git_reference_free(new); return error; } - if (out) { - *out = new; - } else { - git_reference_free(new); - } - + *out = new; return 0; } static int refdb_fs_backend__compress(git_refdb_backend *_backend) { - refdb_fs_backend *backend; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; - assert(_backend); - backend = (refdb_fs_backend *)_backend; + assert(backend); - if (packed_load(backend) < 0 || /* load the existing packfile */ + if (packed_reload(backend) < 0 || /* load the existing packfile */ packed_loadloose(backend) < 0 || /* add all the loose refs */ packed_write(backend) < 0) /* write back to disk */ return -1; @@ -1206,29 +1028,13 @@ static int refdb_fs_backend__compress(git_refdb_backend *_backend) return 0; } -static void refcache_free(git_refcache *refs) -{ - assert(refs); - - if (refs->packfile) { - struct packref *reference; - - git_strmap_foreach_value(refs->packfile, reference, { - git__free(reference); - }); - - git_strmap_free(refs->packfile); - } -} - static void refdb_fs_backend__free(git_refdb_backend *_backend) { - refdb_fs_backend *backend; + refdb_fs_backend *backend = (refdb_fs_backend *)_backend; - assert(_backend); - backend = (refdb_fs_backend *)_backend; + assert(backend); - refcache_free(&backend->refcache); + git_sortedcache_free(backend->refcache); git__free(backend->path); git__free(backend); } @@ -1252,7 +1058,7 @@ static int setup_namespace(git_buf *path, git_repository *repo) if (parts == NULL) return -1; - /** + /* * From `man gitnamespaces`: * namespaces which include a / will expand to a hierarchy * of namespaces; for example, GIT_NAMESPACE=foo/bar will store @@ -1269,7 +1075,7 @@ static int setup_namespace(git_buf *path, git_repository *repo) if (git_futils_mkdir_r(git_buf_cstr(path), repo->path_repository, 0777) < 0) return -1; - /* Return the root of the namespaced path, i.e. without the trailing '/refs' */ + /* Return root of the namespaced path, i.e. without the trailing '/refs' */ git_buf_rtruncate_at_char(path, '/'); return 0; } @@ -1286,13 +1092,19 @@ int git_refdb_backend_fs( backend->repo = repository; - if (setup_namespace(&path, repository) < 0) { - git__free(backend); - return -1; - } + if (setup_namespace(&path, repository) < 0) + goto fail; backend->path = git_buf_detach(&path); + if (git_buf_joinpath(&path, backend->path, GIT_PACKEDREFS_FILE) < 0 || + git_sortedcache_new( + &backend->refcache, offsetof(struct packref, name), + NULL, NULL, packref_cmp, git_buf_cstr(&path)) < 0) + goto fail; + + git_buf_free(&path); + backend->parent.exists = &refdb_fs_backend__exists; backend->parent.lookup = &refdb_fs_backend__lookup; backend->parent.iterator = &refdb_fs_backend__iterator; @@ -1304,4 +1116,10 @@ int git_refdb_backend_fs( *backend_out = (git_refdb_backend *)backend; return 0; + +fail: + git_buf_free(&path); + git__free(backend->path); + git__free(backend); + return -1; } diff --git a/tests-clar/threads/refdb.c b/tests-clar/threads/refdb.c new file mode 100644 index 000000000..11e90a458 --- /dev/null +++ b/tests-clar/threads/refdb.c @@ -0,0 +1,200 @@ +#include "clar_libgit2.h" +#include "git2/refdb.h" +#include "refdb.h" + +static git_repository *g_repo; +static int g_expected = 0; + +void test_threads_refdb__initialize(void) +{ + g_repo = NULL; +} + +void test_threads_refdb__cleanup(void) +{ + cl_git_sandbox_cleanup(); + g_repo = NULL; +} + +#define REPEAT 20 +#define THREADS 20 + +static void *iterate_refs(void *arg) +{ + git_reference_iterator *i; + git_reference *ref; + int count = 0, *id = arg; + + usleep(THREADS - *id); + + cl_git_pass(git_reference_iterator_new(&i, g_repo)); + + for (count = 0; !git_reference_next(&ref, i); ++count) { + cl_assert(ref != NULL); + git_reference_free(ref); + } + + if (g_expected > 0) + cl_assert_equal_i(g_expected, count); + + git_reference_iterator_free(i); + + return arg; +} + +void test_threads_refdb__iterator(void) +{ + int r, t; + git_thread th[THREADS]; + int id[THREADS]; + git_oid head; + git_reference *ref; + char name[128]; + git_refdb *refdb; + + g_repo = cl_git_sandbox_init("testrepo2"); + + cl_git_pass(git_reference_name_to_id(&head, g_repo, "HEAD")); + + /* make a bunch of references */ + + for (r = 0; r < 200; ++r) { + snprintf(name, sizeof(name), "refs/heads/direct-%03d", r); + cl_git_pass(git_reference_create(&ref, g_repo, name, &head, 0)); + git_reference_free(ref); + } + + cl_git_pass(git_repository_refdb(&refdb, g_repo)); + cl_git_pass(git_refdb_compress(refdb)); + git_refdb_free(refdb); + + g_expected = 206; + + for (r = 0; r < REPEAT; ++r) { + g_repo = cl_git_sandbox_reopen(); /* reopen to flush caches */ + + for (t = 0; t < THREADS; ++t) { + id[t] = t; + cl_git_pass(git_thread_create(&th[t], NULL, iterate_refs, &id[t])); + } + + for (t = 0; t < THREADS; ++t) { + cl_git_pass(git_thread_join(th[t], NULL)); + } + + memset(th, 0, sizeof(th)); + } +} + +static void *create_refs(void *arg) +{ + int *id = arg, i; + git_oid head; + char name[128]; + git_reference *ref[10]; + + cl_git_pass(git_reference_name_to_id(&head, g_repo, "HEAD")); + + for (i = 0; i < 10; ++i) { + snprintf(name, sizeof(name), "refs/heads/thread-%03d-%02d", *id, i); + cl_git_pass(git_reference_create(&ref[i], g_repo, name, &head, 0)); + + if (i == 5) { + git_refdb *refdb; + cl_git_pass(git_repository_refdb(&refdb, g_repo)); + cl_git_pass(git_refdb_compress(refdb)); + git_refdb_free(refdb); + } + } + + for (i = 0; i < 10; ++i) + git_reference_free(ref[i]); + + return arg; +} + +static void *delete_refs(void *arg) +{ + int *id = arg, i; + git_reference *ref; + char name[128]; + + for (i = 0; i < 10; ++i) { + snprintf( + name, sizeof(name), "refs/heads/thread-%03d-%02d", (*id) & ~0x3, i); + + if (!git_reference_lookup(&ref, g_repo, name)) { + fprintf(stderr, "deleting %s\n", name); + cl_git_pass(git_reference_delete(ref)); + git_reference_delete(ref); + } + + if (i == 5) { + git_refdb *refdb; + cl_git_pass(git_repository_refdb(&refdb, g_repo)); + cl_git_pass(git_refdb_compress(refdb)); + git_refdb_free(refdb); + } + } + + return arg; +} + +void test_threads_refdb__edit_while_iterate(void) +{ + int r, t; + git_thread th[THREADS]; + int id[THREADS]; + git_oid head; + git_reference *ref; + char name[128]; + git_refdb *refdb; + + g_repo = cl_git_sandbox_init("testrepo2"); + + cl_git_pass(git_reference_name_to_id(&head, g_repo, "HEAD")); + + /* make a bunch of references */ + + for (r = 0; r < 50; ++r) { + snprintf(name, sizeof(name), "refs/heads/starter-%03d", r); + cl_git_pass(git_reference_create(&ref, g_repo, name, &head, 0)); + git_reference_free(ref); + } + + cl_git_pass(git_repository_refdb(&refdb, g_repo)); + cl_git_pass(git_refdb_compress(refdb)); + git_refdb_free(refdb); + + g_expected = -1; + + g_repo = cl_git_sandbox_reopen(); /* reopen to flush caches */ + + for (t = 0; t < THREADS; ++t) { + void *(*fn)(void *arg); + + switch (t & 0x3) { + case 0: fn = create_refs; break; + case 1: fn = delete_refs; break; + default: fn = iterate_refs; break; + } + + id[t] = t; + cl_git_pass(git_thread_create(&th[t], NULL, fn, &id[t])); + } + + for (t = 0; t < THREADS; ++t) { + cl_git_pass(git_thread_join(th[t], NULL)); + } + + memset(th, 0, sizeof(th)); + + for (t = 0; t < THREADS; ++t) { + id[t] = t; + cl_git_pass(git_thread_create(&th[t], NULL, iterate_refs, &id[t])); + } + + for (t = 0; t < THREADS; ++t) { + cl_git_pass(git_thread_join(th[t], NULL)); + } +} From b37359aac5b02f59c07b453b53191432abc0f942 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 21 Aug 2013 16:50:03 -0700 Subject: [PATCH 06/19] Fix warnings when compiling without threads --- src/sortedcache.c | 2 ++ tests-clar/threads/refdb.c | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/sortedcache.c b/src/sortedcache.c index 2efa3c4e9..d7e74d3e5 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -176,6 +176,8 @@ bool git_sortedcache_out_of_date(git_sortedcache *sc) /* lock sortedcache while making modifications */ int git_sortedcache_lock(git_sortedcache *sc) { + GIT_UNUSED(sc); /* to prevent warning when compiled w/o threads */ + if (git_mutex_lock(&sc->lock) < 0) { giterr_set(GITERR_OS, "Unable to acquire mutex lock"); return -1; diff --git a/tests-clar/threads/refdb.c b/tests-clar/threads/refdb.c index 11e90a458..ffe437575 100644 --- a/tests-clar/threads/refdb.c +++ b/tests-clar/threads/refdb.c @@ -23,9 +23,7 @@ static void *iterate_refs(void *arg) { git_reference_iterator *i; git_reference *ref; - int count = 0, *id = arg; - - usleep(THREADS - *id); + int count = 0; cl_git_pass(git_reference_iterator_new(&i, g_repo)); @@ -75,12 +73,19 @@ void test_threads_refdb__iterator(void) for (t = 0; t < THREADS; ++t) { id[t] = t; +#ifdef GIT_THREADS cl_git_pass(git_thread_create(&th[t], NULL, iterate_refs, &id[t])); +#else + th[t] = t; + iterate_refs(&id[t]); +#endif } +#ifdef GIT_THREADS for (t = 0; t < THREADS; ++t) { cl_git_pass(git_thread_join(th[t], NULL)); } +#endif memset(th, 0, sizeof(th)); } @@ -124,7 +129,6 @@ static void *delete_refs(void *arg) name, sizeof(name), "refs/heads/thread-%03d-%02d", (*id) & ~0x3, i); if (!git_reference_lookup(&ref, g_repo, name)) { - fprintf(stderr, "deleting %s\n", name); cl_git_pass(git_reference_delete(ref)); git_reference_delete(ref); } @@ -180,9 +184,15 @@ void test_threads_refdb__edit_while_iterate(void) } id[t] = t; +#ifdef GIT_THREADS cl_git_pass(git_thread_create(&th[t], NULL, fn, &id[t])); +#else + th[t] = t; + fn(&id[t]); +#endif } +#ifdef GIT_THREADS for (t = 0; t < THREADS; ++t) { cl_git_pass(git_thread_join(th[t], NULL)); } @@ -197,4 +207,5 @@ void test_threads_refdb__edit_while_iterate(void) for (t = 0; t < THREADS; ++t) { cl_git_pass(git_thread_join(th[t], NULL)); } +#endif } From e8c5eb5537cc58fa29d4ed48ab9114238ba4cab0 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 21 Aug 2013 22:44:56 -0700 Subject: [PATCH 07/19] No need to lock newly created tgt in copy --- src/sortedcache.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/sortedcache.c b/src/sortedcache.c index d7e74d3e5..d0a8031c3 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -130,9 +130,6 @@ int git_sortedcache_copy( return -1; } - if (git_sortedcache_lock(tgt) < 0) - goto fail; - git_vector_foreach(&src->items, i, src_item) { if (git_sortedcache_upsert( &tgt_item, tgt, ((char *)src_item) + src->item_path_offset) < 0) @@ -141,7 +138,6 @@ int git_sortedcache_copy( goto fail; } - git_sortedcache_unlock(tgt); git_sortedcache_unlock(src); *out = tgt; From 3eecadcce583fd7e825e35e2b6f101071c2be613 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 21 Aug 2013 22:50:37 -0700 Subject: [PATCH 08/19] Improve comments on locking for sortedcache APIs --- src/sortedcache.h | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/sortedcache.h b/src/sortedcache.h index f63ad645b..c1c9d1341 100644 --- a/src/sortedcache.h +++ b/src/sortedcache.h @@ -62,22 +62,24 @@ int git_sortedcache_copy( int (*copy_item)(void *payload, void *tgt_item, void *src_item), void *payload); -/* free sorted cache (first calling free_item callbacks) */ +/* free sorted cache (first calling free_item callbacks) + * don't call on a locked collection - it may acquire a lock + */ void git_sortedcache_free(git_sortedcache *sc); -/* increment reference count */ +/* increment reference count - balance with call to free */ void git_sortedcache_incref(git_sortedcache *sc); -/* release all items in sorted cache - lock during clear if lock is true */ +/* release all items in sorted cache - lock during clear if `lock` is true */ void git_sortedcache_clear(git_sortedcache *sc, bool lock); /* check file stamp to see if reload is required */ bool git_sortedcache_out_of_date(git_sortedcache *sc); -/* lock sortedcache while making modifications */ +/* lock sortedcache during access or modification */ int git_sortedcache_lock(git_sortedcache *sc); -/* unlock sorted cache when done with modifications */ +/* unlock sorted cache when done */ int git_sortedcache_unlock(git_sortedcache *sc); /* if the file has changed, lock cache and load file contents into buf; @@ -85,24 +87,33 @@ int git_sortedcache_unlock(git_sortedcache *sc); */ int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf); -/* find and/or insert item, returning pointer to item data - lock first */ +/* find and/or insert item, returning pointer to item data + * should only call on locked collection + */ int git_sortedcache_upsert( void **out, git_sortedcache *sc, const char *key); -/* lookup item by key */ +/* lookup item by key + * should only call on locked collection if return value will be used + */ void *git_sortedcache_lookup(const git_sortedcache *sc, const char *key); /* find out how many items are in the cache */ size_t git_sortedcache_entrycount(const git_sortedcache *sc); -/* lookup item by index */ +/* lookup item by index + * should only call on locked collection if return value will be used + */ void *git_sortedcache_entry(const git_sortedcache *sc, size_t pos); -/* lookup index of item by key */ +/* lookup index of item by key + * if collection is not locked, there is no guarantee the returned index + * will be correct if it used to look up the item + */ int git_sortedcache_lookup_index( size_t *out, git_sortedcache *sc, const char *key); -/* remove entry from cache */ +/* remove entry from cache - lock during delete if `lock` is true */ int git_sortedcache_remove(git_sortedcache *sc, size_t pos, bool lock); #endif From 8d9a85d43aa6ed7a9fb15a2ac9e0f9ba1c33461e Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 22 Aug 2013 11:40:53 -0700 Subject: [PATCH 09/19] Convert sortedcache to use rwlock This is the first use we have of pthread_rwlock_t in libgit2. Hopefully it won't cause any serious portability problems. --- src/refdb_fs.c | 131 +++++++++++++-------------- src/sortedcache.c | 161 +++++++++++++++++++--------------- src/sortedcache.h | 106 ++++++++++++++-------- src/thread-utils.h | 27 +++++- tests-clar/core/sortedcache.c | 31 ++++--- 5 files changed, 268 insertions(+), 188 deletions(-) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 876e84588..04516a5b0 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -172,7 +172,7 @@ static int packed_reload(refdb_fs_backend *backend) ref->flags |= PACKREF_CANNOT_PEEL; } - git_sortedcache_unlock(backend->refcache); + git_sortedcache_wunlock(backend->refcache); git_buf_free(&packedrefs); return 0; @@ -181,7 +181,7 @@ parse_failed: giterr_set(GITERR_REFERENCE, "Corrupted packed references file"); git_sortedcache_clear(backend->refcache, false); - git_sortedcache_unlock(backend->refcache); + git_sortedcache_wunlock(backend->refcache); git_buf_free(&packedrefs); return -1; @@ -244,7 +244,7 @@ static int loose_lookup_to_packfile(refdb_fs_backend *backend, const char *name) if ((error = loose_parse_oid(&oid, name, &ref_file)) < 0) goto done; - git_sortedcache_lock(backend->refcache); + git_sortedcache_wlock(backend->refcache); if (!(error = git_sortedcache_upsert( (void **)&ref, backend->refcache, name))) { @@ -253,7 +253,7 @@ static int loose_lookup_to_packfile(refdb_fs_backend *backend, const char *name) ref->flags = PACKREF_WAS_LOOSE; } - git_sortedcache_unlock(backend->refcache); + git_sortedcache_wunlock(backend->refcache); done: git_buf_free(&ref_file); @@ -317,7 +317,7 @@ static int refdb_fs_backend__exists( return -1; *exists = git_path_isfile(ref_path.ptr) || - git_sortedcache_lookup(backend->refcache, ref_name); + (git_sortedcache_lookup(backend->refcache, ref_name) != NULL); git_buf_free(&ref_path); return 0; @@ -395,7 +395,8 @@ static int packed_lookup( if (packed_reload(backend) < 0) return -1; - git_sortedcache_lock(backend->refcache); + if (git_sortedcache_rlock(backend->refcache) < 0) + return -1; entry = git_sortedcache_lookup(backend->refcache, ref_name); if (!entry) { @@ -406,7 +407,8 @@ static int packed_lookup( error = -1; } - git_sortedcache_unlock(backend->refcache); + git_sortedcache_runlock(backend->refcache); + return error; } @@ -486,11 +488,11 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) (iter->glob && p_fnmatch(iter->glob, ref_name, 0) != 0)) continue; - git_sortedcache_lock(backend->refcache); + git_sortedcache_rlock(backend->refcache); ref = git_sortedcache_lookup(backend->refcache, ref_name); if (ref) ref->flags |= PACKREF_SHADOWED; - git_sortedcache_unlock(backend->refcache); + git_sortedcache_runlock(backend->refcache); ref_dup = git_pool_strdup(&iter->pool, ref_name); if (!ref_dup) @@ -508,6 +510,7 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) static int refdb_fs_backend__iterator_next( git_reference **out, git_reference_iterator *_iter) { + int error = GIT_ITEROVER; refdb_fs_iter *iter = (refdb_fs_iter *)_iter; refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.db->backend; struct packref *ref; @@ -521,7 +524,7 @@ static int refdb_fs_backend__iterator_next( giterr_clear(); } - git_sortedcache_lock(backend->refcache); + git_sortedcache_rlock(backend->refcache); while (iter->packed_pos < git_sortedcache_entrycount(backend->refcache)) { ref = git_sortedcache_entry(backend->refcache, iter->packed_pos++); @@ -530,57 +533,56 @@ static int refdb_fs_backend__iterator_next( if (ref->flags & PACKREF_SHADOWED) continue; - if (iter->glob && p_fnmatch(iter->glob, ref->name, 0) != 0) continue; *out = git_reference__alloc(ref->name, &ref->oid, &ref->peel); - git_sortedcache_unlock(backend->refcache); - return (*out != NULL) ? 0 : -1; + error = (*out != NULL) ? 0 : -1; + break; } - git_sortedcache_unlock(backend->refcache); - return GIT_ITEROVER; + git_sortedcache_runlock(backend->refcache); + return error; } static int refdb_fs_backend__iterator_next_name( const char **out, git_reference_iterator *_iter) { + int error = GIT_ITEROVER; refdb_fs_iter *iter = (refdb_fs_iter *)_iter; refdb_fs_backend *backend = (refdb_fs_backend *)iter->parent.db->backend; + struct packref *ref; while (iter->loose_pos < iter->loose.length) { const char *path = git_vector_get(&iter->loose, iter->loose_pos++); - if (loose_lookup(NULL, backend, path) != 0) { - giterr_clear(); - continue; + if (loose_lookup(NULL, backend, path) == 0) { + *out = path; + return 0; } - *out = path; - return 0; + giterr_clear(); } - git_sortedcache_lock(backend->refcache); + git_sortedcache_rlock(backend->refcache); while (iter->packed_pos < git_sortedcache_entrycount(backend->refcache)) { - struct packref *ref = - git_sortedcache_entry(backend->refcache, iter->packed_pos++); + ref = git_sortedcache_entry(backend->refcache, iter->packed_pos++); + if (!ref) /* stop now if another thread deleted refs and we past end */ + break; if (ref->flags & PACKREF_SHADOWED) continue; - - *out = ref->name; - - if (iter->glob && p_fnmatch(iter->glob, *out, 0) != 0) + if (iter->glob && p_fnmatch(iter->glob, ref->name, 0) != 0) continue; - git_sortedcache_unlock(backend->refcache); - return 0; + *out = ref->name; + error = 0; + break; } - git_sortedcache_unlock(backend->refcache); - return GIT_ITEROVER; + git_sortedcache_runlock(backend->refcache); + return error; } static int refdb_fs_backend__iterator( @@ -658,26 +660,25 @@ static int reference_path_available( if (exists) { giterr_set(GITERR_REFERENCE, "Failed to write reference '%s': a reference with " - " that name already exists.", new_ref); + "that name already exists.", new_ref); return GIT_EEXISTS; } } - git_sortedcache_lock(backend->refcache); + git_sortedcache_rlock(backend->refcache); for (i = 0; i < git_sortedcache_entrycount(backend->refcache); ++i) { - struct packref *this_ref = - git_sortedcache_entry(backend->refcache, i); + struct packref *ref = git_sortedcache_entry(backend->refcache, i); - if (!ref_is_available(old_ref, new_ref, this_ref->name)) { - git_sortedcache_unlock(backend->refcache); + if (ref && !ref_is_available(old_ref, new_ref, ref->name)) { + git_sortedcache_runlock(backend->refcache); giterr_set(GITERR_REFERENCE, - "The path to reference '%s' collides with an existing one", new_ref); + "Path to reference '%s' collides with existing one", new_ref); return -1; } } - git_sortedcache_unlock(backend->refcache); + git_sortedcache_runlock(backend->refcache); return 0; } @@ -816,7 +817,7 @@ static int packed_remove_loose(refdb_fs_backend *backend) for (i = 0; i < git_sortedcache_entrycount(backend->refcache); ++i) { struct packref *ref = git_sortedcache_entry(backend->refcache, i); - if ((ref->flags & PACKREF_WAS_LOOSE) == 0) + if (!ref || !(ref->flags & PACKREF_WAS_LOOSE)) continue; if (git_buf_joinpath(&full_path, backend->path, ref->name) < 0) @@ -849,67 +850,53 @@ static int packed_remove_loose(refdb_fs_backend *backend) */ static int packed_write(refdb_fs_backend *backend) { + git_sortedcache *refcache = backend->refcache; git_filebuf pack_file = GIT_FILEBUF_INIT; size_t i; - git_buf pack_file_path = GIT_BUF_INIT; /* lock the cache to updates while we do this */ - if (git_sortedcache_lock(backend->refcache) < 0) + if (git_sortedcache_wlock(refcache) < 0) return -1; /* Open the file! */ - if (git_buf_joinpath( - &pack_file_path, backend->path, GIT_PACKEDREFS_FILE) < 0) - goto cleanup_memory; - - if (git_filebuf_open(&pack_file, pack_file_path.ptr, 0) < 0) - goto cleanup_packfile; + if (git_filebuf_open(&pack_file, git_sortedcache_path(refcache), 0) < 0) + goto fail; /* Packfiles have a header... apparently * This is in fact not required, but we might as well print it * just for kicks */ if (git_filebuf_printf(&pack_file, "%s\n", GIT_PACKEDREFS_HEADER) < 0) - goto cleanup_packfile; + goto fail; - for (i = 0; i < git_sortedcache_entrycount(backend->refcache); ++i) { - struct packref *ref = - git_sortedcache_entry(backend->refcache, i); + for (i = 0; i < git_sortedcache_entrycount(refcache); ++i) { + struct packref *ref = git_sortedcache_entry(refcache, i); if (packed_find_peel(backend, ref) < 0) - goto cleanup_packfile; + goto fail; if (packed_write_ref(ref, &pack_file) < 0) - goto cleanup_packfile; + goto fail; } /* if we've written all the references properly, we can commit * the packfile to make the changes effective */ if (git_filebuf_commit(&pack_file, GIT_PACKEDREFS_FILE_MODE) < 0) - goto cleanup_memory; + goto fail; /* when and only when the packfile has been properly written, * we can go ahead and remove the loose refs */ if (packed_remove_loose(backend) < 0) - goto cleanup_memory; + goto fail; - git_sortedcache_unlock(backend->refcache); - - /* update filestamp to latest value */ - if (git_futils_filestamp_check( - &backend->refcache->stamp, pack_file_path.ptr) < 0) - giterr_clear(); - - git_buf_free(&pack_file_path); + git_sortedcache_updated(refcache); + git_sortedcache_wunlock(refcache); /* we're good now */ return 0; -cleanup_packfile: +fail: git_filebuf_cleanup(&pack_file); - -cleanup_memory: - git_sortedcache_unlock(backend->refcache); - git_buf_free(&pack_file_path); + git_sortedcache_wunlock(refcache); return -1; } @@ -961,14 +948,14 @@ static int refdb_fs_backend__delete( return -1; /* If a packed reference exists, remove it from the packfile and repack */ - if (git_sortedcache_lock(backend->refcache) < 0) + if (git_sortedcache_wlock(backend->refcache) < 0) return -1; if (!(error = git_sortedcache_lookup_index( &pack_pos, backend->refcache, ref_name))) - error = git_sortedcache_remove(backend->refcache, pack_pos, false); + error = git_sortedcache_remove(backend->refcache, pack_pos); - git_sortedcache_unlock(backend->refcache); + git_sortedcache_wunlock(backend->refcache); if (error == GIT_ENOTFOUND) return loose_deleted ? 0 : ref_error_notfound(ref_name); diff --git a/src/sortedcache.c b/src/sortedcache.c index d0a8031c3..c087dbbe9 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -23,13 +23,13 @@ int git_sortedcache_new( (sc->map = git_strmap_alloc()) == NULL) goto fail; - if (git_mutex_init(&sc->lock)) { - giterr_set(GITERR_OS, "Failed to initialize mutex"); + if (git_rwlock_init(&sc->lock)) { + giterr_set(GITERR_OS, "Failed to initialize lock"); goto fail; } - sc->item_path_offset = item_path_offset; - sc->free_item = free_item; + sc->item_path_offset = item_path_offset; + sc->free_item = free_item; sc->free_item_payload = free_item_payload; GIT_REFCOUNT_INC(sc); if (pathlen) @@ -52,6 +52,11 @@ void git_sortedcache_incref(git_sortedcache *sc) GIT_REFCOUNT_INC(sc); } +const char *git_sortedcache_path(git_sortedcache *sc) +{ + return sc->path; +} + static void sortedcache_clear(git_sortedcache *sc) { git_strmap_clear(sc->map); @@ -72,19 +77,17 @@ static void sortedcache_clear(git_sortedcache *sc) static void sortedcache_free(git_sortedcache *sc) { - if (git_mutex_lock(&sc->lock) < 0) { - giterr_set(GITERR_OS, "Unable to acquire mutex lock for free"); + /* acquire write lock to make sure everyone else is done */ + if (git_sortedcache_wlock(sc) < 0) return; - } sortedcache_clear(sc); - git_vector_free(&sc->items); git_strmap_free(sc->map); - git_mutex_unlock(&sc->lock); - git_mutex_free(&sc->lock); + git_sortedcache_wunlock(sc); + git_rwlock_free(&sc->lock); git__free(sc); } @@ -107,88 +110,88 @@ static int sortedcache_copy_item(void *payload, void *tgt_item, void *src_item) int git_sortedcache_copy( git_sortedcache **out, git_sortedcache *src, + bool wlock, int (*copy_item)(void *payload, void *tgt_item, void *src_item), void *payload) { + int error = 0; git_sortedcache *tgt; size_t i; void *src_item, *tgt_item; + /* just use memcpy if no special copy fn is passed in */ if (!copy_item) { copy_item = sortedcache_copy_item; payload = src; } - if (git_sortedcache_new( + if ((error = git_sortedcache_new( &tgt, src->item_path_offset, src->free_item, src->free_item_payload, - src->items._cmp, src->path) < 0) - return -1; + src->items._cmp, src->path)) < 0) + return error; - if (git_sortedcache_lock(src) < 0) { + if (wlock && git_sortedcache_wlock(src) < 0) { git_sortedcache_free(tgt); return -1; } git_vector_foreach(&src->items, i, src_item) { - if (git_sortedcache_upsert( - &tgt_item, tgt, ((char *)src_item) + src->item_path_offset) < 0) - goto fail; - if (copy_item(payload, tgt_item, src_item) < 0) - goto fail; + char *path = ((char *)src_item) + src->item_path_offset; + + if ((error = git_sortedcache_upsert(&tgt_item, tgt, path)) < 0 || + (error = copy_item(payload, tgt_item, src_item)) < 0) + break; } - git_sortedcache_unlock(src); + if (wlock) + git_sortedcache_wunlock(src); + if (error) + git_sortedcache_free(tgt); - *out = tgt; - return 0; + *out = !error ? tgt : NULL; -fail: - git_sortedcache_unlock(src); - git_sortedcache_free(tgt); - return -1; -} - -/* release all items in sorted cache */ -void git_sortedcache_clear(git_sortedcache *sc, bool lock) -{ - if (lock && git_mutex_lock(&sc->lock) < 0) { - giterr_set(GITERR_OS, "Unable to acquire mutex lock for clear"); - return; - } - - sortedcache_clear(sc); - - if (lock) - git_mutex_unlock(&sc->lock); -} - -/* check file stamp to see if reload is required */ -bool git_sortedcache_out_of_date(git_sortedcache *sc) -{ - return (git_futils_filestamp_check(&sc->stamp, sc->path) != 0); + return error; } /* lock sortedcache while making modifications */ -int git_sortedcache_lock(git_sortedcache *sc) +int git_sortedcache_wlock(git_sortedcache *sc) { - GIT_UNUSED(sc); /* to prevent warning when compiled w/o threads */ + GIT_UNUSED(sc); /* prevent warning when compiled w/o threads */ - if (git_mutex_lock(&sc->lock) < 0) { - giterr_set(GITERR_OS, "Unable to acquire mutex lock"); + if (git_rwlock_wrlock(&sc->lock) < 0) { + giterr_set(GITERR_OS, "Unable to acquire write lock on cache"); return -1; } return 0; } /* unlock sorted cache when done with modifications */ -int git_sortedcache_unlock(git_sortedcache *sc) +void git_sortedcache_wunlock(git_sortedcache *sc) { git_vector_sort(&sc->items); - git_mutex_unlock(&sc->lock); + git_rwlock_wrunlock(&sc->lock); +} + +/* lock sortedcache for read */ +int git_sortedcache_rlock(git_sortedcache *sc) +{ + GIT_UNUSED(sc); /* prevent warning when compiled w/o threads */ + + if (git_rwlock_rdlock(&sc->lock) < 0) { + giterr_set(GITERR_OS, "Unable to acquire read lock on cache"); + return -1; + } return 0; } +/* unlock sorted cache when done reading */ +void git_sortedcache_runlock(git_sortedcache *sc) +{ + GIT_UNUSED(sc); /* prevent warning when compiled w/o threads */ + git_rwlock_rdunlock(&sc->lock); +} + /* if the file has changed, lock cache and load file contents into buf; * returns <0 on error, >0 if file has not changed */ @@ -196,7 +199,7 @@ int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf) { int error, fd; - if ((error = git_sortedcache_lock(sc)) < 0) + if ((error = git_sortedcache_wlock(sc)) < 0) return error; if ((error = git_futils_filestamp_check(&sc->stamp, sc->path)) <= 0) @@ -224,13 +227,33 @@ int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf) return 1; /* return 1 -> file needs reload and was successfully loaded */ unlock: - git_sortedcache_unlock(sc); + git_sortedcache_wunlock(sc); return error; } +void git_sortedcache_updated(git_sortedcache *sc) +{ + /* update filestamp to latest value */ + if (git_futils_filestamp_check(&sc->stamp, sc->path) < 0) + giterr_clear(); +} + +/* release all items in sorted cache */ +int git_sortedcache_clear(git_sortedcache *sc, bool wlock) +{ + if (wlock && git_sortedcache_wlock(sc) < 0) + return -1; + + sortedcache_clear(sc); + + if (wlock) + git_sortedcache_wunlock(sc); + + return 0; +} + /* find and/or insert item, returning pointer to item data */ -int git_sortedcache_upsert( - void **out, git_sortedcache *sc, const char *key) +int git_sortedcache_upsert(void **out, git_sortedcache *sc, const char *key) { int error = 0; khiter_t pos; @@ -246,7 +269,10 @@ int git_sortedcache_upsert( keylen = strlen(key); item = git_pool_mallocz(&sc->pool, sc->item_path_offset + keylen + 1); - GITERR_CHECK_ALLOC(item); + if (!item) { /* don't use GITERR_CHECK_ALLOC b/c of lock */ + error = -1; + goto done; + } /* one strange thing is that even if the vector or hash table insert * fail, there is no way to free the pool item so we just abandon it @@ -289,11 +315,16 @@ size_t git_sortedcache_entrycount(const git_sortedcache *sc) } /* lookup item by index */ -void *git_sortedcache_entry(const git_sortedcache *sc, size_t pos) +void *git_sortedcache_entry(git_sortedcache *sc, size_t pos) { + /* make sure the items are sorted so this gets the correct item */ + if (!sc->items.sorted) + git_vector_sort(&sc->items); + return git_vector_get(&sc->items, pos); } +/* helper struct so bsearch callback can know offset + key value for cmp */ struct sortedcache_magic_key { size_t offset; const char *key; @@ -319,23 +350,18 @@ int git_sortedcache_lookup_index( } /* remove entry from cache */ -int git_sortedcache_remove(git_sortedcache *sc, size_t pos, bool lock) +int git_sortedcache_remove(git_sortedcache *sc, size_t pos) { - int error = 0; char *item; khiter_t mappos; - if (lock && git_sortedcache_lock(sc) < 0) - return -1; - /* because of pool allocation, this can't actually remove the item, * but we can remove it from the items vector and the hash table. */ if ((item = git_vector_get(&sc->items, pos)) == NULL) { giterr_set(GITERR_INVALID, "Removing item out of range"); - error = GIT_ENOTFOUND; - goto done; + return GIT_ENOTFOUND; } (void)git_vector_remove(&sc->items, pos); @@ -346,9 +372,6 @@ int git_sortedcache_remove(git_sortedcache *sc, size_t pos, bool lock) if (sc->free_item) sc->free_item(sc->free_item_payload, item); -done: - if (lock) - git_sortedcache_unlock(sc); - return error; + return 0; } diff --git a/src/sortedcache.h b/src/sortedcache.h index c1c9d1341..7d1cd2f14 100644 --- a/src/sortedcache.h +++ b/src/sortedcache.h @@ -25,7 +25,7 @@ typedef void (*git_sortedcache_free_item_fn)(void *payload, void *item); typedef struct { git_refcount rc; - git_mutex lock; + git_rwlock lock; size_t item_path_offset; git_sortedcache_free_item_fn free_item; void *free_item_payload; @@ -36,84 +36,120 @@ typedef struct { char path[GIT_FLEX_ARRAY]; } git_sortedcache; -/* create a new sortedcache +/* Create a new sortedcache * - * even though every sortedcache stores items with a GIT_FLEX_ARRAY at + * Even though every sortedcache stores items with a GIT_FLEX_ARRAY at * the end containing their key string, you have to provide the item_cmp * sorting function because the sorting function doesn't get a payload * and therefore can't know the offset to the item key string. :-( */ int git_sortedcache_new( git_sortedcache **out, - size_t item_path_offset, /* use offsetof() macro */ + size_t item_path_offset, /* use offsetof(struct, path-field) macro */ git_sortedcache_free_item_fn free_item, void *free_item_payload, git_vector_cmp item_cmp, const char *path); -/* copy a sorted cache +/* Copy a sorted cache * - * - copy_item can be NULL to memcpy - * - locks src while copying + * - `copy_item` can be NULL to just use memcpy + * - if `wlock`, grabs write lock on `src` during copy and releases after */ int git_sortedcache_copy( git_sortedcache **out, git_sortedcache *src, + bool wlock, int (*copy_item)(void *payload, void *tgt_item, void *src_item), void *payload); -/* free sorted cache (first calling free_item callbacks) - * don't call on a locked collection - it may acquire a lock +/* Free sorted cache (first calling `free_item` callbacks) + * + * Don't call on a locked collection - it may acquire a write lock */ void git_sortedcache_free(git_sortedcache *sc); -/* increment reference count - balance with call to free */ +/* Increment reference count - balance with call to free */ void git_sortedcache_incref(git_sortedcache *sc); -/* release all items in sorted cache - lock during clear if `lock` is true */ -void git_sortedcache_clear(git_sortedcache *sc, bool lock); +/* Get the pathname associated with this cache at creation time */ +const char *git_sortedcache_path(git_sortedcache *sc); -/* check file stamp to see if reload is required */ -bool git_sortedcache_out_of_date(git_sortedcache *sc); +/* + * CACHE WRITE FUNCTIONS + * + * The following functions require you to have a writer lock to make the + * modification. Some of the functions take a `wlock` parameter and + * will optionally lock and unlock for you if that is passed as true. + * + */ -/* lock sortedcache during access or modification */ -int git_sortedcache_lock(git_sortedcache *sc); +/* Lock sortedcache for write */ +int git_sortedcache_wlock(git_sortedcache *sc); -/* unlock sorted cache when done */ -int git_sortedcache_unlock(git_sortedcache *sc); +/* Unlock sorted cache when done with write */ +void git_sortedcache_wunlock(git_sortedcache *sc); -/* if the file has changed, lock cache and load file contents into buf; +/* Lock cache and test if the file has changed. If the file has changed, + * then load the contents into `buf` otherwise unlock and return 0. + * * @return 0 if up-to-date, 1 if out-of-date, <0 on error */ int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf); -/* find and/or insert item, returning pointer to item data - * should only call on locked collection +/* Refresh file timestamp after write completes + * You should already be holding the write lock when you call this. + */ +void git_sortedcache_updated(git_sortedcache *sc); + +/* Release all items in sorted cache + * + * If `wlock` is true, grabs write lock and releases when done, otherwise + * you should already be holding a write lock when you call this. + */ +int git_sortedcache_clear(git_sortedcache *sc, bool wlock); + +/* Find and/or insert item, returning pointer to item data. + * You should already be holding the write lock when you call this. */ int git_sortedcache_upsert( void **out, git_sortedcache *sc, const char *key); -/* lookup item by key - * should only call on locked collection if return value will be used +/* Removes entry at pos from cache + * You should already be holding the write lock when you call this. */ +int git_sortedcache_remove(git_sortedcache *sc, size_t pos); + +/* + * CACHE READ FUNCTIONS + * + * The following functions access items in the cache. To prevent the + * results from being invalidated before they can be used, you should be + * holding either a read lock or a write lock when using these functions. + * + */ + +/* Lock sortedcache for read */ +int git_sortedcache_rlock(git_sortedcache *sc); + +/* Unlock sorted cache when done with read */ +void git_sortedcache_runlock(git_sortedcache *sc); + +/* Lookup item by key - returns NULL if not found */ void *git_sortedcache_lookup(const git_sortedcache *sc, const char *key); -/* find out how many items are in the cache */ +/* Get how many items are in the cache + * + * You can call this function without holding a lock, but be aware + * that it may change before you use it. + */ size_t git_sortedcache_entrycount(const git_sortedcache *sc); -/* lookup item by index - * should only call on locked collection if return value will be used - */ -void *git_sortedcache_entry(const git_sortedcache *sc, size_t pos); +/* Lookup item by index - returns NULL if out of range */ +void *git_sortedcache_entry(git_sortedcache *sc, size_t pos); -/* lookup index of item by key - * if collection is not locked, there is no guarantee the returned index - * will be correct if it used to look up the item - */ +/* Lookup index of item by key - returns GIT_ENOTFOUND if not found */ int git_sortedcache_lookup_index( size_t *out, git_sortedcache *sc, const char *key); -/* remove entry from cache - lock during delete if `lock` is true */ -int git_sortedcache_remove(git_sortedcache *sc, size_t pos, bool lock); - #endif diff --git a/src/thread-utils.h b/src/thread-utils.h index 04e02959f..ffcdb4ab0 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -41,7 +41,8 @@ typedef git_atomic git_atomic_ssize; #ifdef GIT_THREADS #define git_thread pthread_t -#define git_thread_create(thread, attr, start_routine, arg) pthread_create(thread, attr, start_routine, arg) +#define git_thread_create(thread, attr, start_routine, arg) \ + pthread_create(thread, attr, start_routine, arg) #define git_thread_kill(thread) pthread_cancel(thread) #define git_thread_exit(status) pthread_exit(status) #define git_thread_join(id, status) pthread_join(id, status) @@ -61,6 +62,17 @@ typedef git_atomic git_atomic_ssize; #define git_cond_signal(c) pthread_cond_signal(c) #define git_cond_broadcast(c) pthread_cond_broadcast(c) +/* Pthreads rwlock */ +#define git_rwlock pthread_rwlock_t +#define git_rwlock_init(a) pthread_rwlock_init(a, NULL) +#define git_rwlock_rdlock(a) pthread_rwlock_rdlock(a) +#define git_rwlock_rdunlock(a) pthread_rwlock_unlock(a) +#define git_rwlock_wrlock(a) pthread_rwlock_wrlock(a) +#define git_rwlock_wrunlock(a) pthread_rwlock_unlock(a) +#define git_rwlock_free(a) pthread_rwlock_destroy(a) +#define GIT_RWLOCK_STATIC_INIT PTHREAD_RWLOCK_INITIALIZER + + GIT_INLINE(void) git_atomic_set(git_atomic *a, int val) { #if defined(GIT_WIN32) @@ -147,7 +159,7 @@ GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend) #else #define git_thread unsigned int -#define git_thread_create(thread, attr, start_routine, arg) (void)0 +#define git_thread_create(thread, attr, start_routine, arg) 0 #define git_thread_kill(thread) (void)0 #define git_thread_exit(status) (void)0 #define git_thread_join(id, status) (void)0 @@ -167,6 +179,17 @@ GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend) #define git_cond_signal(c) (void)0 #define git_cond_broadcast(c) (void)0 +/* Pthreads rwlock */ +#define git_rwlock unsigned int +#define git_rwlock_init(a) 0 +#define git_rwlock_rdlock(a) 0 +#define git_rwlock_rdunlock(a) (void)0 +#define git_rwlock_wrlock(a) 0 +#define git_rwlock_wrunlock(a) (void)0 +#define git_rwlock_free(a) (void)0 +#define GIT_RWLOCK_STATIC_INIT 0 + + GIT_INLINE(void) git_atomic_set(git_atomic *a, int val) { a->val = val; diff --git a/tests-clar/core/sortedcache.c b/tests-clar/core/sortedcache.c index 91415943d..509d0df6b 100644 --- a/tests-clar/core/sortedcache.c +++ b/tests-clar/core/sortedcache.c @@ -15,13 +15,13 @@ void test_core_sortedcache__name_only(void) cl_git_pass(git_sortedcache_new( &sc, 0, NULL, NULL, name_only_cmp, NULL)); - cl_git_pass(git_sortedcache_lock(sc)); + cl_git_pass(git_sortedcache_wlock(sc)); cl_git_pass(git_sortedcache_upsert(&item, sc, "aaa")); cl_git_pass(git_sortedcache_upsert(&item, sc, "bbb")); cl_git_pass(git_sortedcache_upsert(&item, sc, "zzz")); cl_git_pass(git_sortedcache_upsert(&item, sc, "mmm")); cl_git_pass(git_sortedcache_upsert(&item, sc, "iii")); - cl_git_pass(git_sortedcache_unlock(sc)); + git_sortedcache_wunlock(sc); cl_assert_equal_sz(5, git_sortedcache_entrycount(sc)); @@ -95,7 +95,7 @@ void test_core_sortedcache__in_memory(void) sortedcache_test_struct_free, &free_count, sortedcache_test_struct_cmp, NULL)); - cl_git_pass(git_sortedcache_lock(sc)); + cl_git_pass(git_sortedcache_wlock(sc)); cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "aaa")); item->value = 10; item->smaller_value = 1; @@ -111,10 +111,12 @@ void test_core_sortedcache__in_memory(void) cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "iii")); item->value = 50; item->smaller_value = 9; - cl_git_pass(git_sortedcache_unlock(sc)); + git_sortedcache_wunlock(sc); cl_assert_equal_sz(5, git_sortedcache_entrycount(sc)); + cl_git_pass(git_sortedcache_rlock(sc)); + cl_assert((item = git_sortedcache_lookup(sc, "aaa")) != NULL); cl_assert_equal_s("aaa", item->path); cl_assert_equal_i(10, item->value); @@ -126,6 +128,8 @@ void test_core_sortedcache__in_memory(void) cl_assert_equal_i(30, item->value); cl_assert(git_sortedcache_lookup(sc, "abc") == NULL); + cl_git_pass(git_sortedcache_rlock(sc)); /* grab more than one */ + cl_assert((item = git_sortedcache_entry(sc, 0)) != NULL); cl_assert_equal_s("aaa", item->path); cl_assert_equal_i(10, item->value); @@ -143,6 +147,9 @@ void test_core_sortedcache__in_memory(void) cl_assert_equal_i(30, item->value); cl_assert(git_sortedcache_entry(sc, 5) == NULL); + git_sortedcache_runlock(sc); + git_sortedcache_runlock(sc); + cl_assert_equal_i(0, free_count); git_sortedcache_clear(sc, true); @@ -156,7 +163,7 @@ void test_core_sortedcache__in_memory(void) free_count = 0; - cl_git_pass(git_sortedcache_lock(sc)); + cl_git_pass(git_sortedcache_wlock(sc)); cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "testing")); item->value = 10; item->smaller_value = 3; @@ -166,7 +173,7 @@ void test_core_sortedcache__in_memory(void) cl_git_pass(git_sortedcache_upsert((void **)&item, sc, "final")); item->value = 30; item->smaller_value = 2; - cl_git_pass(git_sortedcache_unlock(sc)); + git_sortedcache_wunlock(sc); cl_assert_equal_sz(3, git_sortedcache_entrycount(sc)); @@ -195,9 +202,11 @@ void test_core_sortedcache__in_memory(void) { size_t pos; + cl_git_pass(git_sortedcache_wlock(sc)); + cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "again")); cl_assert_equal_sz(0, pos); - cl_git_pass(git_sortedcache_remove(sc, pos, true)); + cl_git_pass(git_sortedcache_remove(sc, pos)); cl_assert_equal_i( GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "again")); @@ -205,7 +214,7 @@ void test_core_sortedcache__in_memory(void) cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "testing")); cl_assert_equal_sz(1, pos); - cl_git_pass(git_sortedcache_remove(sc, pos, true)); + cl_git_pass(git_sortedcache_remove(sc, pos)); cl_assert_equal_i( GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "testing")); @@ -213,11 +222,13 @@ void test_core_sortedcache__in_memory(void) cl_git_pass(git_sortedcache_lookup_index(&pos, sc, "final")); cl_assert_equal_sz(0, pos); - cl_git_pass(git_sortedcache_remove(sc, pos, true)); + cl_git_pass(git_sortedcache_remove(sc, pos)); cl_assert_equal_i( GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "final")); cl_assert_equal_sz(0, git_sortedcache_entrycount(sc)); + + git_sortedcache_wunlock(sc); } git_sortedcache_free(sc); @@ -251,7 +262,7 @@ static void sortedcache_test_reload(git_sortedcache *sc) item->smaller_value = (char)(count++); } - cl_git_pass(git_sortedcache_unlock(sc)); + git_sortedcache_wunlock(sc); git_buf_free(&buf); } From 2b6e1908476c95c84d3e3a62ac069f789156b070 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 22 Aug 2013 11:50:10 -0700 Subject: [PATCH 10/19] A bit of item alignment paranoia --- src/sortedcache.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/sortedcache.c b/src/sortedcache.c index c087dbbe9..16dd6a7a1 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -258,7 +258,7 @@ int git_sortedcache_upsert(void **out, git_sortedcache *sc, const char *key) int error = 0; khiter_t pos; void *item; - size_t keylen; + size_t keylen, itemlen; char *item_key; pos = git_strmap_lookup_index(sc->map, key); @@ -267,9 +267,12 @@ int git_sortedcache_upsert(void **out, git_sortedcache *sc, const char *key) goto done; } - keylen = strlen(key); - item = git_pool_mallocz(&sc->pool, sc->item_path_offset + keylen + 1); - if (!item) { /* don't use GITERR_CHECK_ALLOC b/c of lock */ + keylen = strlen(key); + itemlen = sc->item_path_offset + keylen + 1; + itemlen = (itemlen + 7) & ~7; + + if ((item = git_pool_mallocz(&sc->pool, itemlen)) == NULL) { + /* don't use GITERR_CHECK_ALLOC b/c of lock */ error = -1; goto done; } From 972bb689c4a69ba5a5dfaeebacc7198622c4f051 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 22 Aug 2013 14:10:56 -0700 Subject: [PATCH 11/19] Add SRWLock implementation of rwlocks for Win32 --- src/thread-utils.h | 9 +++++++-- src/win32/pthread.c | 38 +++++++++++++++++++++++++++++++++++ src/win32/pthread.h | 14 ++++++++++++- tests-clar/core/sortedcache.c | 6 ++++-- 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/thread-utils.h b/src/thread-utils.h index ffcdb4ab0..819e24e7b 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -66,12 +66,17 @@ typedef git_atomic git_atomic_ssize; #define git_rwlock pthread_rwlock_t #define git_rwlock_init(a) pthread_rwlock_init(a, NULL) #define git_rwlock_rdlock(a) pthread_rwlock_rdlock(a) -#define git_rwlock_rdunlock(a) pthread_rwlock_unlock(a) +#define git_rwlock_rdunlock(a) pthread_rwlock_rdunlock(a) #define git_rwlock_wrlock(a) pthread_rwlock_wrlock(a) -#define git_rwlock_wrunlock(a) pthread_rwlock_unlock(a) +#define git_rwlock_wrunlock(a) pthread_rwlock_wrunlock(a) #define git_rwlock_free(a) pthread_rwlock_destroy(a) #define GIT_RWLOCK_STATIC_INIT PTHREAD_RWLOCK_INITIALIZER +#ifndef GIT_WIN32 +#define pthread_rwlock_rdunlock pthread_rwlock_unlock +#define pthread_rwlock_wrunlock pthread_rwlock_unlock +#endif + GIT_INLINE(void) git_atomic_set(git_atomic *a, int val) { diff --git a/src/win32/pthread.c b/src/win32/pthread.c index 2f263b3e0..41cb7a4c0 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -142,3 +142,41 @@ int pthread_num_processors_np(void) return n ? n : 1; } +int pthread_rwlock_init( + pthread_rwlock_t *GIT_RESTRICT lock, + const pthread_rwlockattr_t *GIT_RESTRICT attr) +{ + (void)attr; + InitializeSRWLock(lock); + return 0; +} + +int pthread_rwlock_rdlock(pthread_rwlock_t *lock) +{ + AcquireSRWLockShared(lock); + return 0; +} + +int pthread_rwlock_rdunlock(pthread_rwlock_t *lock) +{ + ReleaseSRWLockShared(lock); + return 0; +} + +int pthread_rwlock_wrlock(pthread_rwlock_t *lock) +{ + AcquireSRWLockExclusive(lock); + return 0; +} + +int pthread_rwlock_wrunlock(pthread_rwlock_t *lock) +{ + ReleaseSRWLockExclusive(lock); + return 0; +} + +int pthread_rwlock_destroy(pthread_rwlock_t *lock) +{ + (void)lock; + return 0; +} diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 8277ecf6e..54e5286a6 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -19,11 +19,15 @@ typedef int pthread_mutexattr_t; typedef int pthread_condattr_t; typedef int pthread_attr_t; +typedef int pthread_rwlockattr_t; + typedef CRITICAL_SECTION pthread_mutex_t; typedef HANDLE pthread_t; typedef HANDLE pthread_cond_t; +typedef SRWLOCK pthread_rwlock_t; -#define PTHREAD_MUTEX_INITIALIZER {(void*)-1}; +#define PTHREAD_MUTEX_INITIALIZER {(void*)-1} +#define PTHREAD_RWLOCK_INITIALIZER SRWLOCK_INIT int pthread_create( pthread_t *GIT_RESTRICT, @@ -47,4 +51,12 @@ int pthread_cond_signal(pthread_cond_t *); int pthread_num_processors_np(void); +int pthread_rwlock_init( + pthread_rwlock_t *GIT_RESTRICT, const pthread_rwlockattr_t *GIT_RESTRICT); +int pthread_rwlock_rdlock(pthread_rwlock_t *); +int pthread_rwlock_rdunlock(pthread_rwlock_t *); +int pthread_rwlock_wrlock(pthread_rwlock_t *); +int pthread_rwlock_wrunlock(pthread_rwlock_t *); +int pthread_rwlock_destroy(pthread_rwlock_t *); + #endif diff --git a/tests-clar/core/sortedcache.c b/tests-clar/core/sortedcache.c index 509d0df6b..c1869bee0 100644 --- a/tests-clar/core/sortedcache.c +++ b/tests-clar/core/sortedcache.c @@ -128,7 +128,9 @@ void test_core_sortedcache__in_memory(void) cl_assert_equal_i(30, item->value); cl_assert(git_sortedcache_lookup(sc, "abc") == NULL); - cl_git_pass(git_sortedcache_rlock(sc)); /* grab more than one */ + /* not on Windows: + * cl_git_pass(git_sortedcache_rlock(sc)); -- grab more than one + */ cl_assert((item = git_sortedcache_entry(sc, 0)) != NULL); cl_assert_equal_s("aaa", item->path); @@ -148,7 +150,7 @@ void test_core_sortedcache__in_memory(void) cl_assert(git_sortedcache_entry(sc, 5) == NULL); git_sortedcache_runlock(sc); - git_sortedcache_runlock(sc); + /* git_sortedcache_runlock(sc); */ cl_assert_equal_i(0, free_count); From eb868b1e98d7cea8796f9b92be04843a7f819e5e Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 22 Aug 2013 14:34:21 -0700 Subject: [PATCH 12/19] Drop support for THREADSAFE on Windows XP This makes libgit2 require Windows Vista or newer if it is going to be compiled with the THREADSAFE option --- CMakeLists.txt | 7 ++++++- src/thread-utils.h | 14 +++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1c70ec2d6..019777e78 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -287,8 +287,13 @@ FILE(GLOB SRC_H include/git2.h include/git2/*.h include/git2/sys/*.h) # On Windows use specific platform sources IF (WIN32 AND NOT CYGWIN) - ADD_DEFINITIONS(-DWIN32 -D_WIN32_WINNT=0x0501) + ADD_DEFINITIONS(-DWIN32) FILE(GLOB SRC_OS src/win32/*.c src/win32/*.h) + IF (THREADSAFE) + ADD_DEFINITIONS(-D_WIN32_WINNT=0x0600) + ELSE() + ADD_DEFINITIONS(-D_WIN32_WINNT=0x0501) + ENDIF() ELSEIF (AMIGA) ADD_DEFINITIONS(-DNO_ADDRINFO -DNO_READDIR_R) FILE(GLOB SRC_OS src/amiga/*.c src/amiga/*.h) diff --git a/src/thread-utils.h b/src/thread-utils.h index 819e24e7b..371dc0b26 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -40,6 +40,10 @@ typedef git_atomic git_atomic_ssize; #ifdef GIT_THREADS +#if defined(GIT_WIN32) && _WIN32_WINNT < 0x0600 +# error "Unsupported Windows version for thread support" +#endif + #define git_thread pthread_t #define git_thread_create(thread, attr, start_routine, arg) \ pthread_create(thread, attr, start_routine, arg) @@ -62,7 +66,15 @@ typedef git_atomic git_atomic_ssize; #define git_cond_signal(c) pthread_cond_signal(c) #define git_cond_broadcast(c) pthread_cond_broadcast(c) -/* Pthreads rwlock */ +/* Pthread (-ish) rwlock + * + * This differs from normal pthreads rwlocks in two ways: + * 1. Separate APIs for releasing read locks and write locks (as + * opposed to the pure POSIX API which only has one unlock fn) + * 2. You should not use recursive read locks (i.e. grabbing a read + * lock in a thread that already holds a read lock) because the + * Windows implementation doesn't support it + */ #define git_rwlock pthread_rwlock_t #define git_rwlock_init(a) pthread_rwlock_init(a, NULL) #define git_rwlock_rdlock(a) pthread_rwlock_rdlock(a) From b6ac07b51771641f3ae994c17f361fbd8bec36ef Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 22 Aug 2013 14:45:10 -0700 Subject: [PATCH 13/19] Trying to fix Win32 warnings --- src/cc-compat.h | 8 ++++++-- src/diff_tform.c | 6 +++--- src/sortedcache.c | 2 +- src/win32/pthread.h | 12 +++++++----- tests-clar/stress/diff.c | 2 +- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/cc-compat.h b/src/cc-compat.h index a5e4ce17e..37f1ea81e 100644 --- a/src/cc-compat.h +++ b/src/cc-compat.h @@ -54,8 +54,12 @@ #if defined (_MSC_VER) typedef unsigned char bool; -# define true 1 -# define false 0 +# ifndef true +# define true 1 +# endif +# ifndef false +# define false 0 +# endif #else # include #endif diff --git a/src/diff_tform.c b/src/diff_tform.c index ba35d3c14..6b8cf446e 100644 --- a/src/diff_tform.c +++ b/src/diff_tform.c @@ -859,9 +859,9 @@ find_best_matches: } /* write new mapping */ - tgt2src[t].idx = s; + tgt2src[t].idx = (uint32_t)s; tgt2src[t].similarity = (uint32_t)similarity; - src2tgt[s].idx = t; + src2tgt[s].idx = (uint32_t)t; src2tgt[s].similarity = (uint32_t)similarity; } @@ -869,7 +869,7 @@ find_best_matches: if (tgt2src_copy != NULL && tgt2src_copy[t].similarity < (uint32_t)similarity) { - tgt2src_copy[t].idx = s; + tgt2src_copy[t].idx = (uint32_t)s; tgt2src_copy[t].similarity = (uint32_t)similarity; } diff --git a/src/sortedcache.c b/src/sortedcache.c index 16dd6a7a1..33171c48d 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -271,7 +271,7 @@ int git_sortedcache_upsert(void **out, git_sortedcache *sc, const char *key) itemlen = sc->item_path_offset + keylen + 1; itemlen = (itemlen + 7) & ~7; - if ((item = git_pool_mallocz(&sc->pool, itemlen)) == NULL) { + if ((item = git_pool_mallocz(&sc->pool, (uint32_t)itemlen)) == NULL) { /* don't use GITERR_CHECK_ALLOC b/c of lock */ error = -1; goto done; diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 54e5286a6..50d836247 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -30,15 +30,16 @@ typedef SRWLOCK pthread_rwlock_t; #define PTHREAD_RWLOCK_INITIALIZER SRWLOCK_INIT int pthread_create( - pthread_t *GIT_RESTRICT, - const pthread_attr_t *GIT_RESTRICT, + pthread_t *GIT_RESTRICT thread, + const pthread_attr_t *GIT_RESTRICT attr, void *(*start_routine)(void*), - void *__restrict); + void *GIT_RESTRICT arg); int pthread_join(pthread_t, void **); int pthread_mutex_init( - pthread_mutex_t *GIT_RESTRICT, const pthread_mutexattr_t *GIT_RESTRICT); + pthread_mutex_t *GIT_RESTRICT mutex, + const pthread_mutexattr_t *GIT_RESTRICT mutexattr); int pthread_mutex_destroy(pthread_mutex_t *); int pthread_mutex_lock(pthread_mutex_t *); int pthread_mutex_unlock(pthread_mutex_t *); @@ -52,7 +53,8 @@ int pthread_cond_signal(pthread_cond_t *); int pthread_num_processors_np(void); int pthread_rwlock_init( - pthread_rwlock_t *GIT_RESTRICT, const pthread_rwlockattr_t *GIT_RESTRICT); + pthread_rwlock_t *GIT_RESTRICT lock, + const pthread_rwlockattr_t *GIT_RESTRICT attr); int pthread_rwlock_rdlock(pthread_rwlock_t *); int pthread_rwlock_rdunlock(pthread_rwlock_t *); int pthread_rwlock_wrlock(pthread_rwlock_t *); diff --git a/tests-clar/stress/diff.c b/tests-clar/stress/diff.c index 62ccd5ec7..0524aa108 100644 --- a/tests-clar/stress/diff.c +++ b/tests-clar/stress/diff.c @@ -15,7 +15,7 @@ void test_stress_diff__cleanup(void) #define ANOTHER_POEM \ "OH, glorious are the guarded heights\nWhere guardian souls abide—\nSelf-exiled from our gross delights—\nAbove, beyond, outside:\nAn ampler arc their spirit swings—\nCommands a juster view—\nWe have their word for all these things,\nNo doubt their words are true.\n\nYet we, the bond slaves of our day,\nWhom dirt and danger press—\nCo-heirs of insolence, delay,\nAnd leagued unfaithfulness—\nSuch is our need must seek indeed\nAnd, having found, engage\nThe men who merely do the work\nFor which they draw the wage.\n\nFrom forge and farm and mine and bench,\nDeck, altar, outpost lone—\nMill, school, battalion, counter, trench,\nRail, senate, sheepfold, throne—\nCreation's cry goes up on high\nFrom age to cheated age:\n\"Send us the men who do the work\n\"For which they draw the wage!\"\n" -static void test_with_many(size_t expected_new) +static void test_with_many(int expected_new) { git_index *index; git_tree *tree, *new_tree; From 805755f49b0db5bc884f8929621ac61238b2c30e Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 22 Aug 2013 15:44:34 -0700 Subject: [PATCH 14/19] Fix sortedcache docs and other feedback This converts an internal lock from a write lock to a read lock where write isn't needed, and also clarifies some doc things about where various locks are acquired and how various APIs are intended to be used. --- src/sortedcache.c | 8 ++++---- src/sortedcache.h | 35 ++++++++++++++++++++++++++++------- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/sortedcache.c b/src/sortedcache.c index 33171c48d..466e55dbe 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -110,7 +110,7 @@ static int sortedcache_copy_item(void *payload, void *tgt_item, void *src_item) int git_sortedcache_copy( git_sortedcache **out, git_sortedcache *src, - bool wlock, + bool lock, int (*copy_item)(void *payload, void *tgt_item, void *src_item), void *payload) { @@ -131,7 +131,7 @@ int git_sortedcache_copy( src->items._cmp, src->path)) < 0) return error; - if (wlock && git_sortedcache_wlock(src) < 0) { + if (lock && git_sortedcache_rlock(src) < 0) { git_sortedcache_free(tgt); return -1; } @@ -144,8 +144,8 @@ int git_sortedcache_copy( break; } - if (wlock) - git_sortedcache_wunlock(src); + if (lock) + git_sortedcache_runlock(src); if (error) git_sortedcache_free(tgt); diff --git a/src/sortedcache.h b/src/sortedcache.h index 7d1cd2f14..5ebb116ed 100644 --- a/src/sortedcache.h +++ b/src/sortedcache.h @@ -16,9 +16,10 @@ /* * The purpose of this data structure is to cache the parsed contents of a - * file where each item in the file can be identified by a key string and - * you want to both look them up by name and traverse them in sorted - * order. Each item is assumed to itself end in a GIT_FLEX_ARRAY. + * file (a.k.a. the backing file) where each item in the file can be + * identified by a key string and you want to both look them up by name + * and traverse them in sorted order. Each item is assumed to itself end + * in a GIT_FLEX_ARRAY. */ typedef void (*git_sortedcache_free_item_fn)(void *payload, void *item); @@ -42,6 +43,16 @@ typedef struct { * the end containing their key string, you have to provide the item_cmp * sorting function because the sorting function doesn't get a payload * and therefore can't know the offset to the item key string. :-( + * + * @param out The allocated git_sortedcache + * @param item_path_offset Offset to the GIT_FLEX_ARRAY item key in the + * struct - use offsetof(struct mine, key-field) to get this + * @param free_item Optional callback to free each item + * @param free_item_payload Optional payload passed to free_item callback + * @param item_cmp Compare the keys of two items + * @param path The path to the backing store file for this cache; this + * may be NULL. The cache makes it easy to load this and check + * if it has been modified since the last load and/or write. */ int git_sortedcache_new( git_sortedcache **out, @@ -54,12 +65,12 @@ int git_sortedcache_new( /* Copy a sorted cache * * - `copy_item` can be NULL to just use memcpy - * - if `wlock`, grabs write lock on `src` during copy and releases after + * - if `lock`, grabs read lock on `src` during copy and releases after */ int git_sortedcache_copy( git_sortedcache **out, git_sortedcache *src, - bool wlock, + bool lock, int (*copy_item)(void *payload, void *tgt_item, void *src_item), void *payload); @@ -90,8 +101,18 @@ int git_sortedcache_wlock(git_sortedcache *sc); /* Unlock sorted cache when done with write */ void git_sortedcache_wunlock(git_sortedcache *sc); -/* Lock cache and test if the file has changed. If the file has changed, - * then load the contents into `buf` otherwise unlock and return 0. +/* Lock cache and load backing file into a buffer. + * + * This grabs a write lock on the cache then looks at the modification + * time and size of the file on disk. + * + * If the file appears to have changed, this loads the file contents into + * the buffer and returns a positive value leaving the cache locked - the + * caller should parse the file content, update the cache as needed, then + * release the lock. NOTE: In this case, the caller MUST unlock the cache. + * + * If the file appears to be unchanged, then this automatically releases + * the lock on the cache, clears the buffer, and returns 0. * * @return 0 if up-to-date, 1 if out-of-date, <0 on error */ From 44d655318661affa2feb51e9d6d533bb16d7f2b5 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 19 Aug 2013 16:03:15 -0700 Subject: [PATCH 15/19] Fix comment --- tests-clar/refs/pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests-clar/refs/pack.c b/tests-clar/refs/pack.c index 6019ed75a..849a052aa 100644 --- a/tests-clar/refs/pack.c +++ b/tests-clar/refs/pack.c @@ -44,7 +44,7 @@ void test_refs_pack__empty(void) void test_refs_pack__loose(void) { - /* create a packfile from all the loose rn a repo */ + /* create a packfile from all the loose refs in a repo */ git_reference *reference; git_buf temp_path = GIT_BUF_INIT; From 430953417f74dfcdbe030bafc069e1c07edceeb6 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 26 Aug 2013 14:56:31 -0700 Subject: [PATCH 16/19] Load SRWLock APIs at runtime This loads SRWLock APIs at runtime and in their absence (i.e. on Windows before Vista) falls back on a regular CRITICAL_SECTION that will not permit concurrent readers. --- CMakeLists.txt | 7 +--- src/global.c | 12 ++++-- src/hash/hash_win32.c | 3 +- src/thread-utils.h | 4 -- src/win32/pthread.c | 89 ++++++++++++++++++++++++++++++++++++++----- src/win32/pthread.h | 14 ++++++- 6 files changed, 103 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 019777e78..1c70ec2d6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -287,13 +287,8 @@ FILE(GLOB SRC_H include/git2.h include/git2/*.h include/git2/sys/*.h) # On Windows use specific platform sources IF (WIN32 AND NOT CYGWIN) - ADD_DEFINITIONS(-DWIN32) + ADD_DEFINITIONS(-DWIN32 -D_WIN32_WINNT=0x0501) FILE(GLOB SRC_OS src/win32/*.c src/win32/*.h) - IF (THREADSAFE) - ADD_DEFINITIONS(-D_WIN32_WINNT=0x0600) - ELSE() - ADD_DEFINITIONS(-D_WIN32_WINNT=0x0501) - ENDIF() ELSEIF (AMIGA) ADD_DEFINITIONS(-DNO_ADDRINFO -DNO_READDIR_R) FILE(GLOB SRC_OS src/amiga/*.c src/amiga/*.h) diff --git a/src/global.c b/src/global.c index a06d0c81f..b504e5e0a 100644 --- a/src/global.c +++ b/src/global.c @@ -71,18 +71,22 @@ int git_threads_init(void) GIT_MEMORY_BARRIER; + win32_pthread_initialize(); + return error; } void git_threads_shutdown(void) { + /* Shut down any subsystems that have global state */ + win32_pthread_shutdown(); + git_futils_dirs_free(); + git_hash_global_shutdown(); + TlsFree(_tls_index); _tls_init = 0; - git_mutex_free(&git__mwindow_mutex); - /* Shut down any subsystems that have global state */ - git_hash_global_shutdown(); - git_futils_dirs_free(); + git_mutex_free(&git__mwindow_mutex); } git_global_st *git__global_state(void) diff --git a/src/hash/hash_win32.c b/src/hash/hash_win32.c index 43d54ca6d..6732f93d7 100644 --- a/src/hash/hash_win32.c +++ b/src/hash/hash_win32.c @@ -46,7 +46,8 @@ GIT_INLINE(int) hash_cng_prov_init(void) return -1; /* Load bcrypt.dll explicitly from the system directory */ - if ((dll_path_len = GetSystemDirectory(dll_path, MAX_PATH)) == 0 || dll_path_len > MAX_PATH || + if ((dll_path_len = GetSystemDirectory(dll_path, MAX_PATH)) == 0 || + dll_path_len > MAX_PATH || StringCchCat(dll_path, MAX_PATH, "\\") < 0 || StringCchCat(dll_path, MAX_PATH, GIT_HASH_CNG_DLL_NAME) < 0 || (hash_prov.prov.cng.dll = LoadLibrary(dll_path)) == NULL) diff --git a/src/thread-utils.h b/src/thread-utils.h index 371dc0b26..914c1357d 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -40,10 +40,6 @@ typedef git_atomic git_atomic_ssize; #ifdef GIT_THREADS -#if defined(GIT_WIN32) && _WIN32_WINNT < 0x0600 -# error "Unsupported Windows version for thread support" -#endif - #define git_thread pthread_t #define git_thread_create(thread, attr, start_routine, arg) \ pthread_create(thread, attr, start_routine, arg) diff --git a/src/win32/pthread.c b/src/win32/pthread.c index 41cb7a4c0..8775f632a 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -127,9 +127,10 @@ int pthread_cond_signal(pthread_cond_t *cond) return 0; } -/* pthread_cond_broadcast is not implemented because doing so with just Win32 events - * is quite complicated, and no caller in libgit2 uses it yet. */ - +/* pthread_cond_broadcast is not implemented because doing so with just + * Win32 events is quite complicated, and no caller in libgit2 uses it + * yet. + */ int pthread_num_processors_np(void) { DWORD_PTR p, s; @@ -142,41 +143,111 @@ int pthread_num_processors_np(void) return n ? n : 1; } + +static HINSTANCE win32_kernel32_dll; + +typedef void (WINAPI *win32_srwlock_fn)(SRWLOCK *); + +static win32_srwlock_fn win32_srwlock_initialize; +static win32_srwlock_fn win32_srwlock_acquire_shared; +static win32_srwlock_fn win32_srwlock_release_shared; +static win32_srwlock_fn win32_srwlock_acquire_exclusive; +static win32_srwlock_fn win32_srwlock_release_exclusive; + int pthread_rwlock_init( pthread_rwlock_t *GIT_RESTRICT lock, const pthread_rwlockattr_t *GIT_RESTRICT attr) { (void)attr; - InitializeSRWLock(lock); + + if (win32_srwlock_initialize) + win32_srwlock_initialize(&lock->native.srwl); + else + InitializeCriticalSection(&lock->native.csec); + return 0; } int pthread_rwlock_rdlock(pthread_rwlock_t *lock) { - AcquireSRWLockShared(lock); + if (win32_srwlock_acquire_shared) + win32_srwlock_acquire_shared(&lock->native.srwl); + else + EnterCriticalSection(&lock->native.csec); + return 0; } int pthread_rwlock_rdunlock(pthread_rwlock_t *lock) { - ReleaseSRWLockShared(lock); + if (win32_srwlock_release_shared) + win32_srwlock_release_shared(&lock->native.srwl); + else + LeaveCriticalSection(&lock->native.csec); + return 0; } int pthread_rwlock_wrlock(pthread_rwlock_t *lock) { - AcquireSRWLockExclusive(lock); + if (win32_srwlock_acquire_exclusive) + win32_srwlock_acquire_exclusive(&lock->native.srwl); + else + EnterCriticalSection(&lock->native.csec); + return 0; } int pthread_rwlock_wrunlock(pthread_rwlock_t *lock) { - ReleaseSRWLockExclusive(lock); + if (win32_srwlock_release_exclusive) + win32_srwlock_release_exclusive(&lock->native.srwl); + else + LeaveCriticalSection(&lock->native.csec); + return 0; } int pthread_rwlock_destroy(pthread_rwlock_t *lock) { - (void)lock; + if (!win32_srwlock_initialize) + DeleteCriticalSection(&lock->native.csec); + git__memzero(lock, sizeof(*lock)); + return 0; +} + + +int win32_pthread_initialize(void) +{ + if (win32_kernel32_dll) + return 0; + + win32_kernel32_dll = LoadLibrary("Kernel32.dll"); + if (!win32_kernel32_dll) { + giterr_set(GITERR_OS, "Could not load Kernel32.dll!"); + return -1; + } + + win32_srwlock_initialize = (win32_srwlock_fn) + GetProcAddress(win32_kernel32_dll, "InitializeSRWLock"); + win32_srwlock_acquire_shared = (win32_srwlock_fn) + GetProcAddress(win32_kernel32_dll, "AcquireSRWLockShared"); + win32_srwlock_release_shared = (win32_srwlock_fn) + GetProcAddress(win32_kernel32_dll, "ReleaseSRWLockShared"); + win32_srwlock_acquire_exclusive = (win32_srwlock_fn) + GetProcAddress(win32_kernel32_dll, "AcquireSRWLockExclusive"); + win32_srwlock_release_exclusive = (win32_srwlock_fn) + GetProcAddress(win32_kernel32_dll, "ReleaseSRWLockExclusive"); + + return 0; +} + +int win32_pthread_shutdown(void) +{ + if (win32_kernel32_dll) { + FreeLibrary(win32_kernel32_dll); + win32_kernel32_dll = NULL; + } + return 0; } diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 50d836247..e84de471f 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -24,10 +24,17 @@ typedef int pthread_rwlockattr_t; typedef CRITICAL_SECTION pthread_mutex_t; typedef HANDLE pthread_t; typedef HANDLE pthread_cond_t; -typedef SRWLOCK pthread_rwlock_t; + +/* typedef struct { void *Ptr; } SRWLOCK; */ + +typedef struct { + union { + SRWLOCK srwl; + CRITICAL_SECTION csec; + } native; +} pthread_rwlock_t; #define PTHREAD_MUTEX_INITIALIZER {(void*)-1} -#define PTHREAD_RWLOCK_INITIALIZER SRWLOCK_INIT int pthread_create( pthread_t *GIT_RESTRICT thread, @@ -61,4 +68,7 @@ int pthread_rwlock_wrlock(pthread_rwlock_t *); int pthread_rwlock_wrunlock(pthread_rwlock_t *); int pthread_rwlock_destroy(pthread_rwlock_t *); +extern int win32_pthread_initialize(void); +extern int win32_pthread_shutdown(void); + #endif From 2f368a661c55b49a8f15905c221f9e76935bedd0 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 26 Aug 2013 15:17:35 -0700 Subject: [PATCH 17/19] Fix MINGW SRWLock typedefs --- src/win32/mingw-compat.h | 2 ++ src/win32/pthread.h | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/win32/mingw-compat.h b/src/win32/mingw-compat.h index fe0abfb54..b06dda209 100644 --- a/src/win32/mingw-compat.h +++ b/src/win32/mingw-compat.h @@ -24,6 +24,8 @@ GIT_INLINE(size_t) p_strnlen(const char *s, size_t maxlen) { return end ? (size_t)(end - s) : maxlen; } +typedef struct { void *Ptr; } SRWLOCK; + #endif #endif /* INCLUDE_mingw_compat__ */ diff --git a/src/win32/pthread.h b/src/win32/pthread.h index e84de471f..679ebed23 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -25,8 +25,6 @@ typedef CRITICAL_SECTION pthread_mutex_t; typedef HANDLE pthread_t; typedef HANDLE pthread_cond_t; -/* typedef struct { void *Ptr; } SRWLOCK; */ - typedef struct { union { SRWLOCK srwl; From f087bc245e9f3934d43c49f4034ee9b5638884dd Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 27 Aug 2013 12:08:55 -0700 Subject: [PATCH 18/19] Convert to our own SRWLOCK type on Win32 --- src/win32/mingw-compat.h | 2 -- src/win32/pthread.c | 2 +- src/win32/pthread.h | 4 +++- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/win32/mingw-compat.h b/src/win32/mingw-compat.h index b06dda209..fe0abfb54 100644 --- a/src/win32/mingw-compat.h +++ b/src/win32/mingw-compat.h @@ -24,8 +24,6 @@ GIT_INLINE(size_t) p_strnlen(const char *s, size_t maxlen) { return end ? (size_t)(end - s) : maxlen; } -typedef struct { void *Ptr; } SRWLOCK; - #endif #endif /* INCLUDE_mingw_compat__ */ diff --git a/src/win32/pthread.c b/src/win32/pthread.c index 8775f632a..d50ace695 100644 --- a/src/win32/pthread.c +++ b/src/win32/pthread.c @@ -146,7 +146,7 @@ int pthread_num_processors_np(void) static HINSTANCE win32_kernel32_dll; -typedef void (WINAPI *win32_srwlock_fn)(SRWLOCK *); +typedef void (WINAPI *win32_srwlock_fn)(GIT_SRWLOCK *); static win32_srwlock_fn win32_srwlock_initialize; static win32_srwlock_fn win32_srwlock_acquire_shared; diff --git a/src/win32/pthread.h b/src/win32/pthread.h index 679ebed23..2ba2ca552 100644 --- a/src/win32/pthread.h +++ b/src/win32/pthread.h @@ -25,9 +25,11 @@ typedef CRITICAL_SECTION pthread_mutex_t; typedef HANDLE pthread_t; typedef HANDLE pthread_cond_t; +typedef struct { void *Ptr; } GIT_SRWLOCK; + typedef struct { union { - SRWLOCK srwl; + GIT_SRWLOCK srwl; CRITICAL_SECTION csec; } native; } pthread_rwlock_t; From b2d3efcbce2d12cfa9736ab4f9283c91600a8a75 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 28 Aug 2013 09:31:32 -0700 Subject: [PATCH 19/19] Some documentation improvements --- include/git2/commit.h | 16 +++++++++++---- include/git2/revparse.h | 43 ++++++++++++++++++++++++++--------------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/include/git2/commit.h b/include/git2/commit.h index fc0551be1..0eaf917bd 100644 --- a/include/git2/commit.h +++ b/include/git2/commit.h @@ -24,17 +24,24 @@ GIT_BEGIN_DECL /** * Lookup a commit object from a repository. * + * The returned object should be released with `git_commit_free` when no + * longer needed. + * * @param commit pointer to the looked up commit * @param repo the repo to use when locating the commit. * @param id identity of the commit to locate. If the object is * an annotated tag it will be peeled back to the commit. * @return 0 or an error code */ -GIT_EXTERN(int) git_commit_lookup(git_commit **commit, git_repository *repo, const git_oid *id); +GIT_EXTERN(int) git_commit_lookup( + git_commit **commit, git_repository *repo, const git_oid *id); /** - * Lookup a commit object from a repository, - * given a prefix of its identifier (short id). + * Lookup a commit object from a repository, given a prefix of its + * identifier (short id). + * + * The returned object should be released with `git_commit_free` when no + * longer needed. * * @see git_object_lookup_prefix * @@ -45,7 +52,8 @@ GIT_EXTERN(int) git_commit_lookup(git_commit **commit, git_repository *repo, con * @param len the length of the short identifier * @return 0 or an error code */ -GIT_EXTERN(int) git_commit_lookup_prefix(git_commit **commit, git_repository *repo, const git_oid *id, size_t len); +GIT_EXTERN(int) git_commit_lookup_prefix( + git_commit **commit, git_repository *repo, const git_oid *id, size_t len); /** * Close an open commit diff --git a/include/git2/revparse.h b/include/git2/revparse.h index 786a9da57..d170e1621 100644 --- a/include/git2/revparse.h +++ b/include/git2/revparse.h @@ -10,7 +10,6 @@ #include "common.h" #include "types.h" - /** * @file git2/revparse.h * @brief Git revision parsing routines @@ -21,27 +20,37 @@ GIT_BEGIN_DECL /** - * Find a single object, as specified by a revision string. See `man gitrevisions`, - * or http://git-scm.com/docs/git-rev-parse.html#_specifying_revisions for + * Find a single object, as specified by a revision string. + * + * See `man gitrevisions`, or + * http://git-scm.com/docs/git-rev-parse.html#_specifying_revisions for * information on the syntax accepted. * + * The returned object should be released with `git_object_free` when no + * longer needed. + * * @param out pointer to output object * @param repo the repository to search in * @param spec the textual specification for an object * @return 0 on success, GIT_ENOTFOUND, GIT_EAMBIGUOUS, GIT_EINVALIDSPEC or an error code */ -GIT_EXTERN(int) git_revparse_single(git_object **out, git_repository *repo, const char *spec); +GIT_EXTERN(int) git_revparse_single( + git_object **out, git_repository *repo, const char *spec); /** - * Find a single object, as specified by a revision string. - * See `man gitrevisions`, - * or http://git-scm.com/docs/git-rev-parse.html#_specifying_revisions for + * Find a single object and intermediate reference by a revision string. + * + * See `man gitrevisions`, or + * http://git-scm.com/docs/git-rev-parse.html#_specifying_revisions for * information on the syntax accepted. * * In some cases (`@{<-n>}` or `@{upstream}`), the expression may * point to an intermediate reference. When such expressions are being passed * in, `reference_out` will be valued as well. * + * The returned object should be released with `git_object_free` and the + * returned reference with `git_reference_free` when no longer needed. + * * @param object_out pointer to output object * @param reference_out pointer to output reference or NULL * @param repo the repository to search in @@ -76,25 +85,27 @@ typedef struct { git_object *from; /** The right element of the revspec; must be freed by the user */ git_object *to; - /** The intent of the revspec */ + /** The intent of the revspec (i.e. `git_revparse_mode_t` flags) */ unsigned int flags; } git_revspec; /** - * Parse a revision string for `from`, `to`, and intent. See `man gitrevisions` or - * http://git-scm.com/docs/git-rev-parse.html#_specifying_revisions for information - * on the syntax accepted. + * Parse a revision string for `from`, `to`, and intent. * - * @param revspec Pointer to an user-allocated git_revspec struct where the result - * of the rev-parse will be stored + * See `man gitrevisions` or + * http://git-scm.com/docs/git-rev-parse.html#_specifying_revisions for + * information on the syntax accepted. + * + * @param revspec Pointer to an user-allocated git_revspec struct where + * the result of the rev-parse will be stored * @param repo the repository to search in * @param spec the rev-parse spec to parse * @return 0 on success, GIT_INVALIDSPEC, GIT_ENOTFOUND, GIT_EAMBIGUOUS or an error code */ GIT_EXTERN(int) git_revparse( - git_revspec *revspec, - git_repository *repo, - const char *spec); + git_revspec *revspec, + git_repository *repo, + const char *spec); /** @} */