reftable/stack: use lock_file when adding table to "tables.list"

When modifying "tables.list", we need to lock the list before updating it to ensure that no concurrent writers modify the list at the same point in time. While we do this via the `lock_file` subsystem when compacting the stack, we manually handle the lock when adding a new table to it. While not wrong, it is at least inconsistent. Refactor the code to consistently lock "tables.list" via the `lock_file` subsytem. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 8, 2024 at 16:06 UTC 128b9aa3e9d3dc0417f8f65a240aad736db97959
1 file changed +11 -10
reftable/stack.c
+11 -10
@@ -567,7 +567,7 @@ static void format_name(struct strbuf *dest, uint64_t min, uint64_t max)
567 }
568
569 struct reftable_addition {
570 - struct tempfile *lock_file;
570 + struct lock_file tables_list_lock;
571 struct reftable_stack *stack;
572
573 char **new_tables;
@@ -581,13 +581,13 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
581 struct reftable_stack *st)
582 {
583 struct strbuf lock_file_name = STRBUF_INIT;
584 - int err = 0;
585 - add->stack = st;
584 + int err;
585
587 - strbuf_addf(&lock_file_name, "%s.lock", st->list_file);
586 + add->stack = st;
587
589 - add->lock_file = create_tempfile(lock_file_name.buf);
590 - if (!add->lock_file) {
588 + err = hold_lock_file_for_update(&add->tables_list_lock, st->list_file,
589 + LOCK_NO_DEREF);
590 + if (err < 0) {
591 if (errno == EEXIST) {
592 err = REFTABLE_LOCK_ERROR;
593 } else {
@@ -596,7 +596,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
596 goto done;
597 }
598 if (st->opts.default_permissions) {
599 - if (chmod(add->lock_file->filename.buf, st->opts.default_permissions) < 0) {
599 + if (chmod(get_lock_file_path(&add->tables_list_lock),
600 + st->opts.default_permissions) < 0) {
601 err = REFTABLE_IO_ERROR;
602 goto done;
603 }
@@ -635,7 +636,7 @@ static void reftable_addition_close(struct reftable_addition *add)
636 add->new_tables_len = 0;
637 add->new_tables_cap = 0;
638
638 - delete_tempfile(&add->lock_file);
639 + rollback_lock_file(&add->tables_list_lock);
640 strbuf_release(&nm);
641 }
642
@@ -651,7 +652,7 @@ void reftable_addition_destroy(struct reftable_addition *add)
652 int reftable_addition_commit(struct reftable_addition *add)
653 {
654 struct strbuf table_list = STRBUF_INIT;
654 - int lock_file_fd = get_tempfile_fd(add->lock_file);
655 + int lock_file_fd = get_lock_file_fd(&add->tables_list_lock);
656 int err = 0;
657 size_t i;
658
@@ -680,7 +681,7 @@ int reftable_addition_commit(struct reftable_addition *add)
681 goto done;
682 }
683
683 - err = rename_tempfile(&add->lock_file, add->stack->list_file);
684 + err = commit_lock_file(&add->tables_list_lock);
685 if (err < 0) {
686 err = REFTABLE_IO_ERROR;
687 goto done;