fix(flatfile): page-up cursor must not stick on entries[0]

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)
This commit is contained in:
2026-05-04 12:14:54 +03:00
parent 1fe3808b23
commit a7da9e2add
2 changed files with 123 additions and 3 deletions

View File

@@ -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

View File

@@ -7,7 +7,7 @@
* bench_failure_modes.c
* vim: expandtab:ts=4:sts=4:sw=4
*
* F1F15 failure-injection tests. Each test crafts a deliberately corrupt
* F1F16 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);