reftable/system: provide thin wrapper for lockfile subsystem

We use the lockfile subsystem to write lockfiles for "tables.list". As with the tempfile subsystem, the lockfile subsystem also hooks into our infrastructure to prune stale locks via atexit(3p) or signal handlers. Furthermore, the lockfile subsystem also handles locking timeouts, which do add quite a bit of logic. Having to reimplement that in the context of Git wouldn't make a whole lot of sense, and it is quite likely that downstream users of the reftable library may have a better idea for how exactly to implement timeouts. So again, provide a thin wrapper for the lockfile subsystem instead such that the compatibility shim is fully self-contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Nov 18, 2024 at 16:34 UTC 988e7f5e952bbb7b6ae885f4da744f536f22693f
8 files changed +154 -37
reftable/stack.c
+27 -36
@@ -657,7 +657,7 @@ static int format_name(struct reftable_buf *dest, uint64_t min, uint64_t max)
657 }
658
659 struct reftable_addition {
660 - struct lock_file tables_list_lock;
660 + struct reftable_flock tables_list_lock;
661 struct reftable_stack *stack;
662
663 char **new_tables;
@@ -676,10 +676,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
676
677 add->stack = st;
678
679 - err = hold_lock_file_for_update_timeout(&add->tables_list_lock,
680 - st->list_file,
681 - LOCK_NO_DEREF,
682 - st->opts.lock_timeout_ms);
679 + err = flock_acquire(&add->tables_list_lock, st->list_file,
680 + st->opts.lock_timeout_ms);
681 if (err < 0) {
682 if (errno == EEXIST) {
683 err = REFTABLE_LOCK_ERROR;
@@ -689,7 +687,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
687 goto done;
688 }
689 if (st->opts.default_permissions) {
692 - if (chmod(get_lock_file_path(&add->tables_list_lock),
690 + if (chmod(add->tables_list_lock.path,
691 st->opts.default_permissions) < 0) {
692 err = REFTABLE_IO_ERROR;
693 goto done;
@@ -733,7 +731,7 @@ static void reftable_addition_close(struct reftable_addition *add)
731 add->new_tables_len = 0;
732 add->new_tables_cap = 0;
733
736 - rollback_lock_file(&add->tables_list_lock);
734 + flock_release(&add->tables_list_lock);
735 reftable_buf_release(&nm);
736 }
737
@@ -749,7 +747,6 @@ void reftable_addition_destroy(struct reftable_addition *add)
747 int reftable_addition_commit(struct reftable_addition *add)
748 {
749 struct reftable_buf table_list = REFTABLE_BUF_INIT;
752 - int lock_file_fd = get_lock_file_fd(&add->tables_list_lock);
750 int err = 0;
751 size_t i;
752
@@ -767,20 +764,20 @@ int reftable_addition_commit(struct reftable_addition *add)
764 goto done;
765 }
766
770 - err = write_in_full(lock_file_fd, table_list.buf, table_list.len);
767 + err = write_in_full(add->tables_list_lock.fd, table_list.buf, table_list.len);
768 reftable_buf_release(&table_list);
769 if (err < 0) {
770 err = REFTABLE_IO_ERROR;
771 goto done;
772 }
773
777 - err = stack_fsync(&add->stack->opts, lock_file_fd);
774 + err = stack_fsync(&add->stack->opts, add->tables_list_lock.fd);
775 if (err < 0) {
776 err = REFTABLE_IO_ERROR;
777 goto done;
778 }
779
783 - err = commit_lock_file(&add->tables_list_lock);
780 + err = flock_commit(&add->tables_list_lock);
781 if (err < 0) {
782 err = REFTABLE_IO_ERROR;
783 goto done;
@@ -1160,8 +1157,8 @@ static int stack_compact_range(struct reftable_stack *st,
1157 struct reftable_buf new_table_name = REFTABLE_BUF_INIT;
1158 struct reftable_buf new_table_path = REFTABLE_BUF_INIT;
1159 struct reftable_buf table_name = REFTABLE_BUF_INIT;
1163 - struct lock_file tables_list_lock = LOCK_INIT;
1164 - struct lock_file *table_locks = NULL;
1160 + struct reftable_flock tables_list_lock = REFTABLE_FLOCK_INIT;
1161 + struct reftable_flock *table_locks = NULL;
1162 struct reftable_tmpfile new_table = REFTABLE_TMPFILE_INIT;
1163 int is_empty_table = 0, err = 0;
1164 size_t first_to_replace, last_to_replace;
@@ -1179,10 +1176,7 @@ static int stack_compact_range(struct reftable_stack *st,
1176 * Hold the lock so that we can read "tables.list" and lock all tables
1177 * which are part of the user-specified range.
1178 */
1182 - err = hold_lock_file_for_update_timeout(&tables_list_lock,
1183 - st->list_file,
1184 - LOCK_NO_DEREF,
1185 - st->opts.lock_timeout_ms);
1179 + err = flock_acquire(&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);
1180 if (err < 0) {
1181 if (errno == EEXIST)
1182 err = REFTABLE_LOCK_ERROR;
@@ -1205,19 +1199,20 @@ static int stack_compact_range(struct reftable_stack *st,
1199 * older process is still busy compacting tables which are preexisting
1200 * from the point of view of the newer process.
1201 */
1208 - REFTABLE_CALLOC_ARRAY(table_locks, last - first + 1);
1202 + REFTABLE_ALLOC_ARRAY(table_locks, last - first + 1);
1203 if (!table_locks) {
1204 err = REFTABLE_OUT_OF_MEMORY_ERROR;
1205 goto done;
1206 }
1207 + for (i = 0; i < last - first + 1; i++)
1208 + table_locks[i] = REFTABLE_FLOCK_INIT;
1209
1210 for (i = last + 1; i > first; i--) {
1211 err = stack_filename(&table_name, st, reader_name(st->readers[i - 1]));
1212 if (err < 0)
1213 goto done;
1214
1219 - err = hold_lock_file_for_update(&table_locks[nlocks],
1220 - table_name.buf, LOCK_NO_DEREF);
1215 + err = flock_acquire(&table_locks[nlocks], table_name.buf, 0);
1216 if (err < 0) {
1217 /*
1218 * When the table is locked already we may do a
@@ -1253,7 +1248,7 @@ static int stack_compact_range(struct reftable_stack *st,
1248 * run into file descriptor exhaustion when we compress a lot
1249 * of tables.
1250 */
1256 - err = close_lock_file_gently(&table_locks[nlocks++]);
1251 + err = flock_close(&table_locks[nlocks++]);
1252 if (err < 0) {
1253 err = REFTABLE_IO_ERROR;
1254 goto done;
@@ -1265,7 +1260,7 @@ static int stack_compact_range(struct reftable_stack *st,
1260 * "tables.list" lock while compacting the locked tables. This allows
1261 * concurrent updates to the stack to proceed.
1262 */
1268 - err = rollback_lock_file(&tables_list_lock);
1263 + err = flock_release(&tables_list_lock);
1264 if (err < 0) {
1265 err = REFTABLE_IO_ERROR;
1266 goto done;
@@ -1288,10 +1283,7 @@ static int stack_compact_range(struct reftable_stack *st,
1283 * "tables.list". We'll then replace the compacted range of tables with
1284 * the new table.
1285 */
1291 - err = hold_lock_file_for_update_timeout(&tables_list_lock,
1292 - st->list_file,
1293 - LOCK_NO_DEREF,
1294 - st->opts.lock_timeout_ms);
1286 + err = flock_acquire(&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);
1287 if (err < 0) {
1288 if (errno == EEXIST)
1289 err = REFTABLE_LOCK_ERROR;
@@ -1301,7 +1293,7 @@ static int stack_compact_range(struct reftable_stack *st,
1293 }
1294
1295 if (st->opts.default_permissions) {
1304 - if (chmod(get_lock_file_path(&tables_list_lock),
1296 + if (chmod(tables_list_lock.path,
1297 st->opts.default_permissions) < 0) {
1298 err = REFTABLE_IO_ERROR;
1299 goto done;
@@ -1456,7 +1448,7 @@ static int stack_compact_range(struct reftable_stack *st,
1448 goto done;
1449 }
1450
1459 - err = write_in_full(get_lock_file_fd(&tables_list_lock),
1451 + err = write_in_full(tables_list_lock.fd,
1452 tables_list_buf.buf, tables_list_buf.len);
1453 if (err < 0) {
1454 err = REFTABLE_IO_ERROR;
@@ -1464,14 +1456,14 @@ static int stack_compact_range(struct reftable_stack *st,
1456 goto done;
1457 }
1458
1467 - err = stack_fsync(&st->opts, get_lock_file_fd(&tables_list_lock));
1459 + err = stack_fsync(&st->opts, tables_list_lock.fd);
1460 if (err < 0) {
1461 err = REFTABLE_IO_ERROR;
1462 unlink(new_table_path.buf);
1463 goto done;
1464 }
1465
1474 - err = commit_lock_file(&tables_list_lock);
1466 + err = flock_commit(&tables_list_lock);
1467 if (err < 0) {
1468 err = REFTABLE_IO_ERROR;
1469 unlink(new_table_path.buf);
@@ -1492,12 +1484,11 @@ static int stack_compact_range(struct reftable_stack *st,
1484 * readers, so it is expected that unlinking tables may fail.
1485 */
1486 for (i = 0; i < nlocks; i++) {
1495 - struct lock_file *table_lock = &table_locks[i];
1496 - const char *lock_path = get_lock_file_path(table_lock);
1487 + struct reftable_flock *table_lock = &table_locks[i];
1488
1489 reftable_buf_reset(&table_name);
1499 - err = reftable_buf_add(&table_name, lock_path,
1500 - strlen(lock_path) - strlen(".lock"));
1490 + err = reftable_buf_add(&table_name, table_lock->path,
1491 + strlen(table_lock->path) - strlen(".lock"));
1492 if (err)
1493 continue;
1494
@@ -1505,9 +1496,9 @@ static int stack_compact_range(struct reftable_stack *st,
1496 }
1497
1498 done:
1508 - rollback_lock_file(&tables_list_lock);
1499 + flock_release(&tables_list_lock);
1500 for (i = 0; table_locks && i < nlocks; i++)
1510 - rollback_lock_file(&table_locks[i]);
1501 + flock_release(&table_locks[i]);
1502 reftable_free(table_locks);
1503
1504 tmpfile_delete(&new_table);
reftable/system.c
+77
@@ -1,6 +1,7 @@
1 #include "system.h"
2 #include "basics.h"
3 #include "reftable-error.h"
4 +#include "../lockfile.h"
5 #include "../tempfile.h"
6
7 int tmpfile_from_pattern(struct reftable_tmpfile *out, const char *pattern)
@@ -47,3 +48,79 @@ int tmpfile_rename(struct reftable_tmpfile *t, const char *path)
48 return REFTABLE_IO_ERROR;
49 return 0;
50 }
51 +
52 +int flock_acquire(struct reftable_flock *l, const char *target_path,
53 + long timeout_ms)
54 +{
55 + struct lock_file *lockfile;
56 + int err;
57 +
58 + lockfile = reftable_malloc(sizeof(*lockfile));
59 + if (!lockfile)
60 + return REFTABLE_OUT_OF_MEMORY_ERROR;
61 +
62 + err = hold_lock_file_for_update_timeout(lockfile, target_path, LOCK_NO_DEREF,
63 + timeout_ms);
64 + if (err < 0) {
65 + reftable_free(lockfile);
66 + if (errno == EEXIST)
67 + return REFTABLE_LOCK_ERROR;
68 + return -1;
69 + }
70 +
71 + l->fd = get_lock_file_fd(lockfile);
72 + l->path = get_lock_file_path(lockfile);
73 + l->priv = lockfile;
74 +
75 + return 0;
76 +}
77 +
78 +int flock_close(struct reftable_flock *l)
79 +{
80 + struct lock_file *lockfile = l->priv;
81 + int ret;
82 +
83 + if (!lockfile)
84 + return REFTABLE_API_ERROR;
85 +
86 + ret = close_lock_file_gently(lockfile);
87 + l->fd = -1;
88 + if (ret < 0)
89 + return REFTABLE_IO_ERROR;
90 +
91 + return 0;
92 +}
93 +
94 +int flock_release(struct reftable_flock *l)
95 +{
96 + struct lock_file *lockfile = l->priv;
97 + int ret;
98 +
99 + if (!lockfile)
100 + return 0;
101 +
102 + ret = rollback_lock_file(lockfile);
103 + reftable_free(lockfile);
104 + *l = REFTABLE_FLOCK_INIT;
105 + if (ret < 0)
106 + return REFTABLE_IO_ERROR;
107 +
108 + return 0;
109 +}
110 +
111 +int flock_commit(struct reftable_flock *l)
112 +{
113 + struct lock_file *lockfile = l->priv;
114 + int ret;
115 +
116 + if (!lockfile)
117 + return REFTABLE_API_ERROR;
118 +
119 + ret = commit_lock_file(lockfile);
120 + reftable_free(lockfile);
121 + *l = REFTABLE_FLOCK_INIT;
122 + if (ret < 0)
123 + return REFTABLE_IO_ERROR;
124 +
125 + return 0;
126 +}
reftable/system.h
+44 -1
@@ -12,7 +12,6 @@ https://developers.google.com/open-source/licenses/bsd
12 /* This header glues the reftable library to the rest of Git */
13
14 #include "git-compat-util.h"
15 -#include "lockfile.h"
15
16 /*
17 * An implementation-specific temporary file. By making this specific to the
@@ -55,4 +54,48 @@ int tmpfile_delete(struct reftable_tmpfile *t);
54 */
55 int tmpfile_rename(struct reftable_tmpfile *t, const char *path);
56
57 +/*
58 + * An implementation-specific file lock. Same as with `reftable_tmpfile`,
59 + * making this specific to the implementation makes it possible to tie this
60 + * into signal or atexit handlers such that we know to clean up stale locks on
61 + * abnormal exits.
62 + */
63 +struct reftable_flock {
64 + const char *path;
65 + int fd;
66 + void *priv;
67 +};
68 +#define REFTABLE_FLOCK_INIT ((struct reftable_flock){ .fd = -1, })
69 +
70 +/*
71 + * Acquire the lock for the given target path by exclusively creating a file
72 + * with ".lock" appended to it. If that lock exists, we wait up to `timeout_ms`
73 + * to acquire the lock. If `timeout_ms` is 0 we don't wait, if it is negative
74 + * we block indefinitely.
75 + *
76 + * Retrun 0 on success, a reftable error code on error.
77 + */
78 +int flock_acquire(struct reftable_flock *l, const char *target_path,
79 + long timeout_ms);
80 +
81 +/*
82 + * Close the lockfile's file descriptor without removing the lock itself. This
83 + * is a no-op in case the lockfile has already been closed beforehand. Returns
84 + * 0 on success, a reftable error code on error.
85 + */
86 +int flock_close(struct reftable_flock *l);
87 +
88 +/*
89 + * Release the lock by unlinking the lockfile. This is a no-op in case the
90 + * lockfile has already been released or committed beforehand. Returns 0 on
91 + * success, a reftable error code on error.
92 + */
93 +int flock_release(struct reftable_flock *l);
94 +
95 +/*
96 + * Commit the lock by renaming the lockfile into place. Returns 0 on success, a
97 + * reftable error code on error.
98 + */
99 +int flock_commit(struct reftable_flock *l);
100 +
101 #endif
t/unit-tests/lib-reftable.c
+1
@@ -2,6 +2,7 @@
2 #include "test-lib.h"
3 #include "reftable/constants.h"
4 #include "reftable/writer.h"
5 +#include "strbuf.h"
6
7 void t_reftable_set_hash(uint8_t *p, int i, enum reftable_hash id)
8 {
t/unit-tests/t-reftable-block.c
+1
@@ -11,6 +11,7 @@ https://developers.google.com/open-source/licenses/bsd
11 #include "reftable/blocksource.h"
12 #include "reftable/constants.h"
13 #include "reftable/reftable-error.h"
14 +#include "strbuf.h"
15
16 static void t_ref_block_read_write(void)
17 {
t/unit-tests/t-reftable-pq.c
+1
@@ -9,6 +9,7 @@ https://developers.google.com/open-source/licenses/bsd
9 #include "test-lib.h"
10 #include "reftable/constants.h"
11 #include "reftable/pq.h"
12 +#include "strbuf.h"
13
14 static void merged_iter_pqueue_check(const struct merged_iter_pqueue *pq)
15 {
t/unit-tests/t-reftable-readwrite.c
+1
@@ -13,6 +13,7 @@ https://developers.google.com/open-source/licenses/bsd
13 #include "reftable/reader.h"
14 #include "reftable/reftable-error.h"
15 #include "reftable/reftable-writer.h"
16 +#include "strbuf.h"
17
18 static const int update_index = 5;
19
t/unit-tests/t-reftable-stack.c
+2
@@ -13,6 +13,8 @@ https://developers.google.com/open-source/licenses/bsd
13 #include "reftable/reader.h"
14 #include "reftable/reftable-error.h"
15 #include "reftable/stack.h"
16 +#include "strbuf.h"
17 +#include "tempfile.h"
18 #include <dirent.h>
19
20 static void clear_dir(const char *dirname)