diff --git a/reftable/stack.c b/reftable/stack.c index 703548417c..c3d4deff29 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -536,6 +536,8 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir, goto out; } + p->list_lock = REFTABLE_FLOCK_INIT; + err = reftable_stack_reload_maybe_reuse(p, 1); if (err < 0) goto out; @@ -628,10 +630,16 @@ int reftable_stack_reload(struct reftable_stack *st) } struct reftable_addition { - struct reftable_flock tables_list_lock; struct reftable_stack *stack; struct reftable_write_options opts; + /* + * While the list lock is acquired on the stack, we need to distinguish + * which 'reftable_addition' is responsible for the lock. This avoids + * clearing the lock of another 'reftable_addition'. + */ + unsigned int locked : 1; + char **new_tables; size_t new_tables_len, new_tables_cap; uint64_t next_update_index; @@ -653,7 +661,9 @@ static void reftable_addition_close(struct reftable_addition *add) add->new_tables_len = 0; add->new_tables_cap = 0; - flock_release(&add->tables_list_lock); + if (add->locked) + flock_release(&add->stack->list_lock); + add->locked = 0; reftable_buf_release(&nm); } @@ -669,13 +679,14 @@ static int reftable_stack_init_addition(struct reftable_addition *add, if (opts) add->opts = *opts; - err = flock_acquire(&add->tables_list_lock, st->list_file, + err = flock_acquire(&add->stack->list_lock, st->list_file, add->opts.lock_timeout_ms); if (err < 0) goto done; + add->locked = 1; if (add->opts.default_permissions) { - if (chmod(add->tables_list_lock.path, + if (chmod(add->stack->list_lock.path, add->opts.default_permissions) < 0) { err = REFTABLE_IO_ERROR; goto done; @@ -774,7 +785,7 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err = reftable_write_data(add->tables_list_lock.fd, + err = reftable_write_data(add->stack->list_lock.fd, table_list.buf, table_list.len); reftable_buf_release(&table_list); if (err < 0) { @@ -782,17 +793,18 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err = fsync(add->tables_list_lock.fd); + err = fsync(add->stack->list_lock.fd); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } - err = flock_commit(&add->tables_list_lock); + err = flock_commit(&add->stack->list_lock); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } + add->locked = 0; /* success, no more state to clean up. */ for (i = 0; i < add->new_tables_len; i++) diff --git a/reftable/stack.h b/reftable/stack.h index f7901e6c6f..52e07ad551 100644 --- a/reftable/stack.h +++ b/reftable/stack.h @@ -10,7 +10,6 @@ #define STACK_H #include "system.h" -#include "reftable-writer.h" #include "reftable-stack.h" struct reftable_stack { @@ -18,6 +17,12 @@ struct reftable_stack { char *list_file; int list_fd; + /* + * Set while an addition holds the stack locked. Used by + * stack_uptodate() to skip reload checks while locked. + */ + struct reftable_flock list_lock; + char *reftable_dir; struct reftable_stack_options opts; diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c index 04927113c2..b6f1c6cc52 100644 --- a/t/unit-tests/u-reftable-stack.c +++ b/t/unit-tests/u-reftable-stack.c @@ -1310,3 +1310,31 @@ void test_reftable_stack__invalid_limit_updates(void) reftable_stack_destroy(st); clear_dir(dir); } + +void test_reftable_stack__two_additions(void) +{ + struct reftable_stack *st = NULL; + char *dir = get_tmp_dir(__LINE__); + struct reftable_addition *add1 = NULL; + struct reftable_addition *add2 = NULL; + + struct reftable_ref_record ref = { + .refname = (char *) "HEAD", + .update_index = 1, + .value_type = REFTABLE_REF_SYMREF, + .value.symref = (char *) "master", + }; + + cl_assert_equal_i(reftable_new_stack(&st, dir, NULL), 0); + + cl_assert_equal_i(reftable_stack_addition_new(&add1, st, NULL), 0); + cl_assert_equal_i(reftable_stack_addition_new(&add2, st, NULL), REFTABLE_LOCK_ERROR); + + cl_assert_equal_i(reftable_addition_add(add1, write_test_ref, &ref), 0); + + cl_assert_equal_i(reftable_addition_commit(add1), 0); + + reftable_addition_destroy(add1); + reftable_stack_destroy(st); + clear_dir(dir); +}