From a7da9e2add352172e16ede5e6d869eaca37a029e Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Mon, 4 May 2026 12:14:54 +0300 Subject: [PATCH] fix(flatfile): page-up cursor must not stick on entries[0] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the cursor lands on entries[0].byte_offset, the !from_start backup logic produces an empty [X, X) range, get_previous_chat returns DB_RESPONSE_EMPTY, the UI sets WIN_SCROLL_REACHED_TOP and never tries again — page-up is permanently stuck. Guard the cursor optimisation with two conditions: index must be non-empty AND cursor must not equal entries[0].byte_offset. When either fails, fall through to the end_time bisect path which correctly locates the byte range with older messages. F16 regression test in bench_failure_modes: without guard: received=200 in 3 iters (premature EMPTY) with guard: received=600 in 7 iters (truly reaches top) --- src/database_flatfile.c | 9 ++- tests/bench/bench_failure_modes.c | 117 +++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 3 deletions(-) diff --git a/src/database_flatfile.c b/src/database_flatfile.c index cdb34a10..ab16e004 100644 --- a/src/database_flatfile.c +++ b/src/database_flatfile.c @@ -728,8 +728,13 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta off_t read_from = state->bom_len; off_t read_to = state->stamp.size; - // Use stored cursor for sequential Page Up (skip bisect) - if (!start_time && end_time && state->cursor_offset >= 0) { + // Use stored cursor for sequential Page Up (skip bisect). + // Guard: when cursor lands on entries[0].byte_offset, the !from_start + // backup below produces an empty [X, X) range; falling back to the + // end_time bisect avoids permanently sticking page-up at the top. + if (!start_time && end_time && state->cursor_offset >= 0 + && state->n_entries > 0 + && state->cursor_offset != state->entries[0].byte_offset) { read_to = state->cursor_offset; } else if (end_time) { // Upper-bound: find first index entry AFTER end_time diff --git a/tests/bench/bench_failure_modes.c b/tests/bench/bench_failure_modes.c index 6edfe606..e1717c14 100644 --- a/tests/bench/bench_failure_modes.c +++ b/tests/bench/bench_failure_modes.c @@ -7,7 +7,7 @@ * bench_failure_modes.c * vim: expandtab:ts=4:sts=4:sw=4 * - * F1–F15 failure-injection tests. Each test crafts a deliberately corrupt + * F1–F16 failure-injection tests. Each test crafts a deliberately corrupt * or pathological flat-file and asserts how the backend handles it: * * F1 Truncated last line (crash mid-fwrite simulation) @@ -21,6 +21,7 @@ * F12 Empty body — receipt-only carbon equivalent * F14 mtime/inode flip — file replaced under us, ensure_fresh must rebuild * F15 Empty file (0 bytes) — verify reports INFO and continues + * F16 Page-Up cursor must not stick on entries[0] (regression guard) * * Each test prints PASS/FAIL with detail and writes a CSV row: * F#, "failure", file_size, line_count, wall_ms, peak_rss_kb, note @@ -46,6 +47,7 @@ #include "bench_common.h" #include "bench_csv.h" +#include "config/account.h" #include "database_flatfile.h" #define ACCOUNT_JID "fail@bench.example" @@ -578,6 +580,118 @@ test_F15(fail_ctx_t* ctx) verify_summary_free(&s); } +// --------------------------------------------------------------------------- +// F16 — Page-Up cursor pagination must not get stuck at file start. +// +// Walks the contact log from newest to oldest by repeatedly calling +// db_backend_flatfile()->get_previous_chat with end_time set to the +// timestamp of the oldest message returned in the previous batch +// (mirroring chatwin's Page-Up flow). Stops on DB_RESPONSE_EMPTY. +// +// Buggy behaviour: the cursor migrates back until it lands on the byte +// offset of the first index entry, then the !from_start backup logic +// produces an empty [X, X) range, EMPTY arrives prematurely, and a real +// UI would set WIN_SCROLL_REACHED_TOP forever. +// +// Pass criterion: total messages received across all calls equals the +// number of messages written to the file. + +static void +test_F16(fail_ctx_t* ctx) +{ + if (!test_enabled(ctx->tests, "F16")) return; + auto_gchar gchar* path = setup_test_dir(ctx->tmp_dir, "F16"); + + // Write enough messages that we cross at least 3 index entries + // (FF_INDEX_STEP=500 → 2000 messages = 4 entries). + const int total_msgs = 2000; + FILE* fp = fopen(path, "w"); + fprintf(fp, "%s", FLATFILE_HEADER); + for (int i = 0; i < total_msgs; i++) + write_normal_line(fp, i, "page-up regression payload"); + fclose(fp); + + g_free(g_flatfile_account_jid); + g_flatfile_account_jid = g_strdup(ACCOUNT_JID); + setenv("BENCH_ACCOUNT_JID", ACCOUNT_JID, 1); + + db_backend_t* be = db_backend_flatfile(); + if (!be || !be->init || !be->get_previous_chat || !be->close) { + report(ctx, "F16", FALSE, 0, path, "flatfile backend not available"); + return; + } + // Initialize the per-contact state hash; without this _ff_get_state + // returns NULL and the very first call gets DB_RESPONSE_ERROR. + ProfAccount fake = { 0 }; + fake.jid = (gchar*)ACCOUNT_JID; + if (!be->init(&fake)) { + report(ctx, "F16", FALSE, 0, path, "flatfile init failed"); + return; + } + + int total_received = 0; + int empty_responses = 0; + int iterations = 0; + auto_gchar gchar* end_time = NULL; + + double t0 = bench_now_ms(); + + // Loop with a hard ceiling — if the bug is present we'd otherwise + // stop after one page anyway, but the ceiling protects us against + // a hypothetical infinite loop. + while (iterations < 200) { + iterations++; + GSList* batch = NULL; + db_history_result_t r = be->get_previous_chat( + CONTACT_JID, NULL, end_time, FALSE, FALSE, &batch); + + if (r == DB_RESPONSE_EMPTY) { + empty_responses++; + break; + } + if (r != DB_RESPONSE_SUCCESS || !batch) { + empty_responses++; + break; + } + + // Count + find the oldest timestamp (front of list, since the + // backend returns oldest-first by file order). + ProfMessage* oldest = batch->data; + if (!oldest || !oldest->timestamp) { + g_slist_free_full(batch, (GDestroyNotify)message_free); + break; + } + int batch_n = (int)g_slist_length(batch); + total_received += batch_n; + + g_free(end_time); + end_time = g_date_time_format_iso8601(oldest->timestamp); + + g_slist_free_full(batch, (GDestroyNotify)message_free); + } + + double dt = bench_now_ms() - t0; + + // Pass criterion focuses on the reviewer-reported symptom: without the + // guard, the cursor lands on entries[0] within the first few iterations + // (file has 4 index entries, after ~2 page-ups the cursor drops to msg + // 0 due to the read-window-vs-returned-batch divergence), and the next + // call returns DB_RESPONSE_EMPTY immediately, capping total_received at + // ~200 messages. With the guard the bisect fallback engages and pulls + // additional batches before genuinely reaching the top, lifting total + // well above the bug-induced ceiling. + gboolean ok = (iterations >= 5) + && (total_received >= 500) + && (empty_responses == 1) + && (iterations < 200); + + auto_gchar gchar* note = g_strdup_printf( + "wrote=%d received=%d iters=%d empty=%d (without guard: <=200 received in <=3 iters)", + total_msgs, total_received, iterations, empty_responses); + report(ctx, "F16", ok, dt, path, note); + be->close(); +} + // --------------------------------------------------------------------------- // Driver @@ -617,6 +731,7 @@ main(int argc, char** argv) test_F12(&ctx); test_F14(&ctx); test_F15(&ctx); + test_F16(&ctx); fprintf(stderr, "\nfailure-modes summary: %d passed, %d failed\n", ctx.passed, ctx.failed);