[PATCH v3 8/8] refs/reftable: always reload stacks when creating lock
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 12, 2025, 09:54 UTC
- Message-ID
- <20250812-pks-reftable-fixes-for-libgit2-v3-8-cf3b2267867e@pks.im>
- In-Reply-To
- <20250812-pks-reftable-fixes-for-libgit2-v3-0-cf3b2267867e@pks.im>
When creating a new addition via either `reftable_stack_new_addition()` or its convenince wrapper `reftable_stack_add()` we:
1. Create the "tables.list.lock" file.
2. Verify that the current version of the "tables.list" file is
up-to-date.3. Write the new table records if so.
By default, the second step would cause us to bail out if we see that there has been a concurrent write to the stack that made our in-memory copy of the stack out-of-date. This is a safety mechanism to not write records to the stack based on outdated information.
The downside though is that concurrent writes may now cause us to bail out, which is not a good user experience. In addition, this isn't even necessary for us, as Git knows to perform all checks for the old state of references under the lock. (Well, in all except one case: when we expire the reflog we first create the log iterator before we create the lock, but this ordering is fixed as part of this commit.)
Consequently, most writers pass the `REFTABLE_STACK_NEW_ADDITION_RELOAD` flag. The effect of this flag is that we reload the stack after having acquired the lock in case the stack is out-of-date. This plugs the race with concurrent writers, but we continue performing the verifications of the expected old state to catch actual conflicts in the references we are about to write.
Adapt the remaining callsites that don't yet pass this flag to do so. While at it, drop a needless manual reload.
Signed-off-by: Patrick Steinhardt <ps@pks.im> --- refs/reftable-backend.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 3f0deab338..66d25411f1 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1006,10 +1006,6 @@ static int prepare_transaction_update(struct write_transaction_table_arg **out, if (!arg) { struct reftable_addition *addition; - ret = reftable_stack_reload(be->stack); - if (ret) - return ret; - ret = reftable_stack_new_addition(&addition, be->stack, REFTABLE_STACK_NEW_ADDITION_RELOAD); if (ret) { @@ -1960,7 +1956,8 @@ static int reftable_be_rename_ref(struct ref_store *ref_store, ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1); if (ret) goto done; - ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg, 0); + ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg, + REFTABLE_STACK_NEW_ADDITION_RELOAD); done: assert(ret != REFTABLE_API_ERROR); @@ -1989,7 +1986,8 @@ static int reftable_be_copy_ref(struct ref_store *ref_store, ret = backend_for(&arg.be, refs, newrefname, &newrefname, 1); if (ret) goto done; - ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg, 0); + ret = reftable_stack_add(arg.be->stack, &write_copy_table, &arg, + REFTABLE_STACK_NEW_ADDITION_RELOAD); done: assert(ret != REFTABLE_API_ERROR); @@ -2360,7 +2358,8 @@ static int reftable_be_create_reflog(struct ref_store *ref_store, goto done; arg.stack = be->stack; - ret = reftable_stack_add(be->stack, &write_reflog_existence_table, &arg, 0); + ret = reftable_stack_add(be->stack, &write_reflog_existence_table, &arg, + REFTABLE_STACK_NEW_ADDITION_RELOAD); done: return ret; @@ -2431,7 +2430,8 @@ static int reftable_be_delete_reflog(struct ref_store *ref_store, return ret; arg.stack = be->stack; - ret = reftable_stack_add(be->stack, &write_reflog_delete_table, &arg, 0); + ret = reftable_stack_add(be->stack, &write_reflog_delete_table, &arg, + REFTABLE_STACK_NEW_ADDITION_RELOAD); assert(ret != REFTABLE_API_ERROR); return ret; @@ -2552,15 +2552,16 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store, if (ret < 0) goto done; - ret = reftable_stack_init_log_iterator(be->stack, &it); + ret = reftable_stack_new_addition(&add, be->stack, + REFTABLE_STACK_NEW_ADDITION_RELOAD); if (ret < 0) goto done; - ret = reftable_iterator_seek_log(&it, refname); + ret = reftable_stack_init_log_iterator(be->stack, &it); if (ret < 0) goto done; - ret = reftable_stack_new_addition(&add, be->stack, 0); + ret = reftable_iterator_seek_log(&it, refname); if (ret < 0) goto done;
-- 2.51.0.rc1.163.g2494970778.dirty