commit 9f9d4d87d596a24de7c150cddb50071850a6bb31
parent b3b79e5a65a89691d3383d2188f738feb452e506
Author: William Casarin <jb55@jb55.com>
Date: Thu, 17 Jul 2025 10:48:02 -0700
Merge windows memleak testing
This gets windows into a stable state wrt. heap corruption
William Casarin (14):
cli: more results
memory: fix a bunch of memory leaks
mem: search cursor close
mem: reaction stats cleanup
mem: relay iter cleanup
mem: builder clear before free
mem: close cursors in print helpers
test: free things that need to be freed
Revert "mem: search cursor close"
test: fix some memory issues
mem: re-enable profile freeing
win: fix heap corruption with flatbuf
search: fix memleak in profile search
Diffstat:
| M | Makefile | | | 4 | ++++ |
| M | ndb.c | | | 9 | ++++++--- |
| M | src/nostrdb.c | | | 76 | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------ |
| M | test.c | | | 30 | +++++++++++++++++++++++++++--- |
4 files changed, 95 insertions(+), 24 deletions(-)
diff --git a/Makefile b/Makefile
@@ -21,6 +21,8 @@ C_BINDINGS_COMMON=$(BINDINGS)/c/flatbuffers_common_builder.h $(BINDINGS)/c/flatb
C_BINDINGS=$(C_BINDINGS_COMMON) $(C_BINDINGS_PROFILE) $(C_BINDINGS_META)
BIN=ndb
+SANFLAGS = -fsanitize=leak
+
# Detect operating system
UNAME_S := $(shell uname -s)
@@ -189,6 +191,8 @@ testdata/db/.dir:
@mkdir -p testdata/db
touch testdata/db/.dir
+test: CFLAGS += $(SANFLAGS) # compile test objects with ASan/UBSan
+test: LDFLAGS += $(SANFLAGS) # link test binary with the sanitizer runtime
test: test.c $(DEPS) testdata/db/.dir
$(CC) $(CFLAGS) test.c $(LDS) $(LDFLAGS) -o $@
diff --git a/ndb.c b/ndb.c
@@ -362,10 +362,11 @@ int main(int argc, char *argv[])
ndb_filter_json(f, buf, sizeof(buf));
fprintf(stderr, "using filter '%s'\n", buf);
- struct ndb_query_result results[10000];
+ int rsize = 30000;
+ struct ndb_query_result *results = malloc(sizeof(struct ndb_query_result) * rsize);
+ assert(results);
ndb_begin_query(ndb, &txn);
-
clock_gettime(CLOCK_MONOTONIC, &t1);
if (key) {
results[0].note = ndb_get_note_by_key(&txn, key, NULL);
@@ -373,7 +374,7 @@ int main(int argc, char *argv[])
count = 1;
else
count = 0;
- } else if (!ndb_query(&txn, f, 1, results, 10000, &count)) {
+ } else if (!ndb_query(&txn, f, 1, results, rsize, &count)) {
fprintf(stderr, "query error\n");
}
clock_gettime(CLOCK_MONOTONIC, &t2);
@@ -386,7 +387,9 @@ int main(int argc, char *argv[])
}
ndb_end_query(&txn);
+ ndb_filter_destroy(f);
+ free(results);
} else if (argc == 3 && !strcmp(argv[1], "import")) {
if (!strcmp(argv[2], "-")) {
ndb_process_events_stream(ndb, stdin);
diff --git a/src/nostrdb.c b/src/nostrdb.c
@@ -591,6 +591,11 @@ int ndb_filter_end(struct ndb_filter *filter)
size_t orig_size;
#endif
size_t data_len, elem_len;
+ unsigned char *rel;
+
+ assert(filter);
+ assert(filter->elem_buf.start);
+
if (filter->finalized == 1)
return 0;
@@ -609,7 +614,10 @@ int ndb_filter_end(struct ndb_filter *filter)
memmove(filter->elem_buf.p, filter->data_buf.start, data_len);
// realloc the whole thing
- filter->elem_buf.start = realloc(filter->elem_buf.start, elem_len + data_len);
+ rel = realloc(filter->elem_buf.start, elem_len + data_len);
+ if (rel)
+ filter->elem_buf.start = rel;
+ assert(filter->elem_buf.start);
filter->elem_buf.end = filter->elem_buf.start + elem_len;
filter->elem_buf.p = filter->elem_buf.end;
@@ -2666,13 +2674,16 @@ void ndb_profile_record_builder_init(struct ndb_profile_record_builder *b)
void ndb_profile_record_builder_free(struct ndb_profile_record_builder *b)
{
- if (b->builder)
- free(b->builder);
if (b->flatbuf)
- free(b->flatbuf);
+ flatcc_builder_aligned_free(b->flatbuf);
+ if (b->builder) {
+ flatcc_builder_clear(b->builder);
+ free(b->builder);
+ }
b->builder = NULL;
b->flatbuf = NULL;
+
}
int ndb_process_profile_note(struct ndb_note *note,
@@ -3379,7 +3390,7 @@ static int ndb_write_reaction_stats(struct ndb_txn *txn, struct ndb_note *note)
if (root == NULL) {
ndb_debug("failed to create note metadata record\n");
- return 0;
+ goto fail;
}
// metadata is keyed on id because we want to collect stats regardless
@@ -3397,13 +3408,18 @@ static int ndb_write_reaction_stats(struct ndb_txn *txn, struct ndb_note *note)
if ((rc = mdb_put(txn->mdb_txn, txn->lmdb->dbs[NDB_DB_META], &key, &val, 0))) {
ndb_debug("write reaction stats to db failed: %s\n", mdb_strerror(rc));
- free(root);
- return 0;
+ goto fail;
}
free(root);
+ flatcc_builder_clear(&builder);
return 1;
+
+fail:
+ free(root);
+ flatcc_builder_clear(&builder);
+ return 0;
}
@@ -3657,7 +3673,8 @@ static int ndb_query_plan_execute_ids(struct ndb_txn *txn,
uint64_t note_id, until, *pint;
size_t note_size;
unsigned char *id;
- struct ndb_note_relay_iterator note_relay_iter;
+ struct ndb_note_relay_iterator note_relay_iter = {0};
+ struct ndb_note_relay_iterator *relay_iter = NULL;
until = UINT64_MAX;
@@ -3698,17 +3715,20 @@ static int ndb_query_plan_execute_ids(struct ndb_txn *txn,
if (!(note = ndb_get_note_by_key(txn, note_id, ¬e_size)))
continue;
- if (need_relays)
- ndb_note_relay_iterate_start(txn, ¬e_relay_iter, note_id);
+ relay_iter = need_relays ? ¬e_relay_iter : NULL;
+ if (relay_iter)
+ ndb_note_relay_iterate_start(txn, relay_iter, note_id);
// Sure this particular lookup matched the index query, but
// does it match the entire filter? Check! We also pass in
// things we've already matched via the filter so we don't have
// to check again. This can be pretty important for filters
// with a large number of entries.
- if (!ndb_filter_matches_with(filter, note, 1 << NDB_FILTER_IDS,
- need_relays ? ¬e_relay_iter : NULL))
+ if (!ndb_filter_matches_with(filter, note, 1 << NDB_FILTER_IDS, relay_iter)) {
+ ndb_note_relay_iterate_close(relay_iter);
continue;
+ }
+ ndb_note_relay_iterate_close(relay_iter);
ndb_query_result_init(&res, note, note_size, note_id);
if (!push_query_result(results, &res))
@@ -3959,8 +3979,9 @@ static int ndb_query_plan_execute_tags(struct ndb_txn *txn,
if (!(len = ndb_encode_tag_key(key_buffer, sizeof(key_buffer),
tags->field.tag, tag, taglen,
- until)))
- return 0;
+ until))) {
+ goto fail;
+ }
k.mv_data = key_buffer;
k.mv_size = len;
@@ -4006,6 +4027,9 @@ next:
mdb_cursor_close(cur);
return 1;
+fail:
+ mdb_cursor_close(cur);
+ return 0;
}
static int ndb_query_plan_execute_author_kinds(
@@ -4152,12 +4176,12 @@ static int ndb_query_plan_execute_profile_search(
// get the authors element after we finalize the filter, since
// the data could have moved
if (!(els = ndb_filter_find_elements(f, NDB_FILTER_AUTHORS)))
- return 0;
+ goto fail;
// grab pointer to pubkey in the filter so that we can
// update the filter as we go
if (!(filter_pubkey = ndb_filter_get_id_element(f, els, 0)))
- return 0;
+ goto fail;
for (i = 0; !query_is_full(results, limit); i++) {
if (i == 0) {
@@ -4173,10 +4197,16 @@ static int ndb_query_plan_execute_profile_search(
// Look up the corresponding note associated with that pubkey
if (!ndb_query_plan_execute_author_kinds(txn, f, results, limit))
- return 0;
+ goto fail;
}
+ ndb_search_profile_end(&profile_search);
+ ndb_filter_destroy(f);
return 1;
+
+fail:
+ ndb_filter_destroy(f);
+ return 0;
}
static int ndb_query_plan_execute_relay_kinds(
@@ -5598,7 +5628,7 @@ static void *ndb_writer_thread(void *data)
free((void*)msg->note.relay);
} else if (msg->type == NDB_WRITER_PROFILE) {
free(msg->profile.note.note);
- //ndb_profile_record_builder_free(&msg->profile.record);
+ ndb_profile_record_builder_free(&msg->profile.record);
} else if (msg->type == NDB_WRITER_BLOCKS) {
ndb_blocks_free(msg->blocks.blocks);
} else if (msg->type == NDB_WRITER_NOTE_RELAY) {
@@ -7821,6 +7851,8 @@ int ndb_print_author_kind_index(struct ndb_txn *txn)
i++;
}
+ mdb_cursor_close(cur);
+
return i;
}
@@ -7845,6 +7877,8 @@ int ndb_print_relay_kind_index(struct ndb_txn *txn)
i++;
}
+ mdb_cursor_close(cur);
+
return i;
}
@@ -7864,6 +7898,8 @@ int ndb_print_tag_index(struct ndb_txn *txn)
i++;
}
+ mdb_cursor_close(cur);
+
return 1;
}
@@ -7886,6 +7922,8 @@ int ndb_print_kind_keys(struct ndb_txn *txn)
i++;
}
+ mdb_cursor_close(cur);
+
return 1;
}
@@ -7913,6 +7951,8 @@ int ndb_print_search_keys(struct ndb_txn *txn)
i++;
}
+ mdb_cursor_close(cur);
+
return 1;
}
diff --git a/test.c b/test.c
@@ -76,12 +76,13 @@ static void test_filters()
assert(ndb_filter_start_field(f, NDB_FILTER_KINDS) == 0);
assert(ndb_filter_start_field(f, NDB_FILTER_TAGS) == 0);
ndb_filter_end_field(f);
- ndb_filter_end(f);
// should be sorted after end
assert(current->elements[0] == 2);
assert(current->elements[1] == 1337);
+ ndb_filter_end(f);
+
// try matching the filter
assert(ndb_filter_matches(f, note));
@@ -233,6 +234,7 @@ static void test_timeline_query()
assert(ndb_query(&txn, &filter, 1, results,
sizeof(results)/sizeof(results[0]), &count));
ndb_end_query(&txn);
+ ndb_filter_destroy(&filter);
assert(count == 10);
}
@@ -291,6 +293,9 @@ static void test_fetched_at()
fetched_at = ndb_read_last_profile_fetch(&txn, pubkey);
assert(fetched_at == t1);
+
+ ndb_end_query(&txn);
+ ndb_destroy(ndb);
}
static void test_reaction_counter()
@@ -422,6 +427,7 @@ static void test_profile_updates()
assert(!strcmp(name, "c"));
+ ndb_end_query(&txn);
ndb_destroy(ndb);
free(json);
}
@@ -912,6 +918,7 @@ static void test_parse_filter_json()
}
}
+ ndb_filter_destroy(f);
}
static void test_parse_json() {
@@ -1413,6 +1420,7 @@ static void test_tag_query()
assert(!strcmp(ndb_note_content(results[0].note), "hi"));
ndb_end_query(&txn);
+ ndb_filter_destroy(f);
ndb_destroy(ndb);
}
@@ -1491,6 +1499,7 @@ static void test_query()
assert(!strcmp(ndb_note_content(results[1].note), "what"));
ndb_end_query(&txn);
+ ndb_filter_destroy(f);
ndb_destroy(ndb);
}
@@ -1614,6 +1623,7 @@ static void test_subscriptions()
assert(ndb_num_subscriptions(ndb) == 0);
ndb_end_query(&txn);
+ ndb_filter_destroy(f);
ndb_destroy(ndb);
}
@@ -1675,6 +1685,7 @@ static void test_weird_note_corruption() {
assert(i == 1);
ndb_end_query(&txn);
+ ndb_filter_destroy(f);
ndb_destroy(ndb);
}
@@ -1703,6 +1714,9 @@ static void test_filter_eq() {
ndb_filter_end(f2);
assert(ndb_filter_eq(f, f2));
+
+ ndb_filter_destroy(f);
+ ndb_filter_destroy(f2);
}
static void test_filter_is_subset() {
@@ -1738,6 +1752,9 @@ static void test_filter_is_subset() {
assert(ndb_filter_is_subset_of(k, g) == 1);
assert(ndb_filter_is_subset_of(ki, k) == 1);
assert(ndb_filter_is_subset_of(k, ki) == 0);
+
+ ndb_filter_destroy(k);
+ ndb_filter_destroy(ki);
}
static void test_filter_search()
@@ -1752,6 +1769,7 @@ static void test_filter_search()
ndb_filter_end_field(f);
assert(ndb_filter_end(f));
+ ndb_filter_destroy(f);
}
static void test_filter_parse_search_json() {
@@ -1784,6 +1802,8 @@ static void test_filter_parse_search_json() {
assert(ndb_filter_json(f, (char *)buf, sizeof(buf)));
printf("search json: '%s'\n", (const char *)buf);
assert(!strcmp((const char*)buf, json));
+
+ ndb_filter_destroy(f);
}
static void test_note_relay_index()
@@ -1862,6 +1882,7 @@ static void test_note_relay_index()
assert(ndb_end_query(&txn));
// Cleanup
+ ndb_filter_destroy(f);
ndb_destroy(ndb);
printf("ok test_note_relay_index\n");
@@ -1903,6 +1924,7 @@ static void test_nip50_profile_search() {
assert(!memcmp(ndb_note_id(result.note), expected_id, 32));
// Cleanup
+ ndb_filter_destroy(f);
ndb_destroy(ndb);
printf("ok test_nip50_profile_search\n");
@@ -1954,7 +1976,7 @@ static void test_custom_filter()
struct ndb_filter filter, *f = &filter;
struct ndb_filter filter2, *f2 = &filter2;
int count, nres = 2;
- uint64_t sub_id, note_key;
+ uint64_t sub_id, note_keys[2];
struct ndb_query_result results[2];
struct ndb_ingest_meta meta;
@@ -1989,7 +2011,7 @@ static void test_custom_filter()
assert(ndb_process_event_with(ndb, reply_json, strlen(reply_json), &meta));
for (nres = 2; nres > 0;)
- nres -= ndb_wait_for_notes(ndb, sub_id, ¬e_key, 2);
+ nres -= ndb_wait_for_notes(ndb, sub_id, note_keys, 2);
ndb_begin_query(ndb, &txn);
ndb_query(&txn, f, 1, results, 2, &count);
@@ -1999,6 +2021,8 @@ static void test_custom_filter()
assert(ndb_note_id(results[0].note)[0] == 0x3d);
// Cleanup
+ ndb_filter_destroy(f);
+ ndb_filter_destroy(f2);
ndb_destroy(ndb);
printf("ok test_custom_filter\n");
