diff --git a/reftable/reader.c b/reftable/reader.c index 037417fcf63d4a..64a0953e687555 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -621,6 +621,7 @@ int reftable_reader_new(struct reftable_reader **out, r->source = *source; r->name = xstrdup(name); r->hash_id = 0; + r->refcount = 1; err = block_source_read_block(source, &footer, r->size, footer_size(r->version)); @@ -645,10 +646,21 @@ int reftable_reader_new(struct reftable_reader **out, return err; } -void reftable_reader_free(struct reftable_reader *r) +void reftable_reader_incref(struct reftable_reader *r) +{ + if (!r->refcount) + BUG("cannot increment ref counter of dead reader"); + r->refcount++; +} + +void reftable_reader_decref(struct reftable_reader *r) { if (!r) return; + if (!r->refcount) + BUG("cannot decrement ref counter of dead reader"); + if (--r->refcount) + return; block_source_close(&r->source); FREE_AND_NULL(r->name); reftable_free(r); @@ -812,7 +824,7 @@ int reftable_reader_print_blocks(const char *tablename) } done: - reftable_reader_free(r); + reftable_reader_decref(r); table_iter_close(&ti); return err; } diff --git a/reftable/reader.h b/reftable/reader.h index 88b4f3b421299c..3710ee09b4ca32 100644 --- a/reftable/reader.h +++ b/reftable/reader.h @@ -50,6 +50,8 @@ struct reftable_reader { struct reftable_reader_offsets ref_offsets; struct reftable_reader_offsets obj_offsets; struct reftable_reader_offsets log_offsets; + + uint64_t refcount; }; const char *reader_name(struct reftable_reader *r); diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index 2f2ff787b261d2..0494e7955a5f37 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -279,7 +279,7 @@ static void test_log_write_read(void) /* cleanup. */ strbuf_release(&buf); free_names(names); - reftable_reader_free(reader); + reftable_reader_decref(reader); } static void test_log_zlib_corruption(void) @@ -341,7 +341,7 @@ static void test_log_zlib_corruption(void) reftable_iterator_destroy(&it); /* cleanup. */ - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&buf); } @@ -383,7 +383,7 @@ static void test_table_read_write_sequential(void) EXPECT(j == N); reftable_iterator_destroy(&it); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&buf); free_names(names); } @@ -431,7 +431,7 @@ static void test_table_read_api(void) } reftable_iterator_destroy(&it); reftable_free(names); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&buf); } @@ -498,7 +498,7 @@ static void test_table_read_write_seek(int index, int hash_id) reftable_free(names[i]); } reftable_free(names); - reftable_reader_free(reader); + reftable_reader_decref(reader); } static void test_table_read_write_seek_linear(void) @@ -611,7 +611,7 @@ static void test_table_refs_for(int indexed) strbuf_release(&buf); free_names(want_names); reftable_iterator_destroy(&it); - reftable_reader_free(reader); + reftable_reader_decref(reader); } static void test_table_refs_for_no_index(void) @@ -657,7 +657,7 @@ static void test_write_empty_table(void) EXPECT(err > 0); reftable_iterator_destroy(&it); - reftable_reader_free(rd); + reftable_reader_decref(rd); strbuf_release(&buf); } @@ -863,7 +863,7 @@ static void test_write_multiple_indices(void) reftable_iterator_destroy(&it); reftable_writer_free(writer); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&writer_buf); strbuf_release(&buf); } @@ -919,7 +919,7 @@ static void test_write_multi_level_index(void) reftable_iterator_destroy(&it); reftable_writer_free(writer); - reftable_reader_free(reader); + reftable_reader_decref(reader); strbuf_release(&writer_buf); strbuf_release(&buf); } diff --git a/reftable/reftable-reader.h b/reftable/reftable-reader.h index 8a05c846858a1b..a600452b565803 100644 --- a/reftable/reftable-reader.h +++ b/reftable/reftable-reader.h @@ -33,6 +33,18 @@ struct reftable_reader; int reftable_reader_new(struct reftable_reader **pp, struct reftable_block_source *src, const char *name); +/* + * Manage the reference count of the reftable reader. A newly initialized + * reader starts with a refcount of 1 and will be deleted once the refcount has + * reached 0. + * + * This is required because readers may have longer lifetimes than the stack + * they belong to. The stack may for example be reloaded while the old tables + * are still being accessed by an iterator. + */ +void reftable_reader_incref(struct reftable_reader *reader); +void reftable_reader_decref(struct reftable_reader *reader); + /* Initialize a reftable iterator for reading refs. */ void reftable_reader_init_ref_iterator(struct reftable_reader *r, struct reftable_iterator *it); @@ -44,9 +56,6 @@ void reftable_reader_init_log_iterator(struct reftable_reader *r, /* returns the hash ID used in this table. */ uint32_t reftable_reader_hash_id(struct reftable_reader *r); -/* closes and deallocates a reader. */ -void reftable_reader_free(struct reftable_reader *); - /* return an iterator for the refs pointing to `oid`. */ int reftable_reader_refs_for(struct reftable_reader *r, struct reftable_iterator *it, uint8_t *oid); diff --git a/reftable/stack.c b/reftable/stack.c index 0ac9cdf8de19fe..8e85f8b4d99708 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -186,7 +186,7 @@ void reftable_stack_destroy(struct reftable_stack *st) if (names && !has_name(names, name)) { stack_filename(&filename, st, name); } - reftable_reader_free(st->readers[i]); + reftable_reader_decref(st->readers[i]); if (filename.len) { /* On Windows, can only unlink after closing. */ @@ -290,7 +290,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, const char *name = reader_name(cur[i]); stack_filename(&table_path, st, name); - reftable_reader_free(cur[i]); + reftable_reader_decref(cur[i]); /* On Windows, can only unlink after closing. */ unlink(table_path.buf); @@ -299,7 +299,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, done: for (i = 0; i < new_readers_len; i++) - reftable_reader_free(new_readers[i]); + reftable_reader_decref(new_readers[i]); reftable_free(new_readers); reftable_free(cur); strbuf_release(&table_path); @@ -1534,7 +1534,7 @@ static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max, goto done; update_idx = reftable_reader_max_update_index(rd); - reftable_reader_free(rd); + reftable_reader_decref(rd); if (update_idx <= max) { unlink(table_path.buf); diff --git a/reftable/stack_test.c b/reftable/stack_test.c index de0669b7b8a956..bc3bf772749d54 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -1036,10 +1036,8 @@ static void test_reftable_stack_compaction_concurrent(void) static void unclean_stack_close(struct reftable_stack *st) { /* break abstraction boundary to simulate unclean shutdown. */ - int i = 0; - for (; i < st->readers_len; i++) { - reftable_reader_free(st->readers[i]); - } + for (size_t i = 0; i < st->readers_len; i++) + reftable_reader_decref(st->readers[i]); st->readers_len = 0; FREE_AND_NULL(st->readers); } diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c index 87c2f42aaed790..f6d855a9d7ffae 100644 --- a/t/helper/test-reftable.c +++ b/t/helper/test-reftable.c @@ -152,7 +152,7 @@ static int dump_reftable(const char *tablename) done: reftable_merged_table_free(mt); - reftable_reader_free(r); + reftable_reader_decref(r); return err; } diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c index 8f51aca1b625ab..081d3c8b69774a 100644 --- a/t/unit-tests/t-reftable-merged.c +++ b/t/unit-tests/t-reftable-merged.c @@ -115,7 +115,7 @@ merged_table_from_records(struct reftable_ref_record **refs, static void readers_destroy(struct reftable_reader **readers, const size_t n) { for (size_t i = 0; i < n; i++) - reftable_reader_free(readers[i]); + reftable_reader_decref(readers[i]); reftable_free(readers); } @@ -437,7 +437,7 @@ static void t_default_write_opts(void) err = reftable_merged_table_new(&merged, &rd, 1, GIT_SHA1_FORMAT_ID); check(!err); - reftable_reader_free(rd); + reftable_reader_decref(rd); reftable_merged_table_free(merged); strbuf_release(&buf); }