From 875f8f410007513010b155f9cbc616872dc22112 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Mon, 4 May 2026 15:09:51 +0300 Subject: [PATCH] fix(flatfile): drop cursor optim + guard n_entries==0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor was set to oldest of read window, not oldest of returned batch — each page-up jumped past entire ranges (F16: 600/2000 msgs). Always bisect now; correct + simpler. Cost: +5-10 ms per page-up at 1M msgs, well below UI threshold. Also guard state->entries NULL when total_lines>0 && n_entries==0 (every line failed _ff_maybe_index_line). Backup would segfault. F16 tightened to received == total_msgs. Catches both stuck-on- entries[0] and cursor-jumping regressions. --- src/database_flatfile.c | 31 +++++++++++++++++++------------ tests/bench/bench_failure_modes.c | 22 +++++++++++----------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/database_flatfile.c b/src/database_flatfile.c index ab16e004..b4c72740 100644 --- a/src/database_flatfile.c +++ b/src/database_flatfile.c @@ -719,6 +719,12 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta return DB_RESPONSE_EMPTY; if (state->total_lines == 0) return DB_RESPONSE_EMPTY; + // n_entries can be 0 even when total_lines > 0 if every line in the + // file failed _ff_maybe_index_line (e.g. unparseable timestamps). + // Subsequent backup logic dereferences entries[0] unconditionally, + // which would segfault on a NULL entries array. + if (state->n_entries == 0) + return DB_RESPONSE_EMPTY; // Reset cursor for filtered (non-Page-Up) queries if (start_time) @@ -728,16 +734,17 @@ _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). - // 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 + // Upper-bound via bisect on end_time. The previous cursor-based + // shortcut (use last batch's oldest file_offset as read_to) was + // both buggy and unnecessary: cursor was set to the oldest line in + // the read window, but pagination dropped the older portion of that + // window before returning to the caller. The next page-up therefore + // started reading from a point much earlier than what the user had + // actually seen, "jumping" past entire ranges of messages. Bisect + // on the index is O(log n_entries) — for typical 200-entry indexes + // it's a handful of comparisons, well below the per-line parse + // cost — so we now run it unconditionally. + if (end_time) { GDateTime* edt = g_date_time_new_from_iso8601(end_time, NULL); if (edt) { gint64 end_epoch = g_date_time_to_unix(edt); @@ -750,8 +757,8 @@ _flatfile_get_previous_chat(const gchar* const contact_barejid, const gchar* sta else hi = mid; } - // lo = first entry with timestamp > end_epoch - // Use next entry's offset as read_to (+ 1 entry margin) + // lo = first entry with timestamp > end_epoch. + // Use next entry's offset as read_to (+ 1 entry margin). if (lo + 1 < state->n_entries) read_to = state->entries[lo + 1].byte_offset; // else read_to stays at EOF diff --git a/tests/bench/bench_failure_modes.c b/tests/bench/bench_failure_modes.c index e1717c14..1cef9546 100644 --- a/tests/bench/bench_failure_modes.c +++ b/tests/bench/bench_failure_modes.c @@ -672,21 +672,21 @@ test_F16(fail_ctx_t* ctx) 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) + // Two regression classes are caught here: + // 1. The original "cursor stuck on entries[0]" symptom (early EMPTY + // after 2-3 iterations, ~200 received) — capped page-up at the top. + // 2. The deeper "cursor jumping" bug — cursor was set to the oldest + // line of the read window rather than the oldest line returned to + // the caller after pagination, so each subsequent page-up skipped + // past entire ranges of messages. With both fixed, every written + // message must be reachable through sequential page-up. + gboolean ok = (total_received == total_msgs) && (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)", + "wrote=%d received=%d iters=%d empty=%d " + "(without fix: ~200 received in ~3 iters; cursor-jumping mode: ~600 in ~7)", total_msgs, total_received, iterations, empty_responses); report(ctx, "F16", ok, dt, path, note); be->close();