From b0bec448d30f198dc4817d71fc8c72654907a3a9 Mon Sep 17 00:00:00 2001 From: eunhhu Date: Wed, 30 Sep 2026 05:37:11 +0100 Subject: Balance the snapshot register's count, and free the runtime lists in order Under AddressSanitizer with leaks counted, the module checks lost 672 KB in 10,185 allocations. Two causes, both from the snapshot rework (d1e8f38): - A snapshot's register counted its snapshots with ++ under its spin, but a snapshot's last release took one off atomically outside it. A release on a reader's thread racing the owner's next snapshot could lose either change: the register then outlived every snapshot (the leak seen), or went while one still used it. The count is atomic now. - pure_app_stop handed what its lists let go to the newest snapshot (or freed it) and only then freed the outbox and the numbers heard, whose shared nodes and texts went to that hand-over's list again, for good. They are freed with the other lists now. make check builds the module checks a second time under AddressSanitizer and UndefinedBehaviorSanitizer (build/test-module-sanitized, 25 s) and fails on a leak. Before d1e8f38 the same build stops at a heap-use-after-free in retire; now it is clean. --- Makefile | 14 ++++++++++++-- core/module.c | 6 +++--- core/test-module.c | 4 ++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 3e3e143..cc50b01 100644 --- a/Makefile +++ b/Makefile @@ -75,6 +75,15 @@ $(BUILD)/test-module: core/test-module.c $(MODEL) $(STORE) $(HEADERS) @mkdir -p $(BUILD) $(CC) $(CPPFLAGS) $(CFLAGS) $(INCLUDES) -DPURE_STORE_TEST core/test-module.c $(MODEL) $(STORE) $(LDFLAGS) -pthread -lm $(LDLIBS) -o $@ +# The same, under AddressSanitizer and UndefinedBehaviorSanitizer with leaks +# counted: snapshots are read and let go on other threads (eebdeea left a +# race on their register and leaks that the plain build passes). +$(BUILD)/test-module-sanitized: core/test-module.c $(MODEL) $(STORE) $(HEADERS) + @mkdir -p $(BUILD) + $(CC) $(CPPFLAGS) -std=c17 -Wall -Wextra -Wpedantic -Werror -O1 -g -fno-omit-frame-pointer \ + -fsanitize=address,undefined -fno-sanitize-recover=all $(INCLUDES) -DPURE_STORE_TEST core/test-module.c \ + $(MODEL) $(STORE) $(LDFLAGS) -pthread -lm $(LDLIBS) -o $@ + # A link's two sides through a network of the test's own (pure-crypto.so beside it). $(BUILD)/test-link: link/test-link.c $(BUILD)/libpure-link.a $(BUILD)/pure-crypto.so $(wildcard link/*.h) @mkdir -p $(BUILD) @@ -104,11 +113,12 @@ $(BUILD)/test-tree-store: store/test-tree-store.c $(STORE) $(HEADERS) # The HTTP host's checks build their apps with aup (the sibling checkout) and # compare documents with the earlier document server. AUP ?= ../aup -check: all $(BUILD)/test-module $(BUILD)/test-tree-store $(BUILD)/test-log $(BUILD)/test-link $(BUILD)/test-message-json \ - $(BUILD)/module-probe $(BUILD)/clock-back.so +check: all $(BUILD)/test-module $(BUILD)/test-module-sanitized $(BUILD)/test-tree-store $(BUILD)/test-log \ + $(BUILD)/test-link $(BUILD)/test-message-json $(BUILD)/module-probe $(BUILD)/clock-back.so $(BUILD)/test-tree-store $(BUILD)/test-log $(BUILD)/test-module + ASAN_OPTIONS=detect_leaks=1 $(BUILD)/test-module-sanitized > /dev/null $(BUILD)/test-link $(BUILD)/test-message-json $(MAKE) -C $(AUP) build/aup diff --git a/core/module.c b/core/module.c index 8a57595..97270ef 100644 --- a/core/module.c +++ b/core/module.c @@ -2702,6 +2702,8 @@ void pure_app_stop(PureApp *app) { pure_list_free(app->lists[i]); pure_draft_close(app->list_drafts[i]); } + pure_list_free(app->outbox); /* the runtime's lists too, before what they let go is handed on */ + pure_list_free(app->heard); /* Items the newest snapshot may still read go with it. */ if (app->snapshot) { lock_register(app->snapshot->all); @@ -2711,8 +2713,6 @@ void pure_app_stop(PureApp *app) { } else pure_retired_free(&app->retired); pure_draft_close(app->stored_values); - pure_list_free(app->outbox); - pure_list_free(app->heard); free(app->carried); pure_draft_close(app->stored_links); pure_draft_close(app->stored_seq); @@ -3337,7 +3337,7 @@ PureSnapshot *pure_save_snapshot(PureApp *app, uint32_t runtime) { made->all = all; lock_register(all); all->epoch = made->epoch; - ++all->refs; + __atomic_add_fetch(&all->refs, 1, __ATOMIC_ACQ_REL); /* a snapshot's last release takes it off outside the lock */ made->older = last; if (last) { /* what the app let go while last was its newest goes with it */ last->newer = made; diff --git a/core/test-module.c b/core/test-module.c index b318aee..0a11f1c 100644 --- a/core/test-module.c +++ b/core/test-module.c @@ -2101,7 +2101,7 @@ static void deferred_messages(void) { CHECK(load(editor_spec, &module_a) == PURE_OK); PureApp app; unsigned char buffer[64]; - char key[8]; + char key[16]; CHECK(pure_app_start(&app, &module_a, pure_tree_data(path)) == PURE_OK && pure_app_defer(&app, true)); for (int i = 0; i < 3; ++i) { snprintf(key, sizeof(key), "k%d", i); @@ -2137,7 +2137,7 @@ static void pending_keys(void) { CHECK(load(editor_spec, &module_a) == PURE_OK); PureApp app; unsigned char buffer[64]; - char key[8]; + char key[16]; CHECK(pure_app_start(&app, &module_a, pure_tree_data(path)) == PURE_OK && pure_app_defer(&app, true)); for (int i = 0; i < 200; ++i) { snprintf(key, sizeof(key), "p%d", i); -- cgit v1.2.3