reftable/stack: fix stale lock when dying

When starting a transaction via `reftable_stack_init_addition()`, we
create a lockfile for the reftable stack itself which we'll write the
new list of tables to. But if we terminate abnormally e.g. via a call to
`die()`, then we do not remove the lockfile. Subsequent executions of
Git which try to modify references will thus fail with an out-of-date
error.

Fix this bug by registering the lock as a `struct tempfile`, which
ensures automatic cleanup for us.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Patrick Steinhardt 2023-12-11 10:07:54 +01:00 committed by Junio C Hamano
parent d779996a10
commit 3054fbd93e

View file

@ -17,6 +17,8 @@ license that can be found in the LICENSE file or at
#include "reftable-merged.h" #include "reftable-merged.h"
#include "writer.h" #include "writer.h"
#include "tempfile.h"
static int stack_try_add(struct reftable_stack *st, static int stack_try_add(struct reftable_stack *st,
int (*write_table)(struct reftable_writer *wr, int (*write_table)(struct reftable_writer *wr,
void *arg), void *arg),
@ -440,8 +442,7 @@ static void format_name(struct strbuf *dest, uint64_t min, uint64_t max)
} }
struct reftable_addition { struct reftable_addition {
int lock_file_fd; struct tempfile *lock_file;
struct strbuf lock_file_name;
struct reftable_stack *stack; struct reftable_stack *stack;
char **new_tables; char **new_tables;
@ -449,24 +450,19 @@ struct reftable_addition {
uint64_t next_update_index; uint64_t next_update_index;
}; };
#define REFTABLE_ADDITION_INIT \ #define REFTABLE_ADDITION_INIT {0}
{ \
.lock_file_name = STRBUF_INIT \
}
static int reftable_stack_init_addition(struct reftable_addition *add, static int reftable_stack_init_addition(struct reftable_addition *add,
struct reftable_stack *st) struct reftable_stack *st)
{ {
struct strbuf lock_file_name = STRBUF_INIT;
int err = 0; int err = 0;
add->stack = st; add->stack = st;
strbuf_reset(&add->lock_file_name); strbuf_addf(&lock_file_name, "%s.lock", st->list_file);
strbuf_addstr(&add->lock_file_name, st->list_file);
strbuf_addstr(&add->lock_file_name, ".lock");
add->lock_file_fd = open(add->lock_file_name.buf, add->lock_file = create_tempfile(lock_file_name.buf);
O_EXCL | O_CREAT | O_WRONLY, 0666); if (!add->lock_file) {
if (add->lock_file_fd < 0) {
if (errno == EEXIST) { if (errno == EEXIST) {
err = REFTABLE_LOCK_ERROR; err = REFTABLE_LOCK_ERROR;
} else { } else {
@ -475,7 +471,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
goto done; goto done;
} }
if (st->config.default_permissions) { if (st->config.default_permissions) {
if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) { if (chmod(add->lock_file->filename.buf, st->config.default_permissions) < 0) {
err = REFTABLE_IO_ERROR; err = REFTABLE_IO_ERROR;
goto done; goto done;
} }
@ -495,6 +491,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
if (err) { if (err) {
reftable_addition_close(add); reftable_addition_close(add);
} }
strbuf_release(&lock_file_name);
return err; return err;
} }
@ -512,15 +509,7 @@ static void reftable_addition_close(struct reftable_addition *add)
add->new_tables = NULL; add->new_tables = NULL;
add->new_tables_len = 0; add->new_tables_len = 0;
if (add->lock_file_fd > 0) { delete_tempfile(&add->lock_file);
close(add->lock_file_fd);
add->lock_file_fd = 0;
}
if (add->lock_file_name.len > 0) {
unlink(add->lock_file_name.buf);
strbuf_release(&add->lock_file_name);
}
strbuf_release(&nm); strbuf_release(&nm);
} }
@ -536,8 +525,10 @@ void reftable_addition_destroy(struct reftable_addition *add)
int reftable_addition_commit(struct reftable_addition *add) int reftable_addition_commit(struct reftable_addition *add)
{ {
struct strbuf table_list = STRBUF_INIT; struct strbuf table_list = STRBUF_INIT;
int lock_file_fd = get_tempfile_fd(add->lock_file);
int i = 0; int i = 0;
int err = 0; int err = 0;
if (add->new_tables_len == 0) if (add->new_tables_len == 0)
goto done; goto done;
@ -550,28 +541,20 @@ int reftable_addition_commit(struct reftable_addition *add)
strbuf_addstr(&table_list, "\n"); strbuf_addstr(&table_list, "\n");
} }
err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len); err = write_in_full(lock_file_fd, table_list.buf, table_list.len);
strbuf_release(&table_list); strbuf_release(&table_list);
if (err < 0) { if (err < 0) {
err = REFTABLE_IO_ERROR; err = REFTABLE_IO_ERROR;
goto done; goto done;
} }
err = close(add->lock_file_fd); err = rename_tempfile(&add->lock_file, add->stack->list_file);
add->lock_file_fd = 0;
if (err < 0) {
err = REFTABLE_IO_ERROR;
goto done;
}
err = rename(add->lock_file_name.buf, add->stack->list_file);
if (err < 0) { if (err < 0) {
err = REFTABLE_IO_ERROR; err = REFTABLE_IO_ERROR;
goto done; goto done;
} }
/* success, no more state to clean up. */ /* success, no more state to clean up. */
strbuf_release(&add->lock_file_name);
for (i = 0; i < add->new_tables_len; i++) { for (i = 0; i < add->new_tables_len; i++) {
reftable_free(add->new_tables[i]); reftable_free(add->new_tables[i]);
} }