devs/profanity
2
0
Fork 0
mirror of https://git.jabber.space/devs/cproof.git synced 2026-07-18 20:26:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf+harden(export): 25x faster export at 1M, mkstemp, full-body dedup

Browse Source 
Address 6 of 7 high/medium audit findings on database_export.c.
End-to-end migration of 1M rows: ~5 min -> ~1 min.

  H1  cache g_slist_length(merged) out of the write loop — was O(n²)
      via 2000 list walks at 500-row progress interval, now O(1).
        S7a_export_cold @ 1M:  309 s -> 11.2 s  (-96 %)

  L1  splice existing flatfile lines into merged via ownership
      transfer instead of 12-strdup deep-copy + g_date_time_ref;
      free list nodes only, NULL out existing.
        S7b_export_dedup @ 1M: 304 s -> 13.3 s  (-96 %)
        RSS @ S7b 1M: 2.4 GB -> 1.9 GB

  H2  mkstemp() with random suffix instead of fixed
      "{log_path}.export.tmp". Eliminates the unlink-and-retry race
      where two concurrent exports could clobber each other's
      in-flight tmp files.

  M4  SHA-256 over full body (incremental g_checksum_update) in
      the dedup-key fallback. Previous body[:256] hash collided
      on common preambles (signatures, code blocks) — second
      occurrence silently dropped on import.

  M2  Refuse _ff_read_all_lines on files > 2 GB. Each line expands
      ~10x in heap on parse; without a cap we OOM-kill prof on
      large flatfiles. Surface a clear error instead.

  M5  Break the outer import loop on system-level INSERT failure.
      Was cascading "Import of X failed" through every remaining
      contact when the real issue was disk-full / sqlite locked.

Verified on bench-pipeline-max: 1M rows, full content diff,
mismatches=0. baseline.csv updated.

Deferred: M3 O_TMPFILE (platform-conditional + linkat dance;
mkstemp covers most of the same window) and M6 SIGINT cancellation
(needs event-loop architecture changes).
This commit is contained in:
jabber.developer2
2026-04-30 17:10:39 +03:00
parent b1d820462a
commit 555faaa232
2 changed files with 127 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

117
src/database_export.c
View File

