diff options
| -rw-r--r-- | src/fileops.c | 10 | ||||
| -rw-r--r-- | src/path.c | 4 | ||||
| -rw-r--r-- | src/refdb.c | 6 | ||||
| -rw-r--r-- | src/refdb_fs.c | 130 | ||||
| -rw-r--r-- | src/sortedcache.c | 18 | ||||
| -rw-r--r-- | tests/threads/refdb.c | 165 |
6 files changed, 185 insertions, 148 deletions
diff --git a/src/fileops.c b/src/fileops.c index fcc0301f9..a82202c98 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -72,8 +72,16 @@ int git_futils_creat_locked(const char *path, const mode_t mode) O_EXCL | O_BINARY | O_CLOEXEC, mode); if (fd < 0) { + int error = errno; giterr_set(GITERR_OS, "Failed to create locked file '%s'", path); - return errno == EEXIST ? GIT_ELOCKED : -1; + switch (error) { + case EEXIST: + return GIT_ELOCKED; + case ENOENT: + return GIT_ENOTFOUND; + default: + return -1; + } } return fd; diff --git a/src/path.c b/src/path.c index f91e42242..2b1a9622e 100644 --- a/src/path.c +++ b/src/path.c @@ -644,6 +644,10 @@ int git_path_set_error(int errno_value, const char *path, const char *action) giterr_set(GITERR_OS, "Failed %s - '%s' already exists", action, path); return GIT_EEXISTS; + case EACCES: + giterr_set(GITERR_OS, "Failed %s - '%s' is locked", action, path); + return GIT_ELOCKED; + default: giterr_set(GITERR_OS, "Could not %s '%s'", action, path); return -1; diff --git a/src/refdb.c b/src/refdb.c index debba1276..85c84892b 100644 --- a/src/refdb.c +++ b/src/refdb.c @@ -125,13 +125,15 @@ int git_refdb_lookup(git_reference **out, git_refdb *db, const char *ref_name) int git_refdb_iterator(git_reference_iterator **out, git_refdb *db, const char *glob) { + int error; + if (!db->backend || !db->backend->iterator) { giterr_set(GITERR_REFERENCE, "This backend doesn't support iterators"); return -1; } - if (db->backend->iterator(out, db->backend, glob) < 0) - return -1; + if ((error = db->backend->iterator(out, db->backend, glob)) < 0) + return error; GIT_REFCOUNT_INC(db); (*out)->db = db; diff --git a/src/refdb_fs.c b/src/refdb_fs.c index f978038e6..7601aa0ac 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -326,12 +326,13 @@ static int refdb_fs_backend__exists( { refdb_fs_backend *backend = (refdb_fs_backend *)_backend; git_buf ref_path = GIT_BUF_INIT; + int error; assert(backend); - if (packed_reload(backend) < 0 || - git_buf_joinpath(&ref_path, backend->path, ref_name) < 0) - return -1; + if ((error = packed_reload(backend)) < 0 || + (error = git_buf_joinpath(&ref_path, backend->path, ref_name)) < 0) + return error; *exists = git_path_isfile(ref_path.ptr) || (git_sortedcache_lookup(backend->refcache, ref_name) != NULL); @@ -409,8 +410,8 @@ static int packed_lookup( int error = 0; struct packref *entry; - if (packed_reload(backend) < 0) - return -1; + if ((error = packed_reload(backend)) < 0) + return error; if (git_sortedcache_rlock(backend->refcache) < 0) return -1; @@ -615,13 +616,14 @@ static int refdb_fs_backend__iterator_next_name( static int refdb_fs_backend__iterator( git_reference_iterator **out, git_refdb_backend *_backend, const char *glob) { + int error; refdb_fs_iter *iter; refdb_fs_backend *backend = (refdb_fs_backend *)_backend; assert(backend); - if (packed_reload(backend) < 0) - return -1; + if ((error = packed_reload(backend)) < 0) + return error; iter = git__calloc(1, sizeof(refdb_fs_iter)); GITERR_CHECK_ALLOC(iter); @@ -674,16 +676,18 @@ static int reference_path_available( int force) { size_t i; + int error; - if (packed_reload(backend) < 0) - return -1; + if ((error = packed_reload(backend)) < 0) + return error; if (!force) { int exists; - if (refdb_fs_backend__exists( - &exists, (git_refdb_backend *)backend, new_ref) < 0) - return -1; + if ((error = refdb_fs_backend__exists( + &exists, (git_refdb_backend *)backend, new_ref)) < 0) { + return error; + } if (exists) { giterr_set(GITERR_REFERENCE, @@ -901,40 +905,60 @@ static int packed_write_ref(struct packref *ref, git_filebuf *file) static int packed_remove_loose(refdb_fs_backend *backend) { size_t i; - git_buf full_path = GIT_BUF_INIT; - int failed = 0; + git_filebuf lock = GIT_FILEBUF_INIT; + git_buf ref_content = GIT_BUF_INIT; + int error = 0; /* 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); + git_oid current_id; if (!ref || !(ref->flags & PACKREF_WAS_LOOSE)) continue; - if (git_buf_joinpath(&full_path, backend->path, ref->name) < 0) - return -1; /* critical; do not try to recover on oom */ + git_filebuf_cleanup(&lock); - if (git_path_exists(full_path.ptr) && p_unlink(full_path.ptr) < 0) { - if (failed) - continue; + /* We need to stop anybody from updating the ref while we try to do a safe delete */ + error = loose_lock(&lock, backend, ref->name); + /* If someone else is updating it, let them do it */ + if (error == GIT_EEXISTS || error == GIT_ENOTFOUND) + continue; - giterr_set(GITERR_REFERENCE, - "Failed to remove loose reference '%s' after packing: %s", - full_path.ptr, strerror(errno)); - failed = 1; + if (error < 0) { + giterr_set(GITERR_REFERENCE, "failed to lock loose reference '%s'", ref->name); + return error; } + error = git_futils_readbuffer(&ref_content, lock.path_original); + /* Someone else beat us to cleaning up the ref, let's simply continue */ + if (error == GIT_ENOTFOUND) + continue; + + /* This became a symref between us packing and trying to delete it, so ignore it */ + if (!git__prefixcmp(ref_content.ptr, GIT_SYMREF)) + continue; + + /* Figure out the current id; if we find a bad ref file, skip it so we can do the rest */ + if (loose_parse_oid(¤t_id, lock.path_original, &ref_content) < 0) + continue; + + /* If the ref moved since we packed it, we must not delete it */ + if (!git_oid_equal(¤t_id, &ref->oid)) + continue; + /* * if we fail to remove a single file, this is *not* good, * but we should keep going and remove as many as possible. - * After we've removed as many files as possible, we return - * the error code anyway. + * If we fail to remove, the ref is still in the old state, so + * we haven't lost information. */ + p_unlink(lock.path_original); } - git_buf_free(&full_path); - return failed ? -1 : 0; + git_filebuf_cleanup(&lock); + return 0; } /* @@ -944,41 +968,42 @@ static int packed_write(refdb_fs_backend *backend) { git_sortedcache *refcache = backend->refcache; git_filebuf pack_file = GIT_FILEBUF_INIT; + int error; size_t i; /* lock the cache to updates while we do this */ - if (git_sortedcache_wlock(refcache) < 0) - return -1; + if ((error = git_sortedcache_wlock(refcache)) < 0) + return error; /* Open the file! */ - if (git_filebuf_open(&pack_file, git_sortedcache_path(refcache), 0, GIT_PACKEDREFS_FILE_MODE) < 0) + if ((error = git_filebuf_open(&pack_file, git_sortedcache_path(refcache), 0, GIT_PACKEDREFS_FILE_MODE)) < 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) + if ((error = git_filebuf_printf(&pack_file, "%s\n", GIT_PACKEDREFS_HEADER)) < 0) goto fail; for (i = 0; i < git_sortedcache_entrycount(refcache); ++i) { struct packref *ref = git_sortedcache_entry(refcache, i); assert(ref); - if (packed_find_peel(backend, ref) < 0) + if ((error = packed_find_peel(backend, ref)) < 0) goto fail; - if (packed_write_ref(ref, &pack_file) < 0) + if ((error = packed_write_ref(ref, &pack_file)) < 0) 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) < 0) + if ((error = git_filebuf_commit(&pack_file)) < 0) 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) + if ((error = packed_remove_loose(backend)) < 0) goto fail; git_sortedcache_updated(refcache); @@ -991,7 +1016,7 @@ fail: git_filebuf_cleanup(&pack_file); git_sortedcache_wunlock(refcache); - return -1; + return error; } static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_oid *old, const git_oid *new, const git_signature *author, const char *message); @@ -1143,8 +1168,7 @@ static int refdb_fs_backend__write( assert(backend); - error = reference_path_available(backend, ref->name, NULL, force); - if (error < 0) + if ((error = reference_path_available(backend, ref->name, NULL, force)) < 0) return error; /* We need to perform the reflog append and old value check under the ref's lock */ @@ -1260,15 +1284,14 @@ static int refdb_fs_backend__delete_tail( if (git_buf_joinpath(&loose_path, backend->path, ref_name) < 0) return -1; - if (git_path_isfile(loose_path.ptr)) { - error = p_unlink(loose_path.ptr); - loose_deleted = 1; - } - - git_buf_free(&loose_path); - if (error != 0) + error = p_unlink(loose_path.ptr); + if (error < 0 && errno == ENOENT) + error = 0; + else if (error < 0) goto cleanup; + else if (error == 0) + loose_deleted = 1; if ((error = packed_reload(backend)) < 0) goto cleanup; @@ -1291,6 +1314,7 @@ static int refdb_fs_backend__delete_tail( error = packed_write(backend); cleanup: + git_buf_free(&loose_path); git_filebuf_cleanup(file); return error; @@ -1362,14 +1386,15 @@ static int refdb_fs_backend__rename( static int refdb_fs_backend__compress(git_refdb_backend *_backend) { + int error; refdb_fs_backend *backend = (refdb_fs_backend *)_backend; assert(backend); - 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; + if ((error = packed_reload(backend)) < 0 || /* load the existing packfile */ + (error = packed_loadloose(backend)) < 0 || /* add all the loose refs */ + (error = packed_write(backend)) < 0) /* write back to disk */ + return error; return 0; } @@ -1789,9 +1814,10 @@ static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, co * there maybe an obsolete/unused directory (or directory hierarchy) in the way. */ if (git_path_isdir(git_buf_cstr(&path))) { - if ((git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_SKIP_NONEMPTY) < 0)) - error = -1; - else if (git_path_isdir(git_buf_cstr(&path))) { + if ((error = git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_SKIP_NONEMPTY)) < 0) { + if (error == GIT_ENOTFOUND) + error = 0; + } else if (git_path_isdir(git_buf_cstr(&path))) { giterr_set(GITERR_REFERENCE, "cannot create reflog at '%s', there are reflogs beneath that folder", ref->name); error = GIT_EDIRECTORY; diff --git a/src/sortedcache.c b/src/sortedcache.c index 5c2a167a7..ed4199b71 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -200,6 +200,7 @@ void git_sortedcache_runlock(git_sortedcache *sc) int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf) { int error, fd; + struct stat st; if ((error = git_sortedcache_wlock(sc)) < 0) return error; @@ -207,19 +208,26 @@ int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf) 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"); + if ((fd = git_futils_open_ro(sc->path)) < 0) { + error = fd; + goto unlock; + } + + if (p_fstat(fd, &st) < 0) { + giterr_set(GITERR_OS, "failed to stat file"); error = -1; goto unlock; } - if ((fd = git_futils_open_ro(sc->path)) < 0) { - error = fd; + if (!git__is_sizet(st.st_size)) { + giterr_set(GITERR_INVALID, "Unable to load file larger than size_t"); + error = -1; + (void)p_close(fd); goto unlock; } if (buf) - error = git_futils_readbuffer_fd(buf, fd, (size_t)sc->stamp.size); + error = git_futils_readbuffer_fd(buf, fd, (size_t)st.st_size); (void)p_close(fd); diff --git a/tests/threads/refdb.c b/tests/threads/refdb.c index f869bcb44..5484b71d6 100644 --- a/tests/threads/refdb.c +++ b/tests/threads/refdb.c @@ -18,14 +18,27 @@ void test_threads_refdb__cleanup(void) #define REPEAT 20 #define THREADS 20 +/* Number of references to create or delete in each thread */ +#define NREFS 10 + +struct th_data { + int id; + const char *path; +}; static void *iterate_refs(void *arg) { + struct th_data *data = (struct th_data *) arg; git_reference_iterator *i; git_reference *ref; - int count = 0; + int count = 0, error; + git_repository *repo; - cl_git_pass(git_reference_iterator_new(&i, g_repo)); + cl_git_pass(git_repository_open(&repo, data->path)); + do { + error = git_reference_iterator_new(&i, repo); + } while (error == GIT_ELOCKED); + cl_git_pass(error); for (count = 0; !git_reference_next(&ref, i); ++count) { cl_assert(ref != NULL); @@ -37,112 +50,91 @@ static void *iterate_refs(void *arg) git_reference_iterator_free(i); + git_repository_free(repo); giterr_clear(); 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) { - p_snprintf(name, sizeof(name), "refs/heads/direct-%03d", r); - cl_git_pass(git_reference_create(&ref, g_repo, name, &head, 0, NULL)); - 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; -#ifdef GIT_THREADS - cl_git_pass(git_thread_create(&th[t], 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)); - } -} - static void *create_refs(void *arg) { - int *id = arg, i; + int i, error; + struct th_data *data = (struct th_data *) arg; git_oid head; char name[128]; - git_reference *ref[10]; + git_reference *ref[NREFS]; + git_repository *repo; - cl_git_pass(git_reference_name_to_id(&head, g_repo, "HEAD")); + cl_git_pass(git_repository_open(&repo, data->path)); + + do { + error = git_reference_name_to_id(&head, repo, "HEAD"); + } while (error == GIT_ELOCKED); + cl_git_pass(error); - for (i = 0; i < 10; ++i) { - p_snprintf(name, sizeof(name), "refs/heads/thread-%03d-%02d", *id, i); - cl_git_pass(git_reference_create(&ref[i], g_repo, name, &head, 0, NULL)); + for (i = 0; i < NREFS; ++i) { + p_snprintf(name, sizeof(name), "refs/heads/thread-%03d-%02d", data->id, i); + do { + error = git_reference_create(&ref[i], repo, name, &head, 0, NULL); + } while (error == GIT_ELOCKED); + cl_git_pass(error); - if (i == 5) { + if (i == NREFS/2) { git_refdb *refdb; - cl_git_pass(git_repository_refdb(&refdb, g_repo)); - cl_git_pass(git_refdb_compress(refdb)); + cl_git_pass(git_repository_refdb(&refdb, repo)); + do { + error = git_refdb_compress(refdb); + } while (error == GIT_ELOCKED); git_refdb_free(refdb); } } - for (i = 0; i < 10; ++i) + for (i = 0; i < NREFS; ++i) git_reference_free(ref[i]); + git_repository_free(repo); + giterr_clear(); return arg; } static void *delete_refs(void *arg) { - int *id = arg, i; + int i, error; + struct th_data *data = (struct th_data *) arg; git_reference *ref; char name[128]; + git_repository *repo; - for (i = 0; i < 10; ++i) { + cl_git_pass(git_repository_open(&repo, data->path)); + + for (i = 0; i < NREFS; ++i) { p_snprintf( - name, sizeof(name), "refs/heads/thread-%03d-%02d", (*id) & ~0x3, i); + name, sizeof(name), "refs/heads/thread-%03d-%02d", (data->id) & ~0x3, i); + + if (!git_reference_lookup(&ref, repo, name)) { + do { + error = git_reference_delete(ref); + } while (error == GIT_ELOCKED); + /* Sometimes we race with other deleter threads */ + if (error == GIT_ENOTFOUND) + error = 0; - if (!git_reference_lookup(&ref, g_repo, name)) { - cl_git_pass(git_reference_delete(ref)); + cl_git_pass(error); git_reference_free(ref); } - if (i == 5) { + if (i == NREFS/2) { git_refdb *refdb; - cl_git_pass(git_repository_refdb(&refdb, g_repo)); - cl_git_pass(git_refdb_compress(refdb)); + cl_git_pass(git_repository_refdb(&refdb, repo)); + do { + error = git_refdb_compress(refdb); + } while (error == GIT_ELOCKED); + cl_git_pass(error); git_refdb_free(refdb); } } + git_repository_free(repo); giterr_clear(); return arg; } @@ -150,7 +142,7 @@ static void *delete_refs(void *arg) void test_threads_refdb__edit_while_iterate(void) { int r, t; - int id[THREADS]; + struct th_data th_data[THREADS]; git_oid head; git_reference *ref; char name[128]; @@ -189,29 +181,26 @@ void test_threads_refdb__edit_while_iterate(void) default: fn = iterate_refs; break; } - id[t] = t; - - /* It appears with all reflog writing changes, etc., that this - * test has started to fail quite frequently, so let's disable it - * for now by just running on a single thread... - */ -/* #ifdef GIT_THREADS */ -/* cl_git_pass(git_thread_create(&th[t], fn, &id[t])); */ -/* #else */ - fn(&id[t]); -/* #endif */ + th_data[t].id = t; + th_data[t].path = git_repository_path(g_repo); + +#ifdef GIT_THREADS + cl_git_pass(git_thread_create(&th[t], fn, &th_data[t])); +#else + fn(&th_data[t]); +#endif } #ifdef GIT_THREADS -/* for (t = 0; t < THREADS; ++t) { */ -/* cl_git_pass(git_thread_join(th[t], NULL)); */ -/* } */ + 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], iterate_refs, &id[t])); + th_data[t].id = t; + cl_git_pass(git_thread_create(&th[t], iterate_refs, &th_data[t])); } for (t = 0; t < THREADS; ++t) { |