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); }