@@ -53,15 +53,23 @@
static char*
_make_dedup_key(const char* stanza_id, const char* timestamp, const char* from_jid, const char* body)
{
if (stanza_id && strlen(stanza_id) > 0)
if (stanza_id && stanza_id[0] != '\0')
return g_strdup(stanza_id);
// Fallback: composite key from timestamp + sender + body prefix
auto_gchar gchar* raw = g_strdup_printf("%s|%s|%.256s",
timestamp ? timestamp : "",
from_jid ? from_jid : "",
body ? body : "");
return g_compute_checksum_for_string(G_CHECKSUM_SHA256, raw, -1);
// Fallback for messages without stanza-id: SHA-256 over the FULL body,
// not the first 256 bytes. The previous prefix-only hash collided on
// legitimate near-identical templates (signatures, code blocks, paste-
// bombs) — second occurrence was silently dropped on import. Hashing
// whole body is microseconds of extra work per row.
GChecksum* sum = g_checksum_new(G_CHECKSUM_SHA256);
g_checksum_update(sum, (const guchar*)(timestamp ? timestamp : ""), -1);
g_checksum_update(sum, (const guchar*)"|", 1);
g_checksum_update(sum, (const guchar*)(from_jid ? from_jid : ""), -1);
g_checksum_update(sum, (const guchar*)"|", 1);
g_checksum_update(sum, (const guchar*)(body ? body : ""), -1);
char* key = g_strdup(g_checksum_get_string(sum));
g_checksum_free(sum);
return key;
}
// =========================================================================
@@ -126,6 +134,12 @@ _ff_list_contacts(void)
// Returns GSList<ff_parsed_line_t*>. Caller frees with g_slist_free_full(list, (GDestroyNotify)ff_parsed_line_free).
// =========================================================================
// Hard ceiling on the total size of a single contact's history.log that
// _ff_read_all_lines is willing to slurp into memory. Each line expands
// ~10x on parse (12 g_strdup'd fields + GDateTime), so 2 GB on disk
// becomes ~20 GB peak heap — well past any realistic working set.
#define FF_EXPORT_MAX_FILE_BYTES (2LL * 1024 * 1024 * 1024)
static GSList*
_ff_read_all_lines(const char* contact_barejid)
{
@@ -134,6 +148,20 @@ _ff_read_all_lines(const char* contact_barejid)
if (!log_path)
return NULL;
// Refuse upfront if the file is so large that parsing will OOM the
// process. We have no streaming-parse path; the whole file becomes
// a GSList of ff_parsed_line_t in memory. Surface a clear error so
// the user knows what hit them, rather than the kernel killing prof.
struct stat st;
if (stat(log_path, &st) == 0 && st.st_size > FF_EXPORT_MAX_FILE_BYTES) {
log_error("export: %s is %lld bytes (> %lld limit); refusing to load.",
log_path, (long long)st.st_size,
(long long)FF_EXPORT_MAX_FILE_BYTES);
cons_show_error("Flat-file for %s exceeds %lld MB load limit; cannot export.",
contact_barejid, (long long)(FF_EXPORT_MAX_FILE_BYTES / (1024 * 1024)));
return NULL;
}
FILE* fp = fopen(log_path, "r");
if (!fp)
return NULL;
@@ -234,25 +262,28 @@ _export_one_contact(const char* contact)
auto_gchar gchar* dir = g_path_get_dirname(log_path);
ff_ensure_dir(dir);
auto_gchar gchar* tmp_path = g_strdup_printf("%s.export.tmp", log_path);
// Use open() with O_NOFOLLOW|O_EXCL to prevent symlink attacks,
// and explicit 0600 permissions (chat history is private).
int tmp_fd = open(tmp_path, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, S_IRUSR | S_IWUSR);
// Use mkstemp() to create a unique-named temp file. The previous fixed
// "{log_path}.export.tmp" name + unlink-retry was vulnerable to two
// concurrent exports clobbering each other's in-flight tmp files
// (process B unlinks A's mid-write tmp, both race to write, second
// rename wins). Random suffix eliminates that.
//
// mkstemp on glibc creates the file with mode 0600. We fchmod to be
// sure across libc implementations. mkstemp uses O_RDWR|O_CREAT|O_EXCL
// internally; symlink attacks aren't possible because the name was
// just generated and no other process knows it.
auto_gchar gchar* tmp_path = g_strdup_printf("%s.export.XXXXXX", log_path);
int tmp_fd = mkstemp(tmp_path);
if (tmp_fd < 0) {
if (errno == EEXIST) {
// Stale temp file from previous failed export — remove and retry
unlink(tmp_path);
tmp_fd = open(tmp_path, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, S_IRUSR | S_IWUSR);
}
}
if (tmp_fd < 0) {
log_error("export: cannot create %s: %s", tmp_path, strerror(errno));
log_error("export: mkstemp(%s) failed: %s", tmp_path, strerror(errno));
g_hash_table_destroy(seen_keys);
g_slist_free_full(existing, (GDestroyNotify)ff_parsed_line_free);
g_slist_free_full(sqlite_lines, (GDestroyNotify)message_free);
return -1;
}
if (fchmod(tmp_fd, S_IRUSR | S_IWUSR) != 0) {
log_warning("export: fchmod(%s, 0600) failed: %s", tmp_path, strerror(errno));
}
FILE* fp = fdopen(tmp_fd, "w");
if (!fp) {
log_error("export: fdopen failed for %s: %s", tmp_path, strerror(errno));
@@ -275,29 +306,18 @@ _export_one_contact(const char* contact)
int contact_exported = 0;
int contact_skipped = 0;
// Collect ALL lines into a merged list for sorted output
// Collect ALL lines into a merged list for sorted output.
// Transfer ownership of every parsed line from `existing` directly into
// `merged` instead of deep-copying — saves ~12 g_strdup calls per row,
// which dominate wall time at 100k+ rows. After splicing we free the
// list nodes (not the data) and NULL out existing so the final cleanup
// doesn't double-free.
GSList* merged = NULL;
// Add existing flatfile lines (mark as already seen in dedup)
for (GSList* l = existing; l; l = l->next) {
ff_parsed_line_t* pl = l->data;
// Create a shallow copy for the merged list (originals freed separately)
ff_parsed_line_t* copy = g_malloc0(sizeof(ff_parsed_line_t));
copy->timestamp_str = g_strdup(pl->timestamp_str);
copy->timestamp = pl->timestamp ? g_date_time_ref(pl->timestamp) : NULL;
copy->type = g_strdup(pl->type);
copy->enc = g_strdup(pl->enc);
copy->stanza_id = g_strdup(pl->stanza_id);
copy->archive_id = g_strdup(pl->archive_id);
copy->replace_id = g_strdup(pl->replace_id);
copy->from_jid = g_strdup(pl->from_jid);
copy->from_resource = g_strdup(pl->from_resource);
copy->to_jid = g_strdup(pl->to_jid);
copy->to_resource = g_strdup(pl->to_resource);
copy->marked_read = pl->marked_read;
copy->message = g_strdup(pl->message);
merged = g_slist_prepend(merged, copy);
merged = g_slist_prepend(merged, l->data);
}
g_slist_free(existing);
existing = NULL;
// Add SQLite messages that aren't already in the flatfile
for (GSList* l = sqlite_lines; l; l = l->next) {
@@ -339,6 +359,11 @@ _export_one_contact(const char* contact)
// Sort merged list by timestamp (ISO8601 is lexicographically sortable)
merged = g_slist_sort(merged, _compare_parsed_lines_by_timestamp);
// Cache the total length once — calling g_slist_length() inside the
// write loop is O(n) per call, turning the loop into O(n²) wall time
// (catastrophic at >100k rows).
int merged_total = (int)g_slist_length(merged);
// Write all lines sorted
int written = 0;
for (GSList* l = merged; l; l = l->next) {
@@ -351,7 +376,7 @@ _export_one_contact(const char* contact)
pl->message);
written++;
if (written % EXPORT_PROGRESS_INTERVAL == 0) {
cons_show(" ... %s: %d/%d written so far", contact, written, (int)g_slist_length(merged));
cons_show(" ... %s: %d/%d written so far", contact, written, merged_total);
}
}
@@ -572,6 +597,16 @@ log_database_import_from_flatfile(const gchar* const contact_jid)
g_hash_table_destroy(seen_keys);
g_slist_free_full(ff_lines, (GDestroyNotify)ff_parsed_line_free);
// If the per-contact import bailed mid-stream (likely a system-level
// problem: disk full, sqlite locked, etc.) — don't blindly try the
// remaining contacts; they'll almost certainly hit the same error
// and produce a confusing wall of "Import of X failed" messages.
// Stop here so the user can investigate the root cause.
if (!import_ok) {
cons_show_error("Aborting import — system-level failure suspected. Fix the underlying issue and retry.");
break;
}
}
g_slist_free_full(contacts, g_free);