From 9d8e5b912ff682a193f46739d51f9030cc3fda8d Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Wed, 26 Aug 2026 16:54:31 +0100 Subject: [PATCH] Close the active segment before compaction removes source segments compact() deletes every source segment, including the one currently open for appends. LittleFS and FatFS refuse to unlink an open file, so on those filesystems the active segment survived compaction ("Failed to unlink ... Has open FD") and on POSIX-like filesystems the handle silently kept writing into an unlinked inode. Close the active file before phase 1 and reopen a segment on both exit paths: seg0 after a successful compaction (the internal callers then move on to seg1 as before), current_segment after a failed one, so a direct compact() call never leaves the store without a writable segment. Adds test_file_store_compact_keeps_store_writable, which fails on master (the record written after compact() is lost on reload) and passes here. --- include/microStore/FileStore.h | 7 +++++ test/test_file_store/test_file_store.cpp | 34 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/include/microStore/FileStore.h b/include/microStore/FileStore.h index c243e21..0a6429a 100644 --- a/include/microStore/FileStore.h +++ b/include/microStore/FileStore.h @@ -1433,6 +1433,11 @@ USTORE_LOG("[ustore] Moving temporary file to: %s\n", seg0); bool compact() { USTORE_LOG("[ustore] Compacting storage...\n"); + // The active segment is one of the sources removed below. LittleFS + // and FatFS refuse to unlink a file that is still open (POSIX would + // allow it), so close it first. Both exit paths reopen a segment so + // the store stays writable for callers that invoke compact() directly. + if (active_file) active_file.close(); // --- Phase 1: write COMPACTING journal (next_seg=0: no source segments deleted yet) --- write_journal(JOURNAL_COMPACTING, 0, 0); @@ -1542,6 +1547,7 @@ USTORE_LOG("[ustore] Closing tmp file: %s\n", tmp_path); outf.close(); if (!write_ok) { + open_segment(current_segment); // keep the store writable after a failed compaction if (committed_segs == 0) { // No source segments were deleted — safe to discard compact.tmp entirely. USTORE_LOG("[ustore] WARNING: Compact aborted: storage full, all segments preserved\n"); @@ -1561,6 +1567,7 @@ USTORE_LOG("[ustore] Closing tmp file: %s\n", tmp_path); write_journal(JOURNAL_COMMIT); finalize_compaction(); // rename + index rebuild + open_segment(current_segment); // seg0 = compacted data; internal callers move on to seg1 clear_journal(); diff --git a/test/test_file_store/test_file_store.cpp b/test/test_file_store/test_file_store.cpp index 4f05dc7..13222d0 100644 --- a/test/test_file_store/test_file_store.cpp +++ b/test/test_file_store/test_file_store.cpp @@ -669,6 +669,39 @@ void test_file_store_compact_basic() { TEST_ASSERT_EQUAL(0, memcmp(buf, "v3", 2)); } +// A direct compact() call must leave the store writable. The active segment is +// one of the source segments compaction deletes, so compact() has to close it +// (LittleFS/FatFS refuse to unlink an open file) and reopen a segment before +// returning; otherwise records written afterwards go to a stale handle and are +// lost on reload. +void test_file_store_compact_keeps_store_writable() { + reset_ram_fs(); + + uint32_t now = microStore::time(); + { + microStore::FileStore store; + auto fs = make_ram_fs(); + store.init(fs, "/p"); + store.put("a", "v1", /*ttl=*/0, now); + store.put("b", "v2", /*ttl=*/0, now); + store.remove("b"); + TEST_ASSERT_TRUE(store.compact()); + store.put("c", "v3", /*ttl=*/0, now); // written after compaction + TEST_ASSERT_TRUE(store.exists("c")); + } + remove_ram_file("/p_index.dat"); // force a rebuild from the segment files + + microStore::FileStore reader; + auto fs = make_ram_fs(); + reader.init(fs, "/p"); + + TEST_ASSERT_EQUAL(2u, reader.size()); // a(v1) and c(v3) + uint8_t buf[32]; uint16_t sz = sizeof(buf); + TEST_ASSERT_TRUE(reader.get("c", buf, &sz)); + TEST_ASSERT_EQUAL(2u, sz); + TEST_ASSERT_EQUAL(0, memcmp(buf, "v3", 2)); +} + /* =========================================================================== * Directory-mode prefix tests * @@ -800,6 +833,7 @@ int runUnityTests(void) { // Additional edge-case tests RUN_TEST(test_file_store_ttl_exists_expires); RUN_TEST(test_file_store_compact_basic); + RUN_TEST(test_file_store_compact_keeps_store_writable); // Directory-mode prefix RUN_TEST(test_dir_prefix_segment_and_index_names); RUN_TEST(test_legacy_prefix_unchanged);